PING - OK, to commit? I have a pending patch that needs this in place.
On 05/11/2019 09:55, Mark Eggleston wrote:
On 25/10/2019 09:03, Tobias Burnus wrote:
Hello Mark, hi all,
On 10/21/19 4:40 PM, Mark Eggleston wrote:
This is an extension to support a legacy feature supported by other
compilers such as flang and the sun compiler. As I understand it
this feature is associated with DEC so it enabled using
-fdec-char-conversions and by -fdec.
It allows character literals to be assigned to numeric (INTEGER,
REAL, COMPLEX) and LOGICAL variables by direct assignment or in DATA
statements.
* arith.c (hollerith2representation): Use
OPT_Wcharacter_truncation in
call to gfc_warning.
This has two effects: First, it permits to toggle the warning on and
off; secondly, it disables the warning by default. It is enabled by
-Wall, however. – I think that's acceptable: while Holleriths are
less transparent as normal strings, for normal strings the result is
identical.
+ result->representation.string[result_len] = '\0'; /* For debugger */
Tiny nit: full stop after 'debugger'.
Done.
+/* Convert character to integer. The constant will be padded or
truncated. */
And here an extra space before '*/'.
Done.
+Allowing character literals to be used in a similar way to
Hollerith constants
+is a non-standard extension.
+
+Character literals can be used in @code{DATA} statements and
assignments with
I wonder whether one should mention here explicitly that only
default-kind (i.e. kind=1) character strings are permitted.
Additionally, I wonder whether -fdec-char-conversion should be
mentioned here – without, it is not supported and the error message
doesn't point to this option.
Now mentions -fdec-char-conversion and kind=1.
+
+ /* Flang allows character conversions similar to Hollerith
conversions
+ - the first characters will be turned into ascii values. */
Is this Flang or DEC or …? I thought we talk about legacy support and
Flang is not really legacy.
Re-worded.
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
+ if ((gfc_numeric_ts (&lhs->ts) || lhs->ts.type == BT_LOGICAL)
+ && rhs->ts.type == BT_CHARACTER
+ && rhs->expr_type != EXPR_CONSTANT)
+ {
+ gfc_error ("Cannot convert %s to %s at %L", gfc_typename (rhs),
+ gfc_typename (lhs), &rhs->where);
+ return false;
+ }
Maybe add a comment like:
/* Happens with flag_dec_char_conversions for nonconstant strings. */
might help casual readers to understand where this if comes from.
Done.
@@ -331,8 +332,9 @@ gfc_conv_constant_to_tree (gfc_expr * expr)
gfc_build_string_const (expr->representation.length,
expr->representation.string));
if (!integer_zerop (tmp) && !integer_onep (tmp))
- gfc_warning (0, "Assigning value other than 0 or 1 to LOGICAL"
- " has undefined result at %L", &expr->where);
+ gfc_warning (OPT_Wsurprising, "Assigning value other than 0
or 1 "
+ "to LOGICAL has undefined result at %L",
+ &expr->where);
I am not happy with this. We had odd issues with combining code
generated by gfortran and ifort and Booleans types ("logical").
Namely, gfortran uses 0 and 1 – while ifort uses -1 and 0. When using
".not. var", it is sufficient to flip a single bit – either the first
or the last bit – and it is sufficient to look only a single bit.
Hence, one can get ".not. var .eqv. var".
The same result one can get when assigning "-1" to logical. Hence, a
default warning makes more sense than -Wsurprising. At least,
-Wsurprising is enabled by default.
Hence, I wonder whether your 'OPT_Wsurprising' or
'flag_dec_char_conversions ? OPT_Wsurprising : 0' makes more sense.
The latter.
Actually, I don't quickly see whether 4_'string' (i.e. kind=4)
strings are rejected or not. The gfc_character2* functions all assume
kind=1 characters – while code like gfc_convert_constant or the
resolve.c code only looks at BT_CHARACTER.
On the other hand, the add_conv calls in intrintrinsic.c's
add_conversions are only added for the default-character kind.
In any case, can you add a test which checks that – even with
-fdec-char-conversion – assigning a 2_'string' and 4_'string' to a
integer/real/complex/logical will be rejected at compile time?
Did not add 2_'string' tests as 2 is not accepted as a valid kind for
characters. The addition of 4_'string' in a data statement resulted in
an ICE which has been fixed by only accepting characters of kind=1.
Otherwise, it looks okay to me.
Tobias
I noticed that warning were not produced for conversion to logicals,
re-ordering of an if..else if sequence fixes that problem. Additional
test cases have been added.
Steve Kargl suggested a revision to the conversion warning adding
"Nonstandard" to the text this has also been done.
Tested on x86_64 using make -j 8 check-fortran.
Please find attached the updated patch, the change logs follow. OK to
commit?
regards,
Mark
gcc/fortran/ChangeLog
Jim MacArthur <jim.macart...@codethink.co.uk>
Mark Eggleston <mark.eggles...@codethink.com>
* arith.c (hollerith2representation): Use
OPT_Wcharacter_truncation in
call to gfc_warning. Add character2representation,
gfc_character2int,
gfc_character2real, gfc_character2complex and gfc_character2logical.
* arith.h: Add prototypes for gfc_character2int, gfc_character2real,
gfc_character2complex and gfc_character2logical.
* expr.c (gfc_check_assign): Return true if left hand side is numeric
or logical and the right hand side is character and of kind=1.
* gfortran.texi: Add -fdec-char-conversions.
* intrinsic.c (add_conversions): Add conversions from character to
integer, real, complex and logical types for their supported kinds.
(gfc_convert_type_warn): Reorder if..else if.. sequence so that
warnings
are produced for conversion to logical.
* invoke.texi: Add option to list of options.
* invoke.texi: Add Character conversion subsection to Extensions
section.
* lang.opt: Add new option.
* options.c (set_dec_flags): Add SET_BITFLAG for
flag_dec_char_conversions.
* resolve.c (resolve_ordindary_assign): Issue error if the left hand
side is numeric or logical and the right hand side is a character
variable.
* simplify.c (gfc_convert_constant): Assign the conversion function
depending on destination type.
* trans-const.c (gfc_constant_to_tree): Use OPT_Wsurprising in
gfc_warning allowing the warning to be switched off only if
flag_dec_char_conversions is enabled.
gcc/testsuite/gfortran.dg
Jim MacArthur <jim.macart...@codethink.co.uk>
Mark Eggleston <mark.eggles...@codethink.com>
PR fortran/89103
* gfortran.dg/dec_char_conversion_in_assignment_1.f90: New test.
* gfortran.dg/dec_char_conversion_in_assignment_2.f90: New test.
* gfortran.dg/dec_char_conversion_in_assignment_3.f90: New test.
* gfortran.dg/dec_char_conversion_in_assignment_4.f90: New test.
* gfortran.dg/dec_char_conversion_in_assignment_5.f90: New test.
* gfortran.dg/dec_char_conversion_in_assignment_6.f90: New test.
* gfortran.dg/dec_char_conversion_in_assignment_7.f90: New test.
* gfortran.dg/dec_char_conversion_in_assignment_8.f90: New test.
* gfortran.dg/dec_char_conversion_in_data_1.f90: New test.
* gfortran.dg/dec_char_conversion_in_data_2.f90: New test.
* gfortran.dg/dec_char_conversion_in_data_3.f90: New test.
* gfortran.dg/dec_char_conversion_in_data_4.f90: New test.
* gfortran.dg/dec_char_conversion_in_data_5.f90: New test.
* gfortran.dg/dec_char_conversion_in_data_6.f90: New test.
* gfortran.dg/dec_char_conversion_in_data_7.f90: New test.
* gfortran.dg/hollerith5.f90: Add -Wsurprising to options.
* gfortran.dg/hollerith_legacy.f90: Add -Wsurprising to options.
* gfortran.dg/no_char_to_numeric_assign.f90: New test.
--
https://www.codethink.co.uk/privacy.html
