Re: [Fortran-Experiments]: patch for error checking on binding labels and derived types, etc.

["Christopher D. Rickett" <crickett at lanl dot gov>]

On Tue, 27 Feb 2007, Tobias Burnus wrote:

> Hi Chris, hi all,
>
> (more to your patch below)
>
> First, I checked in my symbol patch (r122363):
>
> -   conf (is_bind_c, dummy);
> +   if (!function && !subroutine)
> +     conf (is_bind_c, dummy);
>
>
> I didn't add a test case yet, because there is one missing warning and
> one wrong warning:
> -----------------------------------
> ! { dg-do "compile" }
> ! Dummy arguments which are functions are allowed to have
> ! the Bind(c) flag set.
>
> MODULE OpenGL_glut
> IMPLICIT NONE
>
> ! Notes:
> ! (1) gives the wrong warning: "Variable 'func' is a parameter to the
> !     BIND(C) procedure 'glutcreatemenu' but may not be C interoperable"
> ! (2) gives no warning although integer has no C interop. kind.

i look into this once i can update and get your patch too.

> INTERFACE
>  FUNCTION glutCreateMenu(func) BIND(C,NAME="glutCreateMenu")
>    use iso_c_binding   ! ^^^^<<<<< (1)
>    INTEGER :: glutCreateMenu   ! <<<<<<<< (2)
>    INTERFACE                   ! The dummy argument 'func'
>      SUBROUTINE func() BIND(C) ! is allowed to have the bind(c) flag set
>      END SUBROUTINE func
>    END INTERFACE
>  END FUNCTION glutCreateMenu
> END INTERFACE
> END MODULE OpenGL_glut
>
> ! { dg-final { cleanup-modules "OpenGL_glut" } }
> -----------------------------------
>
>
> Let's wait for the ok of Steve.
> Chris, can you already commit the changes yourself?

not yet; i haven't looked at the website Steve sent me yet.  i'm not sure 
how soon i'll have permission to do the commits.

> Comments:
>
> +++ gcc/fortran/decl.c  (working copy)
> +     they're inteoperable if they're BIND(C) and they're params are all
>
> Spelling: interoperable.

oops.  i also think the third "they're" is wrong; it should be "their". 
i'll fix those in my copy.  do you want me to send another patch, or did 
you want to fix this?

> +          /* If it is a BIND(C) function, make sure the return value is a
> +             scalar value.  The previous tests in this function made sure
> +             the type is interoperable.  */
>
> My feeling is that you check previously only for dummy arguments not for
> return values. (This would at least explain why the test case above
> gives no warning.)

when a bind(c) variable is verified, it doesn't look at the parameters. 
that is done separately during resolve_symbol.  the verify_bind_c_sym 
should be warning the user about the type possibly not being C 
interoperable.  i'll have to see why the above case doesn't produce a 
warning.  i have codes that are very similar to it and they give a 
warning.

as a note, i use to verify all of the parameters when i verified the 
procedure, but i had an issue with multiple warnings/errors being 
produced.  i could look into moving the call back and doing it during the 
procedure resolution, if that's preferred?

> +            gfc_error ("Return type of BIND(C) FUNCTION '%s' at %L cannot "
> and
> +              gfc_error ("Return type of BIND(C) FUNCTION '%s' at %L
> cannot "
>
> Not-capitalizing the word "function" ?
>
> (By the way, I think some of the spaces should be replaced by tabs in
> and around these lines)

i can lower-case the "FUNCTION".  i had put it as all-caps simply because 
it was a token and i started doing all caps for other things, like 
BIND(C).  as for the tabs, i am sure i'm missing tabs in places because my 
emacs does not do tabs by default.  i'll try and tabify the functions i've 
added recently, but perhaps i should wait, since that will create a large, 
whitespace patch?

> +      /* TODO: If the name= was given and no binding label (name=""),
> we simply
> +         will let fortran mangle the symbol name as it usually would.
> +         However, this could still let C call it if the user looked up the
> +         symbol in the object file.  Should the name set during mangling in
> +         trans-decl.c be marked with characters that are invalid for C to
> +         prevent this?  */
>
> I think mangling is the wrong approach. Simply not creating the symbol
> name from the subroutine is enough.
>
> The idea is not to make it impossible for C to call these functions, but
> to prevent symbol-name clashes. Example:
> -----------------------------------
> module one
> contains
>  subroutine foo() bind(c)
>  end subroutine foo
> end module one
>
> module two
> contains
>  ! This procedure is only used accessed in C
>  ! as procedural pointer
>  subroutine foo() bind(c, name="")
>  end subroutine foo
> end module two
>
> use one, only: foo_one => foo
> use two, only: foo_two => foo
> end
> -----------------------------------
>
> Thus the name="" merely ensures that there are no two functions with the
> name "foo" and instead of coming up with some clever name
> name="_my_function_foo_not_to_be_called_directly", a simple name="" can
> be used.

what name should it be given for fortran to generate the symbol in the 
object file?  right now, with name="", i let fortran do what it normally 
would do.  is that acceptable?  that should prevent two functions from 
appearing with the same name "foo" i think.

> By the way, the example above does not work in gfortran:
>
> bar.f90:14.29:
> use two, only: foo_two => foo
>                            1
> bar.f90:3.16:
>  subroutine foo() bind(c)
>               2
> Error: Binding label 'foo' at (1) collides with the global entity 'foo'
> at (2)

i'll have to look at that.  thanks for the new test case.  the binding 
label verification definitely doesn't seem like the easiest thing to 
enforce.  yesterday, i worked on adding it for common blocks, which are a 
bit more complicated than other symbols.  i have a patch for them that i'm 
nearly ready to submit that should be a good start.

> +++ gcc/testsuite/gfortran.dg/binding_label_tests_10_main.f03
>
> You should add
> ! { dg-final { cleanup-modules "binding_label_tests_10_main
> binding_label_tests_10" } }
> to delete the created "binding_label_tests_10_main.mod" and
> binding_label_tests_10.mod files afterwards.
>
> +++ gcc/testsuite/gfortran.dg/bind_c_usage_7.f03
> Ditto for "x"
>
> +++ gcc/testsuite/gfortran.dg/binding_label_tests_11_main.f03
> Ditto for "binding_label_tests_11_main"  / binding_label_tests_11.f03
>
> +++ gcc/testsuite/gfortran.dg/binding_label_tests_10.f03
> Add a comment that "binding_label_tests_10.mod" should not be deleted
> since it is will be used by binding_label_tests_10_main.f03.
>
> Ditto for the file binding_label_tests_11.f03
>
> +++ gcc/testsuite/gfortran.dg/bind_c_dts_4.f03
> Delete test.mod
>
> +++ gcc/testsuite/gfortran.dg/binding_label_tests_9.f03
> And x.mod

i'll try and get these updated an submitted, though i might not get it 
done until i can finish the patch i have in the pipe.  how would you like 
me to update the minor typo's you pointed out?  would you like me to 
modify the patch i sent in and resend it?  or do you mind simply fixing 
those, and i can look at the new errors when i update?

Thanks for the feedback.
Chris