Re: [PATCH, Fortran] Allow CHARACTER literals in assignments and DATA statements - for review

2019-10-25 Thread Tobias Burnus

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

2019-10-21 Thread Steve Kargl
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

2019-10-21 Thread Mark Eggleston
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