* [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 @ 2021-10-27 19:09 Harald Anlauf 2021-11-03 20:00 ` *PING* " Harald Anlauf 0 siblings, 1 reply; 7+ messages in thread From: Harald Anlauf @ 2021-10-27 19:09 UTC (permalink / raw) To: fortran, gcc-patches [-- Attachment #1: Type: text/plain, Size: 812 bytes --] 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: pr69419.diff --] [-- Type: text/x-patch, Size: 1262 bytes --] 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* *PING* [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 2021-10-27 19:09 [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 Harald Anlauf @ 2021-11-03 20:00 ` Harald Anlauf 2021-11-03 20:00 ` Harald Anlauf 2021-11-04 9:06 ` Bernhard Reutner-Fischer 0 siblings, 2 replies; 7+ messages in thread From: Harald Anlauf @ 2021-11-03 20:00 UTC (permalink / raw) To: fortran; +Cc: gcc-patches *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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* *PING* [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 2021-11-03 20:00 ` *PING* " Harald Anlauf @ 2021-11-03 20:00 ` Harald Anlauf 2021-11-04 9:06 ` Bernhard Reutner-Fischer 1 sibling, 0 replies; 7+ messages in thread From: Harald Anlauf @ 2021-11-03 20:00 UTC (permalink / raw) To: fortran, gcc-patches *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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: *PING* [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 2021-11-03 20:00 ` *PING* " Harald Anlauf 2021-11-03 20:00 ` Harald Anlauf @ 2021-11-04 9:06 ` Bernhard Reutner-Fischer 2021-11-04 19:49 ` Harald Anlauf 1 sibling, 1 reply; 7+ messages in thread From: Bernhard Reutner-Fischer @ 2021-11-04 9:06 UTC (permalink / raw) To: Harald Anlauf via Fortran; +Cc: rep.dot.nop, Harald Anlauf, gcc-patches 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! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: *PING* [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 2021-11-04 9:06 ` Bernhard Reutner-Fischer @ 2021-11-04 19:49 ` Harald Anlauf 2021-11-04 19:49 ` Harald Anlauf 2021-11-05 21:46 ` Mikael Morin 0 siblings, 2 replies; 7+ messages in thread From: Harald Anlauf @ 2021-11-04 19:49 UTC (permalink / raw) To: Bernhard Reutner-Fischer, Harald Anlauf via Fortran; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: *PING* [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 2021-11-04 19:49 ` Harald Anlauf @ 2021-11-04 19:49 ` Harald Anlauf 2021-11-05 21:46 ` Mikael Morin 1 sibling, 0 replies; 7+ messages in thread From: Harald Anlauf @ 2021-11-04 19:49 UTC (permalink / raw) To: fortran; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: *PING* [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 2021-11-04 19:49 ` Harald Anlauf 2021-11-04 19:49 ` Harald Anlauf @ 2021-11-05 21:46 ` Mikael Morin 1 sibling, 0 replies; 7+ messages in thread From: Mikael Morin @ 2021-11-05 21:46 UTC (permalink / raw) To: Harald Anlauf, fortran; +Cc: gcc-patches Le 04/11/2021 à 20:49, Harald Anlauf via Fortran a écrit : > Let's see what others think. > OK, thanks. Mikael ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-11-05 21:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-27 19:09 [PATCH] PR fortran/69419 - ICE: tree check: expected array_type, have real_type in gfc_conv_array_initializer, at fortran/trans-array.c:5618 Harald Anlauf 2021-11-03 20:00 ` *PING* " Harald Anlauf 2021-11-03 20:00 ` Harald Anlauf 2021-11-04 9:06 ` Bernhard Reutner-Fischer 2021-11-04 19:49 ` Harald Anlauf 2021-11-04 19:49 ` Harald Anlauf 2021-11-05 21:46 ` Mikael Morin
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).