Message ID | trinity-e29ae6f6-871e-4ad9-ba32-87892e503bb6-1635361763453@3c-app-gmx-bap04 |
---|---|
State | New |
Headers | show |
Series | PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 | expand |
*PING* Am 27.10.21 um 21:09 schrieb Harald Anlauf via Fortran: > Dear Fortranners, > > when debugging the testcase, I noticed that a coarray declaration in > a COMMON statement wrongly set the dimension attribute instead of the > codimension. As a consequence, subsequent checks that catch this > invalid situation would not trigger. > > I see two possible solutions: > > - in gfc_match_common, replace > > /* Deal with an optional array specification after the > symbol name. */ > m = gfc_match_array_spec (&as, true, true); > > by > > m = gfc_match_array_spec (&as, true, false); > > which in turn would lead to a syntax error. Interestingly, the Intel > compiler also takes this route and gives a syntax error. > > - check the resulting as->corank and emit an error as in the attached > patch. > > The attached patch regtests fine on x86_64-pc-linux-gnu. OK for mainline? > > Thanks, > Harald >
On Wed, 3 Nov 2021 21:00:41 +0100 Harald Anlauf via Fortran <fortran@gcc.gnu.org> wrote: > *PING* > > Am 27.10.21 um 21:09 schrieb Harald Anlauf via Fortran: > > Dear Fortranners, > > > > when debugging the testcase, I noticed that a coarray declaration in > > a COMMON statement wrongly set the dimension attribute instead of the > > codimension. As a consequence, subsequent checks that catch this > > invalid situation would not trigger. > > > > I see two possible solutions: > > > > - in gfc_match_common, replace > > > > /* Deal with an optional array specification after the > > symbol name. */ > > m = gfc_match_array_spec (&as, true, true); If coarrays are generally forbidden in commons then.. > > > > by > > > > m = gfc_match_array_spec (&as, true, false); .. this sounds right to me. > > > > which in turn would lead to a syntax error. Interestingly, the Intel > > compiler also takes this route and gives a syntax error. > > > > - check the resulting as->corank and emit an error as in the attached > > patch. If we want to be more helpful than a mere syntax error (and we should be) then yes. Otherwise we'd have to explicitly @@ -5275,9 +5275,19 @@ gfc_match_common (void) /* Deal with an optional array specification after the symbol name. */ - m = gfc_match_array_spec (&as, true, true); + m = gfc_match_array_spec (&as, true, false); if (m == MATCH_ERROR) goto cleanup; + if (m == MATCH_NO) + { + /* See if it is a coarray and diagnose it nicely. */ + if (gfc_match_array_spec (&as, false, true) == MATCH_YES) + { + gfc_error ("Symbol %qs in COMMON at %C cannot be a " + "coarray", sym->name); + goto cleanup; + } + } where your patch works equally well and is more concise. Maybe you want to add a test for the double-colon variant too? common /c2/ y[:] ! { dg-error "cannot be a coarray" } A type with a coarray seems to require to be allocatable so is rejected (properly, but not mentioning the coarray) with Error: Derived type variable ‘comm_ty1’ in COMMON at (1) has an ultimate component that is allocatable When reading gfc_match_array_spec i thought that it might have been cleaner to split the coarray handling out to a separate gfc_match_coarray_spec but that's what we have. > > > > The attached patch regtests fine on x86_64-pc-linux-gnu. OK for mainline? LGTM but i cannot approve it. Thanks for the patch!
Hi Bernhard, Am 04.11.21 um 10:06 schrieb Bernhard Reutner-Fischer via Fortran: > On Wed, 3 Nov 2021 21:00:41 +0100 > Harald Anlauf via Fortran <fortran@gcc.gnu.org> wrote: > >> *PING* >> >> Am 27.10.21 um 21:09 schrieb Harald Anlauf via Fortran: >>> Dear Fortranners, >>> >>> when debugging the testcase, I noticed that a coarray declaration in >>> a COMMON statement wrongly set the dimension attribute instead of the >>> codimension. As a consequence, subsequent checks that catch this >>> invalid situation would not trigger. >>> >>> I see two possible solutions: >>> >>> - in gfc_match_common, replace >>> >>> /* Deal with an optional array specification after the >>> symbol name. */ >>> m = gfc_match_array_spec (&as, true, true); > > If coarrays are generally forbidden in commons then.. F2018: (R874) A common-block-object shall not be a dummy argument, a function result, an allocatable variable, a derived-type object with an ultimate component that is allocatable, a procedure pointer, an automatic data object, a variable with the BIND attribute, an unlimited polymorphic pointer, or a coarray. >>> >>> by >>> >>> m = gfc_match_array_spec (&as, true, false); > > .. this sounds right to me. > >>> >>> which in turn would lead to a syntax error. Interestingly, the Intel >>> compiler also takes this route and gives a syntax error. >>> >>> - check the resulting as->corank and emit an error as in the attached >>> patch. > > If we want to be more helpful than a mere syntax error (and we > should be) then yes. > Otherwise we'd have to explicitly > @@ -5275,9 +5275,19 @@ gfc_match_common (void) > > /* Deal with an optional array specification after the > symbol name. */ > - m = gfc_match_array_spec (&as, true, true); > + m = gfc_match_array_spec (&as, true, false); > if (m == MATCH_ERROR) > goto cleanup; > + if (m == MATCH_NO) > + { > + /* See if it is a coarray and diagnose it nicely. */ I think you would need to add gfc_array_spec *as; to avoid clobbering the correct "as" as it is needed later. > + if (gfc_match_array_spec (&as, false, true) == MATCH_YES) > + { > + gfc_error ("Symbol %qs in COMMON at %C cannot be a " > + "coarray", sym->name); > + goto cleanup; > + } > + } > > where your patch works equally well and is more concise. > Maybe you want to add a test for the double-colon variant too? > common /c2/ y[:] ! { dg-error "cannot be a coarray" } Well, that one is already rejected, although with a different error message. I am not sure whether to add that case. In fact, there are issues with not always rejecting things like y[:] in declarations as the last dimension must be "*" or "lbound:*". If checking gets improved in this direction, we would have to adjust the error message. So adding this variant now does not buy us much. > A type with a coarray seems to require to be allocatable so is > rejected (properly, but not mentioning the coarray) with > Error: Derived type variable ‘comm_ty1’ in COMMON at (1) has an ultimate component that is allocatable If multiple errors show up, which ones are most important or must show up? If you ask me, it is more important to get correct results from correct input than optimal error messages on wrong code. I guess users may have an opinion different from mine... > When reading gfc_match_array_spec i thought that it might have been cleaner > to split the coarray handling out to a separate gfc_match_coarray_spec but > that's what we have. >>> >>> The attached patch regtests fine on x86_64-pc-linux-gnu. OK for mainline? > > LGTM but i cannot approve it. > Thanks for the patch! > Thanks for your comments so far. Let's see what others think. Harald
Le 04/11/2021 à 20:49, Harald Anlauf via Fortran a écrit : > Let's see what others think. > OK, thanks. Mikael
Fortran: a symbol in a COMMON cannot be a coarray gcc/fortran/ChangeLog: PR fortran/69419 * match.c (gfc_match_common): Check array spec of a symbol in a COMMON object list and reject it if it is a coarray. gcc/testsuite/ChangeLog: PR fortran/69419 * gfortran.dg/pr69419.f90: New test. diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c index 53a575e616e..df97620634d 100644 --- a/gcc/fortran/match.c +++ b/gcc/fortran/match.c @@ -5314,6 +5314,13 @@ gfc_match_common (void) goto cleanup; } + if (as->corank) + { + gfc_error ("Symbol %qs in COMMON at %C cannot be a " + "coarray", sym->name); + goto cleanup; + } + if (!gfc_add_dimension (&sym->attr, sym->name, NULL)) goto cleanup; diff --git a/gcc/testsuite/gfortran.dg/pr69419.f90 b/gcc/testsuite/gfortran.dg/pr69419.f90 new file mode 100644 index 00000000000..7329808611c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr69419.f90 @@ -0,0 +1,9 @@ +! { dg-do compile } +! { dg-options "-fcoarray=lib" } +! PR fortran/69419 - ICE on invalid coarray in common + +blockdata b + real x ! { dg-error "must be in COMMON" } + common /c/ x[*] ! { dg-error "cannot be a coarray" } + data x /1.0/ +end