[Bug fortran/51308] New: PARAMETER attribute conflicts with SAVE attribute

[Bug fortran/51308] New: PARAMETER attribute conflicts with SAVE attribute

[matthias.moeller at math dot tu-dortmund.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

             Bug #: 51308
           Summary: PARAMETER attribute conflicts with SAVE attribute
    Classification: Unclassified
           Product: gcc
           Version: 4.6.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: matthias.moeller@math.tu-dortmund.de


module mymod
  use iso_c_binding
  implicit none

  private
  public :: c_ptr
  public :: c_null_ptr

end module mymod


does not compile and gives the following error

$> gfortran -c test.f90 
test.f90:7.22:

  public :: c_null_ptr
                      1
Error: PARAMETER attribute conflicts with SAVE attribute in 'c_null_ptr' at (1)

[Bug fortran/51308] PARAMETER attribute conflicts with SAVE attribute

[kargl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-11-26
                 CC|                            |kargl at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #1 from kargl at gcc dot gnu.org 2011-11-26 19:21:29 UTC ---
The problem comes about because of gen_special_c_interop_ptr() in
symbol.c has the following lines:

  /* Set up the symbol's important fields.  Save attr required so we can
     initialize the ptr to NULL.  */
  tmp_sym->attr.save = SAVE_EXPLICIT;

I think that we should be setting 

tmp_sym->attr.flavor = FL_PARAMETER;

or something equivalent rather than the save attribute.

[Bug fortran/51308] PARAMETER attribute conflicts with SAVE attribute

[sgk at troutmask dot apl.washington.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

--- Comment #2 from Steve Kargl <sgk at troutmask dot apl.washington.edu> 2011-11-27 21:56:30 UTC ---
On Sat, Nov 26, 2011 at 07:21:29PM +0000, kargl at gcc dot gnu.org wrote:
> The problem comes about because of gen_special_c_interop_ptr() in
> symbol.c has the following lines:
> 
>   /* Set up the symbol's important fields.  Save attr required so we can
>      initialize the ptr to NULL.  */
>   tmp_sym->attr.save = SAVE_EXPLICIT;
> 
> I think that we should be setting 
> 
> tmp_sym->attr.flavor = FL_PARAMETER;
> 
> or something equivalent rather than the save attribute.
> 

attr.flavor was actually set further down in the file.  The
following patch appears to work, but it ceratinly feels to
be a kludge.

Index: symbol.c
===================================================================
--- symbol.c    (revision 181731)
+++ symbol.c    (working copy)
@@ -742,7 +742,8 @@ check_conflict (symbol_attribute *attr, 
       conf2 (asynchronous);
       conf2 (threadprivate);
       conf2 (value);
-      conf2 (is_bind_c);
+      if (!(name && (strcmp(name, "c_null_ptr") == 0 || strcmp(name,
"c_null_funptr") == 0)))
+    conf2 (is_bind_c);
       conf2 (codimension);
       conf2 (result);
       break;
@@ -3765,7 +3766,11 @@ gen_special_c_interop_ptr (int ptr_id, c

   /* Set up the symbol's important fields.  Save attr required so we can
      initialize the ptr to NULL.  */
-  tmp_sym->attr.save = SAVE_EXPLICIT;
+  tmp_sym->attr.save = SAVE_IMPLICIT;
+  /* Must declare c_null_ptr and c_null_funptr as having the
+     PARAMETER attribute so they can be used in init expressions.  */
+  tmp_sym->attr.flavor = FL_PARAMETER;
+
   tmp_sym->ts.is_c_interop = 1;
   tmp_sym->attr.is_c_interop = 1;
   tmp_sym->ts.is_iso_c = 1;
@@ -3777,6 +3782,7 @@ gen_special_c_interop_ptr (int ptr_id, c
     tmp_sym->ts.u.derived = get_iso_c_binding_dt (ISOCBINDING_PTR);
   else
     tmp_sym->ts.u.derived = get_iso_c_binding_dt (ISOCBINDING_FUNPTR);
+
   if (tmp_sym->ts.u.derived == NULL)
     {
       /* This can occur if the user forgot to declare c_ptr or
@@ -3817,9 +3823,6 @@ gen_special_c_interop_ptr (int ptr_id, c
   c->expr = gfc_get_expr ();
   c->expr->expr_type = EXPR_NULL;
   c->expr->ts.is_iso_c = 1;
-  /* Must declare c_null_ptr and c_null_funptr as having the
-     PARAMETER attribute so they can be used in init expressions.  */
-  tmp_sym->attr.flavor = FL_PARAMETER;

   return SUCCESS;
 }

[Bug fortran/51308] PARAMETER attribute conflicts with SAVE attribute

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-28 07:51:02 UTC ---
(In reply to comment #2)
> -      conf2 (is_bind_c);
> +      if (!(name && (strcmp(name, "c_null_ptr") == 0 || strcmp(name,
> "c_null_funptr") == 0)))
> +    conf2 (is_bind_c);

I think one should use "if (!is_iso_c)". And why do you keep the save
attribute? I think the following should work as well.

(Actually, reading it again, the "Set up the symbol's important fields." is not
really a useful comment - and could thus go.)

--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -744,5 +744,6 @@ check_conflict (symbol_attribute *attr, const char *name,
locus *where)
       conf2 (value);
-      conf2 (is_bind_c);
       conf2 (codimension);
       conf2 (result);
+      if (!attr->is_iso_c)
+       conf2 (is_bind_c);
       break;
@@ -3765,5 +3766,3 @@ gen_special_c_interop_ptr (int ptr_id, const char 

-  /* Set up the symbol's important fields.  Save attr required so we can
-     initialize the ptr to NULL.  */
-  tmp_sym->attr.save = SAVE_EXPLICIT;
+  /* Set up the symbol's important fields. */
   tmp_sym->ts.is_c_interop = 1;
@@ -3772,2 +3771,3 @@ gen_special_c_interop_ptr (int ptr_id, const char 
   tmp_sym->ts.type = BT_DERIVED;
+  tmp_sym->attr.flavor = FL_PARAMETER;

@@ -3819,5 +3819,2 @@ gen_special_c_interop_ptr (int ptr_id, const char 
   c->expr->ts.is_iso_c = 1;
-  /* Must declare c_null_ptr and c_null_funptr as having the
-     PARAMETER attribute so they can be used in init expressions.  */
-  tmp_sym->attr.flavor = FL_PARAMETER;

[Bug fortran/51308] PARAMETER attribute conflicts with SAVE attribute

[sgk at troutmask dot apl.washington.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

--- Comment #4 from Steve Kargl <sgk at troutmask dot apl.washington.edu> 2011-11-28 13:58:02 UTC ---
On Mon, Nov 28, 2011 at 07:51:02AM +0000, burnus at gcc dot gnu.org wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308
> 
> --- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-28 07:51:02 UTC ---
> (In reply to comment #2)
> > -      conf2 (is_bind_c);
> > +      if (!(name && (strcmp(name, "c_null_ptr") == 0 || strcmp(name,
> > "c_null_funptr") == 0)))
> > +    conf2 (is_bind_c);
> 
> I think one should use "if (!is_iso_c)". And why do you keep the save
> attribute? I think the following should work as well.

I kept the SAVE attribute because the comment (which you remove
in your patch) claims that it is needed to actually allow the 
the compiler to initialize c_null_ptr and c_null_funptr to NULL.
I did not go through the code to validate the comment's claim.

I did not use is_iso_c because I forgot about its existence.
If your patch passes regression testing, go ahead and commit it.

[Bug fortran/51308] PARAMETER attribute conflicts with SAVE attribute

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-28 14:17:50 UTC ---
(In reply to comment #4)
> I kept the SAVE attribute because the comment (which you remove
> in your patch) claims that it is needed to actually allow the 
> the compiler to initialize c_null_ptr and c_null_funptr to NULL.

Well, an initialization (sym->value) can only be only used for PARAMETERs or
for variables in static memory ("SAVE"). Having both "SAVE" and "PARAMETER"
does not really make sense. I assume that first the SAVE was added - and only
later PARAMETER (with a similar comment and many lines later in the file).

I have submitted the patch for approval at
  http://gcc.gnu.org/ml/fortran/2011-11/msg00234.html
and given your approval in comment 4 I will commit it. Thanks for looking
through my patch and for debugging and creating the draft patch of comment 2.

[Bug fortran/51308] PARAMETER attribute conflicts with SAVE attribute

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-28 14:21:38 UTC ---
Author: burnus
Date: Mon Nov 28 14:21:33 2011
New Revision: 181778

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181778
Log:
2011-11-28  Tobias Burnus  <burnus@net-b.de>
            Steven G. Kargl  <kargl@gcc.gnu.org>

        PR fortran/51308
        * symbol.c (check_conflict): Ignore BIND(C) + PARAMETER
        conflicts for ISO_C_BINDING variables.
        (gen_special_c_interop_ptr): Don't mark c_ptr_null/c_funptr_null
        as SAVE.

2011-11-28  Tobias Burnus  <burnus@net-b.de>

        PR fortran/51308
        * gfortran.dg/iso_c_binding_compiler_4.f90: New.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/symbol.c
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/51308] PARAMETER attribute conflicts with SAVE attribute

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-28 14:27:22 UTC ---
FIXED on the trunk (4.7).

Thanks Matthias for reporting the bug; it is now fixed at the development
branch, which will be released around March 2012 as GCC 4.7.0.

See http://gcc.gnu.org/wiki/GFortranBinaries for some regular binary builds
(some updated nightly, others lagging behind). The page also contains a link
for building it manually from source or links to Linux distributions (at least
SUSE has already 4.7-devel snapshots on their build server).

[Bug fortran/51308] PARAMETER attribute conflicts with SAVE attribute

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51308

--- Comment #8 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-28 14:47:45 UTC ---
Author: burnus
Date: Mon Nov 28 14:47:39 2011
New Revision: 181779

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181779
Log:
Really commit the test case:

2011-11-28  Tobias Burnus  <burnus@net-b.de>

        PR fortran/51308
        * gfortran.dg/iso_c_binding_compiler_4.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/iso_c_binding_compiler_4.f90