[Bug fortran/117643] F_C_STRING from F23 is missing

[kargls at comcast dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117643

--- Comment #23 from kargls at comcast dot net ---
(In reply to anlauf from comment #20)
> (In reply to Jerry DeLisle from comment #19)
> > (In reply to kargls from comment #17)
> > > I suppose the error in check.cc(gfc_check_f_c_string) that starts
> > > with
> > > 
> > >     if (string->ts.type != BT_CHARACTER
> > >        || (string->ts.type == BT_CHARACTER
> > > 	  && (string->ts.kind != 1 || string->ts.is_c_interop != 1)))
> > >  
> > > can be suppressed for (string.expr_type == EXPR_CONSTANT &&
> > > string->ts.type == BT_CHARACTER && string->ts.kind == 1)
> 
> I am a little confused by the above logic.  So far I was under the
> impression that a string->ts.kind != 1 cannot be interoperable.  There
> is an existing PR (need to find it) that we illegally accept kind=4
> for BIND(C) procedures.  E.g.


18.2.3.9 F_C_STRING (STRING [, ASIS])
   ...
   STRING shall be a character scalar of kind C_CHAR.

If string->ts.type is not BT_CHARACTER. There is an argument mismatch.
So, issue an error.

If string->ts.type is BT_CHARACTER. Then, the kind must be 1 and
it should be marked as C interoperable.

   program foo
     use iso_c_binding, only : c_char, f_c_string
     implicit none
     character(kind=c_char,len=10) s1
     s1 = 'abc'
     if (len(f_c_string(s1)) /= 4) stop 1
   end program foo

(gdb) p *string
$1 = {expr_type = EXPR_VARIABLE, ts = {type = BT_CHARACTER, kind = 1,
      u = {derived = 0x804270390, cl = 0x804270390, pad = 69665680}, interface
= 0x0, 
      is_c_interop = 1, is_iso_c = 0, ...

The problem with f_c_string(c_char_'abc') is that the constant
has neither is_c_interop = 1 nor is_iso_c = 1 when it reaches 
gfc_check_f_c_string ().

(gdb) p *string
$3 = {expr_type = EXPR_CONSTANT, ts = {type = BT_CHARACTER, kind = 1,
      u = {derived = 0x804270510, cl = 0x804270510, pad = 69666064}, interface
= 0x0, 
      is_c_interop = 0, is_iso_c = 0, ...

I traced through primary.cc(match_string_constant), and is_c_interop
is set to 1 when c_char_'abc' is parsed.  Somewhere between parsing
the string constant and passing it to gfc_check_f_c_string, the expression
seems to have is_c_interop reset to 0.

I suspect that somewhere gfortran uses expr.cc(gfc_get_character_expr)
to copy the constant to a gfc_expr entity.  This function ignore ISO C
binding.

>   subroutine sub (s) bind(c)
>     character(kind=4,len=*), intent(in) :: s
>   end
> 
> is silently accepted although it should not.

Looks like a bug unrelated to the implementation of f_c_string().

> Of course we detect that
> when passed as argument to F_C_STRING, but other might scratch their head
> later when looking at the code...
> 
> > Of course after posting what I just did in Comment #18 I see Steve's comment
> > in 17. Maybe I have my logic all wrong on the error message. I saw the kind
> > = 1 and did not expect is_c_interop to be set since c_char_'string_constant'
> > is equivalent to 1_'string_constant'.
> 
> Other compilers seem to print identical values for KIND(1_"a") and
> KIND(c_char_"b"), as verified for NAG and Intel.
> 
> But we can check interoperability for variables (and in theory for functions
> returning character).
> 
> So if I come from the other side, which code to accept and which to diagnose,
> I tried:
> 
>   if (string->ts.type != BT_CHARACTER
>       || string->ts.kind != 1
>       || (string->expr_type == EXPR_VARIABLE && !string->ts.is_c_interop))
> 
> and threw it on:
> 
>   subroutine sub1 (s)
>     character(*), intent(in) :: s
>     print *, f_c_string (s)        ! detected
>     print *, f_c_string (trim(s))  ! not detected
>   end

With the logic in my original patch,

   program foo
     use iso_c_binding, only : c_char, f_c_string
     implicit none
     character(kind=c_char,len=10) s1
     s1 = 'abc'
     call sub1(s1)
     contains
      subroutine sub1 (s)
        character(*), intent(in) :: s
        print *, f_c_string (s)        ! detected
        print *, f_c_string (trim(s))  ! not detected
      end
   end program foo

f_c_string2.f90:10:26:

   10 |      print *, f_c_string (s)        ! detected
      |                          1
Error: 'string' argument of 'f_c_string' intrinsic at (1) shall have a type of
CHARACTER(KIND=C_CHAR)
f_c_string2.f90:11:26:

   11 |      print *, f_c_string (trim(s))  ! not detected
      |                          1
Error: 'string' argument of 'f_c_string' intrinsic at (1) shall have a type of
CHARACTER(KIND=C_CHAR)

Seems to detect the issue.

>   subroutine sub2 (s)
>     character(kind=c_char,len=*), intent(in) :: s
>     print *, f_c_string (s)        ! OK
>     print *, f_c_string (trim(s))  ! OK
>   end

   program foo
     use iso_c_binding, only : c_char, f_c_string
     implicit none
     character(kind=c_char,len=10) s1
     s1 = 'abc'
     call sub1(s1)
     contains
      subroutine sub1 (s)
        character(kind=c_char,len=*), intent(in) :: s
        print *, f_c_string (s)        ! detected
        print *, f_c_string (trim(s))  ! not detected
      end
   end program foo

Looks like a bug in the handling of trim(), which is likely present
with (all?) other functions that return a string.

  16.9.214 TRIM (STRING)
  ...
  Result Characteristics. Character with the same kind type parameter value as
STRING

IMHO, trim() should be setting is_c_interop if STRING has a kind
type parameter of c_char.  is_c_interop appears to be the only
way to distinguish kind('a') and kind(c_char_'a').

> I tried to extend to above conditions to
> .. (string->expr_type == EXPR_VARIABLE || string->expr_type ==
> EXPR_FUNCTION) ..
> but it appears that TRIM() keeps only the kind parameter, and when the
> argument
> is of kind C_CHAR the result string looses the interoperability attribute.
> 
> So we could ignore the issue with function results for the time being and
> use the above variant of the logic in gfc_check_f_c_string.
> 
> The two separate testcase could also be combined into one file as they share
> a lot of code but that's not really important.
> 
> What do you think?

I think the original logic is correct.  It is unfortunate that
kind('a') == kind(c_char_'a'), and the only way to determine 
if a string is interoperable is through the metadata is_c_interop
or is_iso_c.