Re: [PATCH, Fortran] Allow CHARACTER literals in assignments and DATA statements - for review
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'. +/* Convert character to integer. The constant will be padded or truncated. */ And here an extra space before '*/'. +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. + + /* 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. --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c + if ((gfc_numeric_ts (>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), >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. @@ -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", >where); + gfc_warning (OPT_Wsurprising, "Assigning value other than 0 or 1 " +"to LOGICAL has undefined result at %L", +>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. 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? Otherwise, it looks okay to me. Tobias
Re: [PATCH, Fortran] Allow CHARACTER literals in assignments and DATA statements - for review
On Mon, Oct 21, 2019 at 03:40:27PM +0100, 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. > Without this patch, the following code % cat a.f90 integer(4) a a = '1234' print *, a end produces an error % gfcx -c a.f90 a.f90:2:6: 2 | a = '1234' | 1 Error: Cannot convert CHARACTER(4) to INTEGER(4) at (1) With the new option or -fdec, I would hope/expect ar warning would be issued, which can only be suppressed by -w, e.g., % gfcx -c -fdec a.f90 a.f90:2:6: 2 | a = '1234' | 1 Warning: Nonstandard conversion of CHARACTER(4) to INTEGER(4) at (1) This hopefully will encourage people to fix their codes. -- Steve
[PATCH, Fortran] Allow CHARACTER literals in assignments and DATA statements - for review
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. Please find attached the patch which includes changes to the gfortran manual. Tested on x86_64 using "make check-fortran". Change logs: gcc/fortran/ChangeLog Jim MacArthur Mark Eggleston * 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. * gfortran.texi: Add -fdec-char-conversions. * intrinsic.c (add_convdersions): Add conversions from character to integer, real, complex and logical types for their supported kinds. * 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. gcc/testsuite/ChangeLog Jim MacArthur Mark Eggleston 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_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/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 >From 26a2a7f4a65331f519ced628dfe7e0fa7b3ce513 Mon Sep 17 00:00:00 2001 From: Jim MacArthur Date: Thu, 4 Feb 2016 17:18:30 + Subject: [PATCH] Allow CHARACTER literals in assignments and data statements Warnings are raised when this happens. Enable using -fdec-char-as-int or -fdec --- gcc/fortran/arith.c| 94 +- gcc/fortran/arith.h| 4 + gcc/fortran/expr.c | 5 ++ gcc/fortran/gfortran.texi | 24 ++ gcc/fortran/intrinsic.c| 32 +++- gcc/fortran/invoke.texi| 17 ++-- gcc/fortran/lang.opt | 5 ++ gcc/fortran/options.c | 1 + gcc/fortran/resolve.c | 9 +++ gcc/fortran/simplify.c | 29 ++- gcc/fortran/trans-const.c | 6 +- .../dec_char_conversion_in_assignment_1.f90| 61 ++ .../dec_char_conversion_in_assignment_2.f90| 61 ++ .../dec_char_conversion_in_assignment_3.f90| 61 ++ .../gfortran.dg/dec_char_conversion_in_data_1.f90 | 69 .../gfortran.dg/dec_char_conversion_in_data_2.f90 | 69 .../gfortran.dg/dec_char_conversion_in_data_3.f90 | 69 .../gfortran.dg/dec_char_conversion_in_data_4.f90 | 9 +++ gcc/testsuite/gfortran.dg/hollerith5.f90 | 5 +- gcc/testsuite/gfortran.dg/hollerith_legacy.f90 | 2 +- .../gfortran.dg/no_char_to_numeric_assign.f90 | 21 + 21 files changed, 634 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/dec_char_conversion_in_assignment_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/dec_char_conversion_in_assignment_2.f90 create mode 100644 gcc/testsuite/gfortran.dg/dec_char_conversion_in_assignment_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/dec_char_conversion_in_data_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/dec_char_conversion_in_data_2.f90 create mode 100644 gcc/testsuite/gfortran.dg/dec_char_conversion_in_data_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/dec_char_conversion_in_data_4.f90 create mode 100644