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