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 <s...@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 >
