[PATCH] Fold VIEW_CONVERT_EXPR <type, STRING_CST> generated by Fortran FE a lot (PR target/35366)

[Brooks Moses]

Jakub Jelinek wrote, at 11/11/2008 1:01 PM:
> So would the following be acceptable?  It will warn both for
> logical l
> l = 4Hfoob

Personally, I feel that this should be an error, not a warning.  But if
distinguishing this from the transfer case complicates the code (which
my interpretation of your patch would suggest that it does), or if other
people feel differently, I don't have a strong opinion here.

> and
> l = transfer (42, .true.)

That sounds like a reasonable result.  Note, though, that the following
should not give a warning:

l = transfer(transfer(.true., 3.1415), .true.)

Similarly, there are other things that reliably transfer into integer
values of 0 and 1 (e.g., appropriate string values, though those differ
with endianness) that should not give warnings when transferred to a
logical.

I think we should also add a testcase to confirm that this compiles
without warning, if we don't have one already:

i = [something defined at runtime]
l = transfer(i, .true.)

(Also, we should stop using lower-case ells for logical variables,
because they are horrid for legibility.)

> --- gcc/fortran/trans-const.c.jj	2008-09-30 16:56:44.000000000 +0200
> +++ gcc/fortran/trans-const.c	2008-11-11 21:50:16.000000000 +0100
[...]
> +	  if (TREE_CODE (tmp) == INTEGER_CST)
> +	    {
> +	      if (!integer_zerop (tmp) && !integer_onep (tmp))
> +		gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
> +			     " at %L has undefined result", &expr->where);
> +	    }

This looks good.

> +	  else
> +	    gfc_warning ("Assigning value other than 0 or 1 to LOGICAL"
> +			 " at %L might have undefined result", &expr->where);

This doesn't -- if I'm understanding it correctly, it warns on any cast
of a non-integer value to a logical, regardless of whether the result
might or might not in fact be undefined.

I'd be okay with a lack of warning here.  Or we could be really fancy
and transfer all non-integer inputs to integers and then do the integer
checks, but that seems excessive.

> --- gcc/testsuite/gfortran.dg/hollerith.f90.jj	2008-09-30 16:56:06.000000000 +0200
> +++ gcc/testsuite/gfortran.dg/hollerith.f90	2008-11-11 13:52:38.000000000 +0100
> @@ -8,7 +8,7 @@ character z1(4)
>  character*4 z2(2,2)
>  character*80 line
>  integer i
> -logical l
> +integer j
>  real r
>  character*8 c
>  
> @@ -20,15 +20,15 @@ data z2/4h(i7),'xxxx','xxxx','xxxx'/
>  
>  z2 (1,2) = 4h(i8)
>  i = 4hHell
> -l = 4Ho wo
> +j = 4Ho wo
>  r = 4Hrld! 
> -write (line, '(3A4)') i, l, r
> +write (line, '(3A4)') i, j, r
>  if (line .ne. 'Hello world!') call abort
>  i = 2Hab
>  r = 2Hab
> -l = 2Hab
> +j = 2Hab
>  c = 2Hab
> -write (line, '(3A4, 8A)') i, l, r, c
> +write (line, '(3A4, 8A)') i, j, r, c
>  if (line .ne. 'ab  ab  ab  ab      ') call abort
>  
>  write(line, '(4A8, "!")' ) x

Here, we should just get rid of "l" entirely, rather than making it into
a clone of "i".  There's no point in the redundant tests.

> --- gcc/testsuite/gfortran.dg/transfer_simplify_4.f90.jj	2008-09-30 16:56:06.000000000 +0200
> +++ gcc/testsuite/gfortran.dg/transfer_simplify_4.f90	2008-11-11 21:39:07.000000000 +0100
> @@ -6,25 +6,9 @@
>    implicit none
>  
>    integer, parameter :: ip1 = 42
> -  logical, parameter :: ap1 = transfer(ip1, .true.)
> -  integer, parameter :: ip2 = transfer(ap1, 0)
> -
> -  logical :: a
> +  integer, parameter :: ip2 = transfer(transfer(ip1, .true.), 0)
>    integer :: i
>    
>    i = transfer(transfer(ip1, .true.), 0)
>    if (i .ne. ip1) call abort ()

This needs a comment: "This is a quality-of-implementation issue, not
required by the standard.  See discussion at:" and a link to this thread.

Also, you never test the result of ip2.  It looks redundant with the
test for i; I don't see the need for both.  So I'd suggest either
getting rid of it or testing it.

I've been out of the GFortran loop for long enough that I'd like to have
someone else approve this before committing, but my take is that it's
fine with the above changes.

- Brooks