public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function
       [not found] <3150885.ufrke4QsgI@andrew-precision-3520>
@ 2020-02-01  1:06 ` Andrew Benson
  2020-02-09 22:59   ` Andrew Benson
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Benson @ 2020-02-01  1:06 UTC (permalink / raw)
  To: Fortran List; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4031 bytes --]

I've attached on updated patch for PR83113. The now removes the ICE when a 
duplicate DIMENSION attribute is specified in a submodule function.  The patch 
reg tests cleanly.

OK to commit?

Note that this patch doesn't check that the attributes of duplicate 
declarations in a submodule function are valid or consistent with those 
declared in the module. For example both:

module mm
  implicit none
  interface
     module function c()
       integer, dimension(2)  :: c !! Dimension 2 here
     end function c
  end interface
end module mm

submodule (mm) oo
  implicit none
contains
  module function c()
    integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
  end function c
end submodule oo


and


module mm
  implicit none
  interface
     module function c()
       integer, dimension(2)  :: c !! Dimension 2 here
     end function c
  end interface
end module mm

submodule (mm) oo
  implicit none
contains
  module function c()
    integer, dimension(:)  :: c !! Invalid cannot have a deferred shape
  end function c
end submodule oo


compile successfully, but should be rejected. Presumably we should add some 
check that the attributes of the declaration in the submodule match those in 
the module?

If so, I can go ahead and open a PR for this problem.

-Andrew

On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> Below is a partial patch for PR 83113
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> 
> It simply circumvents the check for duplicate attributes when looking at
> declarations in a module procedure during compilation of a submodule.
> 
> As it stands, the patch handles the cases of "allocatable", "dimension", and
> "pointer" attributes (corresponding to the two test cases in the PR plus a
> case that showed up in my own code). There are other attributes which
> should be handled similarly but before I make those changes I wanted to
> seek some opinions on whether this is the correct/best way to fix this
> issue.
> 
> The patch regression tests cleanly - however,  while the patch solves the
> "duplicate attribute" problem for the test case in comment #4:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> 
> that test case then ICEs with:
> 
> $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> f951: internal compiler error: in gfc_set_array_spec, at fortran/array.c:879
> 0x5fd22f gfc_set_array_spec(gfc_symbol*, gfc_array_spec*, locus*)
>         ../../gcc-git/gcc/fortran/array.c:879
> 0x7e868e build_sym
>         ../../gcc-git/gcc/fortran/decl.c:1670
> 0x7f0ada variable_decl
>         ../../gcc-git/gcc/fortran/decl.c:2760
> 0x7f0ada gfc_match_data_decl()
>         ../../gcc-git/gcc/fortran/decl.c:6120
> 0x85b66d match_word
>         ../../gcc-git/gcc/fortran/parse.c:65
> 0x85b66d decode_statement
>         ../../gcc-git/gcc/fortran/parse.c:376
> 0x861094 next_free
>         ../../gcc-git/gcc/fortran/parse.c:1279
> 0x861094 next_statement
>         ../../gcc-git/gcc/fortran/parse.c:1511
> 0x8638ac parse_spec
>         ../../gcc-git/gcc/fortran/parse.c:3738
> 0x86591c parse_progunit
>         ../../gcc-git/gcc/fortran/parse.c:5851
> 0x865df9 parse_contained
>         ../../gcc-git/gcc/fortran/parse.c:5752
> 0x866b5e parse_module
>         ../../gcc-git/gcc/fortran/parse.c:6124
> 0x86709c gfc_parse_file()
>         ../../gcc-git/gcc/fortran/parse.c:6427
> 0x8b756f gfc_be_parse_file
>         ../../gcc-git/gcc/fortran/f95-lang.c:210
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
> 
> 
> This looks like a separate problem to me, but looking quickly at where the
> ICE occurs it seems to be related to coarrays of which my understanding is
> very limited - so any insights on this would be welcome.
> 
> Patch is appended below.
> 
> -Andrew


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 3312 bytes --]

diff --git a/gcc/fortran/array.c b/gcc/fortran/array.c
index c873cf2..ff1ff72 100644
--- a/gcc/fortran/array.c
+++ b/gcc/fortran/array.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "options.h"
 #include "gfortran.h"
+#include "parse.h"
 #include "match.h"
 #include "constructor.h"
 
@@ -835,6 +836,13 @@ gfc_set_array_spec (gfc_symbol *sym, gfc_array_spec *as, locus *error_loc)
   if (as == NULL)
     return true;
 
+  /* If the symbol corresponds to a submodule module procedure the array spec is
+     already set, so do not attempt to set it again here. */
+  if (gfc_state_stack->previous && gfc_state_stack->previous->previous
+      && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
+      && sym->attr.module_procedure)
+    return true;
+  
   if (as->rank
       && !gfc_add_dimension (&sym->attr, sym->name, error_loc))
     return false;
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 96c0fc1..4515de2 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -1014,7 +1014,10 @@ gfc_add_allocatable (symbol_attribute *attr, locus *where)
   if (check_used (attr, NULL, where))
     return false;
 
-  if (attr->allocatable)
+  if (attr->allocatable 
+      && !(gfc_state_stack->previous && gfc_state_stack->previous->previous
+	   && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
+	   && attr->module_procedure))
     {
       duplicate_attr ("ALLOCATABLE", where);
       return false;
@@ -1081,7 +1084,10 @@ gfc_add_dimension (symbol_attribute *attr, const char *name, locus *where)
   if (check_used (attr, name, where))
     return false;
 
-  if (attr->dimension)
+  if (attr->dimension
+      && !(gfc_state_stack->previous && gfc_state_stack->previous->previous
+	   && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
+	   && attr->module_procedure))
     {
       duplicate_attr ("DIMENSION", where);
       return false;
@@ -1208,7 +1214,10 @@ gfc_add_pointer (symbol_attribute *attr, locus *where)
     return false;
 
   if (attr->pointer && !(attr->if_source == IFSRC_IFBODY
-      && !gfc_find_state (COMP_INTERFACE)))
+      && !gfc_find_state (COMP_INTERFACE))
+      && !(gfc_state_stack->previous && gfc_state_stack->previous->previous
+	   && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
+	   && attr->module_procedure))
     {
       duplicate_attr ("POINTER", where);
       return false;
diff --git a/gcc/testsuite/gfortran.dg/pr83113.f90 b/gcc/testsuite/gfortran.dg/pr83113.f90
new file mode 100644
index 0000000..9192f1e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr83113.f90
@@ -0,0 +1,30 @@
+! { dg-do compile }
+! PR fortran/83113
+module mm
+  implicit none
+  interface
+     module function a()
+       integer, allocatable  :: a
+     end function a
+     module function b()
+       integer, pointer  :: b
+     end function b
+     module function c()
+       integer, dimension(2)  :: c
+     end function c
+  end interface
+end module mm
+
+submodule (mm) oo
+  implicit none
+contains
+  module function a()
+    integer, allocatable  :: a
+  end function a
+  module function b()
+    integer, pointer :: b
+  end function b
+  module function c()
+    integer, dimension(2)  :: c
+  end function c
+end submodule oo

[-- Attachment #3: ChangeLog --]
[-- Type: text/x-changelog, Size: 392 bytes --]

2020-01-31  Andrew Benson  <abensonca@gmail.com>

	PR fortran/83113
	* array.c: Do not attempt to set the array spec for a submodule
	function symbol (as it has already been set in the corresponding
	module procedure interface).
	* symbol.c: Do not reject duplicate POINTER, ALLOCATABLE, or
	DIMENSION attributes in declarations of a submodule function.
	* gfortran.dg/pr83113.f90: New test.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function
  2020-02-01  1:06 ` patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function Andrew Benson
@ 2020-02-09 22:59   ` Andrew Benson
  2020-02-10  0:32     ` Steve Kargl
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Benson @ 2020-02-09 22:59 UTC (permalink / raw)
  To: Fortran List; +Cc: gcc-patches

*ping*

On Friday, January 31, 2020 5:06:49 PM PST Andrew Benson wrote:
> I've attached on updated patch for PR83113. The now removes the ICE when a
> duplicate DIMENSION attribute is specified in a submodule function.  The
> patch reg tests cleanly.
> 
> OK to commit?
> 
> Note that this patch doesn't check that the attributes of duplicate
> declarations in a submodule function are valid or consistent with those
> declared in the module. For example both:
> 
> module mm
>   implicit none
>   interface
>      module function c()
>        integer, dimension(2)  :: c !! Dimension 2 here
>      end function c
>   end interface
> end module mm
> 
> submodule (mm) oo
>   implicit none
> contains
>   module function c()
>     integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
>   end function c
> end submodule oo
> 
> 
> and
> 
> 
> module mm
>   implicit none
>   interface
>      module function c()
>        integer, dimension(2)  :: c !! Dimension 2 here
>      end function c
>   end interface
> end module mm
> 
> submodule (mm) oo
>   implicit none
> contains
>   module function c()
>     integer, dimension(:)  :: c !! Invalid cannot have a deferred shape
>   end function c
> end submodule oo
> 
> 
> compile successfully, but should be rejected. Presumably we should add some
> check that the attributes of the declaration in the submodule match those in
> the module?
> 
> If so, I can go ahead and open a PR for this problem.
> 
> -Andrew
> 
> On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> > Below is a partial patch for PR 83113
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> > 
> > It simply circumvents the check for duplicate attributes when looking at
> > declarations in a module procedure during compilation of a submodule.
> > 
> > As it stands, the patch handles the cases of "allocatable", "dimension",
> > and "pointer" attributes (corresponding to the two test cases in the PR
> > plus a case that showed up in my own code). There are other attributes
> > which should be handled similarly but before I make those changes I
> > wanted to seek some opinions on whether this is the correct/best way to
> > fix this issue.
> > 
> > The patch regression tests cleanly - however,  while the patch solves the
> > "duplicate attribute" problem for the test case in comment #4:
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> > 
> > that test case then ICEs with:
> > 
> > $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> > f951: internal compiler error: in gfc_set_array_spec, at
> > fortran/array.c:879 0x5fd22f gfc_set_array_spec(gfc_symbol*,
> > gfc_array_spec*, locus*)
> > 
> >         ../../gcc-git/gcc/fortran/array.c:879
> > 
> > 0x7e868e build_sym
> > 
> >         ../../gcc-git/gcc/fortran/decl.c:1670
> > 
> > 0x7f0ada variable_decl
> > 
> >         ../../gcc-git/gcc/fortran/decl.c:2760
> > 
> > 0x7f0ada gfc_match_data_decl()
> > 
> >         ../../gcc-git/gcc/fortran/decl.c:6120
> > 
> > 0x85b66d match_word
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:65
> > 
> > 0x85b66d decode_statement
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:376
> > 
> > 0x861094 next_free
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:1279
> > 
> > 0x861094 next_statement
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:1511
> > 
> > 0x8638ac parse_spec
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:3738
> > 
> > 0x86591c parse_progunit
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:5851
> > 
> > 0x865df9 parse_contained
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:5752
> > 
> > 0x866b5e parse_module
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:6124
> > 
> > 0x86709c gfc_parse_file()
> > 
> >         ../../gcc-git/gcc/fortran/parse.c:6427
> > 
> > 0x8b756f gfc_be_parse_file
> > 
> >         ../../gcc-git/gcc/fortran/f95-lang.c:210
> > 
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > Please include the complete backtrace with any bug report.
> > See <https://gcc.gnu.org/bugs/> for instructions.
> > 
> > 
> > This looks like a separate problem to me, but looking quickly at where the
> > ICE occurs it seems to be related to coarrays of which my understanding is
> > very limited - so any insights on this would be welcome.
> > 
> > Patch is appended below.
> > 
> > -Andrew


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function
  2020-02-09 22:59   ` Andrew Benson
@ 2020-02-10  0:32     ` Steve Kargl
  2020-02-10  1:01       ` Andrew Benson
  2020-02-10 18:02       ` Andrew Benson
  0 siblings, 2 replies; 8+ messages in thread
From: Steve Kargl @ 2020-02-10  0:32 UTC (permalink / raw)
  To: Andrew Benson; +Cc: Fortran List, gcc-patches

Patch looks to me.  OK to commit.

I'm wondering if we need a macro or helper function to
simplify in conditional from

+  if (gfc_state_stack->previous && gfc_state_stack->previous->previous
+      && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
+      && sym->attr.module_procedure)

to for example

+  if (IN_SUBMODULE (sym))

where is 

#define IN_SUBMODULE (x) \
   (gfc_state_stack->previous && gfc_state_stack->previous->previous \
    && gfc_state_stack->previous->previous->state == COMP_SUBMODULE \
    && sym->attr.module_procedure)

-- 
steve

On Sun, Feb 09, 2020 at 02:59:52PM -0800, Andrew Benson wrote:
> *ping*
> 
> On Friday, January 31, 2020 5:06:49 PM PST Andrew Benson wrote:
> > I've attached on updated patch for PR83113. The now removes the ICE when a
> > duplicate DIMENSION attribute is specified in a submodule function.  The
> > patch reg tests cleanly.
> > 
> > OK to commit?
> > 
> > Note that this patch doesn't check that the attributes of duplicate
> > declarations in a submodule function are valid or consistent with those
> > declared in the module. For example both:
> > 
> > module mm
> >   implicit none
> >   interface
> >      module function c()
> >        integer, dimension(2)  :: c !! Dimension 2 here
> >      end function c
> >   end interface
> > end module mm
> > 
> > submodule (mm) oo
> >   implicit none
> > contains
> >   module function c()
> >     integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
> >   end function c
> > end submodule oo
> > 
> > 
> > and
> > 
> > 
> > module mm
> >   implicit none
> >   interface
> >      module function c()
> >        integer, dimension(2)  :: c !! Dimension 2 here
> >      end function c
> >   end interface
> > end module mm
> > 
> > submodule (mm) oo
> >   implicit none
> > contains
> >   module function c()
> >     integer, dimension(:)  :: c !! Invalid cannot have a deferred shape
> >   end function c
> > end submodule oo
> > 
> > 
> > compile successfully, but should be rejected. Presumably we should add some
> > check that the attributes of the declaration in the submodule match those in
> > the module?
> > 
> > If so, I can go ahead and open a PR for this problem.
> > 
> > -Andrew
> > 
> > On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> > > Below is a partial patch for PR 83113
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> > > 
> > > It simply circumvents the check for duplicate attributes when looking at
> > > declarations in a module procedure during compilation of a submodule.
> > > 
> > > As it stands, the patch handles the cases of "allocatable", "dimension",
> > > and "pointer" attributes (corresponding to the two test cases in the PR
> > > plus a case that showed up in my own code). There are other attributes
> > > which should be handled similarly but before I make those changes I
> > > wanted to seek some opinions on whether this is the correct/best way to
> > > fix this issue.
> > > 
> > > The patch regression tests cleanly - however,  while the patch solves the
> > > "duplicate attribute" problem for the test case in comment #4:
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> > > 
> > > that test case then ICEs with:
> > > 
> > > $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> > > f951: internal compiler error: in gfc_set_array_spec, at
> > > fortran/array.c:879 0x5fd22f gfc_set_array_spec(gfc_symbol*,
> > > gfc_array_spec*, locus*)
> > > 
> > >         ../../gcc-git/gcc/fortran/array.c:879
> > > 
> > > 0x7e868e build_sym
> > > 
> > >         ../../gcc-git/gcc/fortran/decl.c:1670
> > > 
> > > 0x7f0ada variable_decl
> > > 
> > >         ../../gcc-git/gcc/fortran/decl.c:2760
> > > 
> > > 0x7f0ada gfc_match_data_decl()
> > > 
> > >         ../../gcc-git/gcc/fortran/decl.c:6120
> > > 
> > > 0x85b66d match_word
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:65
> > > 
> > > 0x85b66d decode_statement
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:376
> > > 
> > > 0x861094 next_free
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:1279
> > > 
> > > 0x861094 next_statement
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:1511
> > > 
> > > 0x8638ac parse_spec
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:3738
> > > 
> > > 0x86591c parse_progunit
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:5851
> > > 
> > > 0x865df9 parse_contained
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:5752
> > > 
> > > 0x866b5e parse_module
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:6124
> > > 
> > > 0x86709c gfc_parse_file()
> > > 
> > >         ../../gcc-git/gcc/fortran/parse.c:6427
> > > 
> > > 0x8b756f gfc_be_parse_file
> > > 
> > >         ../../gcc-git/gcc/fortran/f95-lang.c:210
> > > 
> > > Please submit a full bug report,
> > > with preprocessed source if appropriate.
> > > Please include the complete backtrace with any bug report.
> > > See <https://gcc.gnu.org/bugs/> for instructions.
> > > 
> > > 
> > > This looks like a separate problem to me, but looking quickly at where the
> > > ICE occurs it seems to be related to coarrays of which my understanding is
> > > very limited - so any insights on this would be welcome.
> > > 
> > > Patch is appended below.
> > > 
> > > -Andrew
> 
> 
> -- 
> 
> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
> 
> * Galacticus: https://github.com/galacticusorg/galacticus

-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function
  2020-02-10  0:32     ` Steve Kargl
@ 2020-02-10  1:01       ` Andrew Benson
  2020-02-10 18:02       ` Andrew Benson
  1 sibling, 0 replies; 8+ messages in thread
From: Andrew Benson @ 2020-02-10  1:01 UTC (permalink / raw)
  To: sgk; +Cc: Fortran List, gcc-patches, Andrew Benson

Hi Steve,

Thanks for the review. Adding a macro as you suggest seems like a very good
idea. I'll make that change tomorrow before committing this.

Thanks,
Andrew


--

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

On Sun, Feb 9, 2020, 4:32 PM Steve Kargl <sgk@troutmask.apl.washington.edu>
wrote:

> Patch looks to me.  OK to commit.
>
> I'm wondering if we need a macro or helper function to
> simplify in conditional from
>
> +  if (gfc_state_stack->previous && gfc_state_stack->previous->previous
> +      && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
> +      && sym->attr.module_procedure)
>
> to for example
>
> +  if (IN_SUBMODULE (sym))
>
> where is
>
> #define IN_SUBMODULE (x) \
>    (gfc_state_stack->previous && gfc_state_stack->previous->previous \
>     && gfc_state_stack->previous->previous->state == COMP_SUBMODULE \
>     && sym->attr.module_procedure)
>
> --
> steve
>
> On Sun, Feb 09, 2020 at 02:59:52PM -0800, Andrew Benson wrote:
> > *ping*
> >
> > On Friday, January 31, 2020 5:06:49 PM PST Andrew Benson wrote:
> > > I've attached on updated patch for PR83113. The now removes the ICE
> when a
> > > duplicate DIMENSION attribute is specified in a submodule function.
> The
> > > patch reg tests cleanly.
> > >
> > > OK to commit?
> > >
> > > Note that this patch doesn't check that the attributes of duplicate
> > > declarations in a submodule function are valid or consistent with those
> > > declared in the module. For example both:
> > >
> > > module mm
> > >   implicit none
> > >   interface
> > >      module function c()
> > >        integer, dimension(2)  :: c !! Dimension 2 here
> > >      end function c
> > >   end interface
> > > end module mm
> > >
> > > submodule (mm) oo
> > >   implicit none
> > > contains
> > >   module function c()
> > >     integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
> > >   end function c
> > > end submodule oo
> > >
> > >
> > > and
> > >
> > >
> > > module mm
> > >   implicit none
> > >   interface
> > >      module function c()
> > >        integer, dimension(2)  :: c !! Dimension 2 here
> > >      end function c
> > >   end interface
> > > end module mm
> > >
> > > submodule (mm) oo
> > >   implicit none
> > > contains
> > >   module function c()
> > >     integer, dimension(:)  :: c !! Invalid cannot have a deferred shape
> > >   end function c
> > > end submodule oo
> > >
> > >
> > > compile successfully, but should be rejected. Presumably we should add
> some
> > > check that the attributes of the declaration in the submodule match
> those in
> > > the module?
> > >
> > > If so, I can go ahead and open a PR for this problem.
> > >
> > > -Andrew
> > >
> > > On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> > > > Below is a partial patch for PR 83113
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> > > >
> > > > It simply circumvents the check for duplicate attributes when
> looking at
> > > > declarations in a module procedure during compilation of a submodule.
> > > >
> > > > As it stands, the patch handles the cases of "allocatable",
> "dimension",
> > > > and "pointer" attributes (corresponding to the two test cases in the
> PR
> > > > plus a case that showed up in my own code). There are other
> attributes
> > > > which should be handled similarly but before I make those changes I
> > > > wanted to seek some opinions on whether this is the correct/best way
> to
> > > > fix this issue.
> > > >
> > > > The patch regression tests cleanly - however,  while the patch
> solves the
> > > > "duplicate attribute" problem for the test case in comment #4:
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> > > >
> > > > that test case then ICEs with:
> > > >
> > > > $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> > > > f951: internal compiler error: in gfc_set_array_spec, at
> > > > fortran/array.c:879 0x5fd22f gfc_set_array_spec(gfc_symbol*,
> > > > gfc_array_spec*, locus*)
> > > >
> > > >         ../../gcc-git/gcc/fortran/array.c:879
> > > >
> > > > 0x7e868e build_sym
> > > >
> > > >         ../../gcc-git/gcc/fortran/decl.c:1670
> > > >
> > > > 0x7f0ada variable_decl
> > > >
> > > >         ../../gcc-git/gcc/fortran/decl.c:2760
> > > >
> > > > 0x7f0ada gfc_match_data_decl()
> > > >
> > > >         ../../gcc-git/gcc/fortran/decl.c:6120
> > > >
> > > > 0x85b66d match_word
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:65
> > > >
> > > > 0x85b66d decode_statement
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:376
> > > >
> > > > 0x861094 next_free
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:1279
> > > >
> > > > 0x861094 next_statement
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:1511
> > > >
> > > > 0x8638ac parse_spec
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:3738
> > > >
> > > > 0x86591c parse_progunit
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:5851
> > > >
> > > > 0x865df9 parse_contained
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:5752
> > > >
> > > > 0x866b5e parse_module
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:6124
> > > >
> > > > 0x86709c gfc_parse_file()
> > > >
> > > >         ../../gcc-git/gcc/fortran/parse.c:6427
> > > >
> > > > 0x8b756f gfc_be_parse_file
> > > >
> > > >         ../../gcc-git/gcc/fortran/f95-lang.c:210
> > > >
> > > > Please submit a full bug report,
> > > > with preprocessed source if appropriate.
> > > > Please include the complete backtrace with any bug report.
> > > > See <https://gcc.gnu.org/bugs/> for instructions.
> > > >
> > > >
> > > > This looks like a separate problem to me, but looking quickly at
> where the
> > > > ICE occurs it seems to be related to coarrays of which my
> understanding is
> > > > very limited - so any insights on this would be welcome.
> > > >
> > > > Patch is appended below.
> > > >
> > > > -Andrew
> >
> >
> > --
> >
> > * Andrew Benson:
> http://users.obs.carnegiescience.edu/abenson/contact.html
> >
> > * Galacticus: https://github.com/galacticusorg/galacticus
>
> --
> Steve
> 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
> 20161221 https://www.youtube.com/watch?v=IbCHE-hONow
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function
  2020-02-10  0:32     ` Steve Kargl
  2020-02-10  1:01       ` Andrew Benson
@ 2020-02-10 18:02       ` Andrew Benson
  2020-02-10 18:05         ` Andrew Benson
  1 sibling, 1 reply; 8+ messages in thread
From: Andrew Benson @ 2020-02-10 18:02 UTC (permalink / raw)
  To: Fortran List; +Cc: sgk, gcc-patches

This is now committed (after adding the macro as Steve suggested):

https://gcc.gnu.org/g:7848054c68bad6e2aa40cb59f77cc99bd8448d52

On Sunday, February 9, 2020 4:32:26 PM PST Steve Kargl wrote:
> Patch looks to me.  OK to commit.
> 
> I'm wondering if we need a macro or helper function to
> simplify in conditional from
> 
> +  if (gfc_state_stack->previous && gfc_state_stack->previous->previous
> +      && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
> +      && sym->attr.module_procedure)
> 
> to for example
> 
> +  if (IN_SUBMODULE (sym))
> 
> where is
> 
> #define IN_SUBMODULE (x) \
>    (gfc_state_stack->previous && gfc_state_stack->previous->previous \
>     && gfc_state_stack->previous->previous->state == COMP_SUBMODULE \
>     && sym->attr.module_procedure)
> 
> > *ping*
> > 
> > On Friday, January 31, 2020 5:06:49 PM PST Andrew Benson wrote:
> > > I've attached on updated patch for PR83113. The now removes the ICE when
> > > a
> > > duplicate DIMENSION attribute is specified in a submodule function.  The
> > > patch reg tests cleanly.
> > > 
> > > OK to commit?
> > > 
> > > Note that this patch doesn't check that the attributes of duplicate
> > > declarations in a submodule function are valid or consistent with those
> > > declared in the module. For example both:
> > > 
> > > module mm
> > > 
> > >   implicit none
> > >   interface
> > >   
> > >      module function c()
> > >      
> > >        integer, dimension(2)  :: c !! Dimension 2 here
> > >      
> > >      end function c
> > >   
> > >   end interface
> > > 
> > > end module mm
> > > 
> > > submodule (mm) oo
> > > 
> > >   implicit none
> > > 
> > > contains
> > > 
> > >   module function c()
> > >   
> > >     integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
> > >   
> > >   end function c
> > > 
> > > end submodule oo
> > > 
> > > 
> > > and
> > > 
> > > 
> > > module mm
> > > 
> > >   implicit none
> > >   interface
> > >   
> > >      module function c()
> > >      
> > >        integer, dimension(2)  :: c !! Dimension 2 here
> > >      
> > >      end function c
> > >   
> > >   end interface
> > > 
> > > end module mm
> > > 
> > > submodule (mm) oo
> > > 
> > >   implicit none
> > > 
> > > contains
> > > 
> > >   module function c()
> > >   
> > >     integer, dimension(:)  :: c !! Invalid cannot have a deferred shape
> > >   
> > >   end function c
> > > 
> > > end submodule oo
> > > 
> > > 
> > > compile successfully, but should be rejected. Presumably we should add
> > > some
> > > check that the attributes of the declaration in the submodule match
> > > those in the module?
> > > 
> > > If so, I can go ahead and open a PR for this problem.
> > > 
> > > -Andrew
> > > 
> > > On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> > > > Below is a partial patch for PR 83113
> > > > 
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> > > > 
> > > > It simply circumvents the check for duplicate attributes when looking
> > > > at
> > > > declarations in a module procedure during compilation of a submodule.
> > > > 
> > > > As it stands, the patch handles the cases of "allocatable",
> > > > "dimension",
> > > > and "pointer" attributes (corresponding to the two test cases in the
> > > > PR
> > > > plus a case that showed up in my own code). There are other attributes
> > > > which should be handled similarly but before I make those changes I
> > > > wanted to seek some opinions on whether this is the correct/best way
> > > > to
> > > > fix this issue.
> > > > 
> > > > The patch regression tests cleanly - however,  while the patch solves
> > > > the
> > > > "duplicate attribute" problem for the test case in comment #4:
> > > > 
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> > > > 
> > > > that test case then ICEs with:
> > > > 
> > > > $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> > > > f951: internal compiler error: in gfc_set_array_spec, at
> > > > fortran/array.c:879 0x5fd22f gfc_set_array_spec(gfc_symbol*,
> > > > gfc_array_spec*, locus*)
> > > > 
> > > >         ../../gcc-git/gcc/fortran/array.c:879
> > > > 
> > > > 0x7e868e build_sym
> > > > 
> > > >         ../../gcc-git/gcc/fortran/decl.c:1670
> > > > 
> > > > 0x7f0ada variable_decl
> > > > 
> > > >         ../../gcc-git/gcc/fortran/decl.c:2760
> > > > 
> > > > 0x7f0ada gfc_match_data_decl()
> > > > 
> > > >         ../../gcc-git/gcc/fortran/decl.c:6120
> > > > 
> > > > 0x85b66d match_word
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:65
> > > > 
> > > > 0x85b66d decode_statement
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:376
> > > > 
> > > > 0x861094 next_free
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:1279
> > > > 
> > > > 0x861094 next_statement
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:1511
> > > > 
> > > > 0x8638ac parse_spec
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:3738
> > > > 
> > > > 0x86591c parse_progunit
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:5851
> > > > 
> > > > 0x865df9 parse_contained
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:5752
> > > > 
> > > > 0x866b5e parse_module
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:6124
> > > > 
> > > > 0x86709c gfc_parse_file()
> > > > 
> > > >         ../../gcc-git/gcc/fortran/parse.c:6427
> > > > 
> > > > 0x8b756f gfc_be_parse_file
> > > > 
> > > >         ../../gcc-git/gcc/fortran/f95-lang.c:210
> > > > 
> > > > Please submit a full bug report,
> > > > with preprocessed source if appropriate.
> > > > Please include the complete backtrace with any bug report.
> > > > See <https://gcc.gnu.org/bugs/> for instructions.
> > > > 
> > > > 
> > > > This looks like a separate problem to me, but looking quickly at where
> > > > the
> > > > ICE occurs it seems to be related to coarrays of which my
> > > > understanding is
> > > > very limited - so any insights on this would be welcome.
> > > > 
> > > > Patch is appended below.
> > > > 
> > > > -Andrew


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function
  2020-02-10 18:02       ` Andrew Benson
@ 2020-02-10 18:05         ` Andrew Benson
  2020-02-10 18:19           ` Segher Boessenkool
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Benson @ 2020-02-10 18:05 UTC (permalink / raw)
  To: Fortran List; +Cc: sgk, gcc-patches

I don't think I have the ability to mark the PR as resolved. Can someone do 
that?

On Monday, February 10, 2020 10:02:24 AM PST Andrew Benson wrote:
> This is now committed (after adding the macro as Steve suggested):
> 
> https://gcc.gnu.org/g:7848054c68bad6e2aa40cb59f77cc99bd8448d52
> 
> On Sunday, February 9, 2020 4:32:26 PM PST Steve Kargl wrote:
> > Patch looks to me.  OK to commit.
> > 
> > I'm wondering if we need a macro or helper function to
> > simplify in conditional from
> > 
> > +  if (gfc_state_stack->previous && gfc_state_stack->previous->previous
> > +      && gfc_state_stack->previous->previous->state == COMP_SUBMODULE
> > +      && sym->attr.module_procedure)
> > 
> > to for example
> > 
> > +  if (IN_SUBMODULE (sym))
> > 
> > where is
> > 
> > #define IN_SUBMODULE (x) \
> > 
> >    (gfc_state_stack->previous && gfc_state_stack->previous->previous \
> >    
> >     && gfc_state_stack->previous->previous->state == COMP_SUBMODULE \
> >     && sym->attr.module_procedure)
> > > 
> > > *ping*
> > > 
> > > On Friday, January 31, 2020 5:06:49 PM PST Andrew Benson wrote:
> > > > I've attached on updated patch for PR83113. The now removes the ICE
> > > > when
> > > > a
> > > > duplicate DIMENSION attribute is specified in a submodule function. 
> > > > The
> > > > patch reg tests cleanly.
> > > > 
> > > > OK to commit?
> > > > 
> > > > Note that this patch doesn't check that the attributes of duplicate
> > > > declarations in a submodule function are valid or consistent with
> > > > those
> > > > declared in the module. For example both:
> > > > 
> > > > module mm
> > > > 
> > > >   implicit none
> > > >   interface
> > > >   
> > > >      module function c()
> > > >      
> > > >        integer, dimension(2)  :: c !! Dimension 2 here
> > > >      
> > > >      end function c
> > > >   
> > > >   end interface
> > > > 
> > > > end module mm
> > > > 
> > > > submodule (mm) oo
> > > > 
> > > >   implicit none
> > > > 
> > > > contains
> > > > 
> > > >   module function c()
> > > >   
> > > >     integer, dimension(3)  :: c  !! Inconsistent dimension 3 here
> > > >   
> > > >   end function c
> > > > 
> > > > end submodule oo
> > > > 
> > > > 
> > > > and
> > > > 
> > > > 
> > > > module mm
> > > > 
> > > >   implicit none
> > > >   interface
> > > >   
> > > >      module function c()
> > > >      
> > > >        integer, dimension(2)  :: c !! Dimension 2 here
> > > >      
> > > >      end function c
> > > >   
> > > >   end interface
> > > > 
> > > > end module mm
> > > > 
> > > > submodule (mm) oo
> > > > 
> > > >   implicit none
> > > > 
> > > > contains
> > > > 
> > > >   module function c()
> > > >   
> > > >     integer, dimension(:)  :: c !! Invalid cannot have a deferred
> > > >     shape
> > > >   
> > > >   end function c
> > > > 
> > > > end submodule oo
> > > > 
> > > > 
> > > > compile successfully, but should be rejected. Presumably we should add
> > > > some
> > > > check that the attributes of the declaration in the submodule match
> > > > those in the module?
> > > > 
> > > > If so, I can go ahead and open a PR for this problem.
> > > > 
> > > > -Andrew
> > > > 
> > > > On Friday, January 24, 2020 10:54:24 AM PST Andrew Benson wrote:
> > > > > Below is a partial patch for PR 83113
> > > > > 
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113
> > > > > 
> > > > > It simply circumvents the check for duplicate attributes when
> > > > > looking
> > > > > at
> > > > > declarations in a module procedure during compilation of a
> > > > > submodule.
> > > > > 
> > > > > As it stands, the patch handles the cases of "allocatable",
> > > > > "dimension",
> > > > > and "pointer" attributes (corresponding to the two test cases in the
> > > > > PR
> > > > > plus a case that showed up in my own code). There are other
> > > > > attributes
> > > > > which should be handled similarly but before I make those changes I
> > > > > wanted to seek some opinions on whether this is the correct/best way
> > > > > to
> > > > > fix this issue.
> > > > > 
> > > > > The patch regression tests cleanly - however,  while the patch
> > > > > solves
> > > > > the
> > > > > "duplicate attribute" problem for the test case in comment #4:
> > > > > 
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83113#c4
> > > > > 
> > > > > that test case then ICEs with:
> > > > > 
> > > > > $ gfortran -c pr83113_2.F90 -o pr83113_2.o
> > > > > f951: internal compiler error: in gfc_set_array_spec, at
> > > > > fortran/array.c:879 0x5fd22f gfc_set_array_spec(gfc_symbol*,
> > > > > gfc_array_spec*, locus*)
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/array.c:879
> > > > > 
> > > > > 0x7e868e build_sym
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/decl.c:1670
> > > > > 
> > > > > 0x7f0ada variable_decl
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/decl.c:2760
> > > > > 
> > > > > 0x7f0ada gfc_match_data_decl()
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/decl.c:6120
> > > > > 
> > > > > 0x85b66d match_word
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:65
> > > > > 
> > > > > 0x85b66d decode_statement
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:376
> > > > > 
> > > > > 0x861094 next_free
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:1279
> > > > > 
> > > > > 0x861094 next_statement
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:1511
> > > > > 
> > > > > 0x8638ac parse_spec
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:3738
> > > > > 
> > > > > 0x86591c parse_progunit
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:5851
> > > > > 
> > > > > 0x865df9 parse_contained
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:5752
> > > > > 
> > > > > 0x866b5e parse_module
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:6124
> > > > > 
> > > > > 0x86709c gfc_parse_file()
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/parse.c:6427
> > > > > 
> > > > > 0x8b756f gfc_be_parse_file
> > > > > 
> > > > >         ../../gcc-git/gcc/fortran/f95-lang.c:210
> > > > > 
> > > > > Please submit a full bug report,
> > > > > with preprocessed source if appropriate.
> > > > > Please include the complete backtrace with any bug report.
> > > > > See <https://gcc.gnu.org/bugs/> for instructions.
> > > > > 
> > > > > 
> > > > > This looks like a separate problem to me, but looking quickly at
> > > > > where
> > > > > the
> > > > > ICE occurs it seems to be related to coarrays of which my
> > > > > understanding is
> > > > > very limited - so any insights on this would be welcome.
> > > > > 
> > > > > Patch is appended below.
> > > > > 
> > > > > -Andrew


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function
  2020-02-10 18:05         ` Andrew Benson
@ 2020-02-10 18:19           ` Segher Boessenkool
  2020-02-10 18:52             ` Andrew Benson
  0 siblings, 1 reply; 8+ messages in thread
From: Segher Boessenkool @ 2020-02-10 18:19 UTC (permalink / raw)
  To: Andrew Benson; +Cc: Fortran List, sgk, gcc-patches

On Mon, Feb 10, 2020 at 10:05:48AM -0800, Andrew Benson wrote:
> I don't think I have the ability to mark the PR as resolved. Can someone do 
> that?

You have an @gcc.gnu.org account; if you use that for your BZ account,
you will magically get everything you need here.


Segher

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function
  2020-02-10 18:19           ` Segher Boessenkool
@ 2020-02-10 18:52             ` Andrew Benson
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Benson @ 2020-02-10 18:52 UTC (permalink / raw)
  To: fortran; +Cc: Segher Boessenkool, sgk, gcc-patches

Thanks - that worked.

On Monday, February 10, 2020 12:18:57 PM PST Segher Boessenkool wrote:
> On Mon, Feb 10, 2020 at 10:05:48AM -0800, Andrew Benson wrote:
> > I don't think I have the ability to mark the PR as resolved. Can someone
> > do
> > that?
> 
> You have an @gcc.gnu.org account; if you use that for your BZ account,
> you will magically get everything you need here.
> 
> 
> Segher


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-02-10 18:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <3150885.ufrke4QsgI@andrew-precision-3520>
2020-02-01  1:06 ` patch, fortan] PR83113 Bogus "duplicate allocatable attribute" error for submodule character function Andrew Benson
2020-02-09 22:59   ` Andrew Benson
2020-02-10  0:32     ` Steve Kargl
2020-02-10  1:01       ` Andrew Benson
2020-02-10 18:02       ` Andrew Benson
2020-02-10 18:05         ` Andrew Benson
2020-02-10 18:19           ` Segher Boessenkool
2020-02-10 18:52             ` Andrew Benson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).