Fortran patches

[Steve Kargl]

I intend to commit the attached patch on Saturday.

2018-12-02 Steven G. Kargl <***@gcc.gnu.org>

PR fortran/87922
* io.c (gfc_match_open): ASYNCHRONOUS must be scalar.

PR fortran/87945
* decl.c (var_element): Inquiry parameter cannot be a data object.
(match_data_constant): Inquiry parameter can be a data
in a data statement.

PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

PR fortran/88025
* expr.c (gfc_apply_init): Remove asserts and check for valid
ts->u.cl->length.

PR fortran/88048
* resolve.c (check_data_variable): Convert gfc_internal_error to
an gfc_error. Add a nearby missing 'return false;'

PR fortran/88116
* simplify.c: Remove internal error and return gfc_bad_expr.

PR fortran/88205
* io.c (gfc_match_open): STATUS must be CHARACTER type.

PR fortran/88206
* match.c (gfc_match_type_spec): REAL can be an intrinsic function.

PR fortran/88228
* expr.c (check_null, check_elemental): Work around -fdec and
initialization with logical operators operating on integers.

PR fortran/88249
* gfortran.h: Update prototype for gfc_resolve_filepos
* io.c (gfc_resolve_filepos): Accept the locus to include in errors.
* resolve.c (gfc_resolve_code): Pass locus.

PR fortran/88269
* io.c (io_constraint): Update macro. Remove incompatible use
of io_constraint and give explicit error.

PR fortran/88328
* io.c (resolve_tag_format): Detect zero-sized array.

2018-12-02 Steven G. Kargl <***@gcc.gnu.org>

PR fortran/87922
* gfortran.dg/pr87922.f90: New test.

PR fortran/887945
* gfortran.dg/pr87945_1.f90: New test.
* gfortran.dg/pr87945_2.f90: New test.

PR fortran/87994
* gfortran.dg/pr87994_1.f90: New test.
* gfortran.dg/pr87994_2.f90: New test.
* gfortran.dg/pr87994_3.f90: New test.

PR fortran/88025
* gfortran.dg/pr88025.f90: New test.

PR fortran/88048
* gfortran.dg/pr88048.f90: New test.

PR fortran/88116
* gfortran.dg/pr88116_1.f90: New test.
* gfortran.dg/pr88116_2.f90: New test.

PR fortran/88139
* gfortran.dg/pr88139.f90: New test.

PR fortran/88205
* gfortran.dg/pr88205.f90: New test.

PR fortran/88206
* gfortran.dg/pr88206.f90: New test.

PR fortran/88228
* gfortran.dg/pr88228.f90: New test.

PR fortran/88249
* gfortran.dg/pr88249.f90: New test.

PR fortran/88269
* gfortran.dg/pr88269.f90: New test.

PR fortran/88328
* gfortran.dg/pr88328.f90: New test.

[Fritz Reese]

On Wed, Dec 5, 2018 at 12:00 AM Steve Kargl

Post by Steve Kargl
I intend to commit the attached patch on Saturday.

Thanks for the work. I assume the patch bootstraps and passes regression tests?

Post by Steve Kargl
PR fortran/88228
* expr.c (check_null, check_elemental): Work around -fdec and
initialization with logical operators operating on integers.

I plan to review this section of the patch later today -- though the
patch hides the segfault from the PR, I need more time to determine
whether it is correct and complete.

Post by Steve Kargl
PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

Post by Steve Kargl
PR fortran/88205
* io.c (gfc_match_open): STATUS must be CHARACTER type.

[...]

Post by Steve Kargl
@@ -2161,6 +2167,12 @@ gfc_match_open (void)
if (!open->file && open->status)
{
+ if (open->status->ts.type != BT_CHARACTER)
+ {
+ gfc_error ("STATUS must be a default character type at %C");
+ goto cleanup;
+ }
+
if (open->status->expr_type == EXPR_CONSTANT
&& gfc_wide_strncasecmp (open->status->value.character.string,
"scratch", 7) != 0)

Both resolve_tag() and is_char_type() actually already catch type
mismatches for STATUS (the latter for constant expressions). The real
problem is the following condition which checks STATUS before it has
been processed yet, since NEWUNIT is processed before STATUS. I think
the correct thing to do is actually to move the NEWUNIT/UNIT if-block
after the STATUS if-block, rather than adding a new phrasing for the
same error. Then we should see:

pr88205.f90:13:29:
open (newunit=n, status=status)
1
Error: STATUS requires a scalar-default-char-expr at (1)

Post by Steve Kargl
PR fortran/88328
* io.c (resolve_tag_format): Detect zero-sized array.

[...]

Post by Steve Kargl
Index: gcc/fortran/io.c
===================================================================
--- gcc/fortran/io.c (revision 266718)
+++ gcc/fortran/io.c (working copy)
@@ -1636,6 +1636,12 @@ resolve_tag_format (gfc_expr *e)
gfc_expr *r;
gfc_char_t *dest, *src;
+ if (e->value.constructor == NULL)
+ {
+ gfc_error ("FORMAT tag at %C cannot be a zero-sized array");
+ return false;
+ }
+
n = 0;
c = gfc_constructor_first (e->value.constructor);
len = c->expr->value.character.length;

[...]

Post by Steve Kargl
@ -3231,12 +3257,21 @@ gfc_resolve_dt (gfc_dt *dt, locus *loc)
{
gfc_expr *e;
io_kind k;
+ locus loc_tmp;
/* This is set in any case. */
gcc_assert (dt->dt_io_kind);
k = dt->dt_io_kind->value.iokind;
- RESOLVE_TAG (&tag_format, dt->format_expr);
+ loc_tmp = gfc_current_locus;
+ gfc_current_locus = *loc;
+ if (!resolve_tag (&tag_format, dt->format_expr))
+ {
+ gfc_current_locus = loc_tmp;
+ return false;
+ }
+ gfc_current_locus = loc_tmp;
+
RESOLVE_TAG (&tag_rec, dt->rec);
RESOLVE_TAG (&tag_spos, dt->pos);
RESOLVE_TAG (&tag_advance, dt->advance);

Is it really true that resolve_tag_format needs the locus at
gfc_resolve_dt::loc instead of e->where as with the other errors in
resolve_tag_format? If so, are the other errors also incorrect in
using e->where? Might it then be better to pass loc from
gfc_resolve_dt down to resolve_tag/RESOLVE_TAG and then
resolve_tag_format, instead of swapping gfc_current_locus?

Post by Steve Kargl
PR fortran/88048
* resolve.c (check_data_variable): Convert gfc_internal_error to
an gfc_error. Add a nearby missing 'return false;'

[...]

Post by Steve Kargl
PR fortran/88025
* expr.c (gfc_apply_init): Remove asserts and check for valid
ts->u.cl->length.

[...]

Post by Steve Kargl
PR fortran/88116
* simplify.c: Remove internal error and return gfc_bad_expr.

These look good.

Post by Steve Kargl
PR fortran/88269
* io.c (io_constraint): Update macro. Remove incompatible use
of io_constraint and give explicit error.

[...]

There should be two separate references to io_constraint and

Post by Steve Kargl
* io.c (io_constraint): Update macro.
(check_io_constraints) Remove incompatible use
of io_constraint and give explicit error.
#define io_constraint(condition,msg,arg)\
if (condition) \
{\
- gfc_error(msg,arg);\
+ if ((arg)->lb != NULL) \
+ gfc_error(msg,arg);\
+ else \
+ gfc_error(msg,&gfc_current_locus);\
m = MATCH_ERROR;\
}

I think you could safely follow style conventions here (Comma-Space
and Function-Space-Parenthesis):

-#define io_constraint(condition,msg,arg)\
+#define io_constraint(condition, msg, arg)\
if (condition) \
{\
- gfc_error(msg,arg);\
+ if ((arg)->lb != NULL) \
+ gfc_error (msg, arg);\
+ else \
+ gfc_error (msg, &gfc_current_locus);\
m = MATCH_ERROR;\
}

Post by Steve Kargl
@@ -3741,11 +3779,14 @@ if (condition) \
if (expr && expr->ts.type != BT_CHARACTER)
{
- io_constraint (gfc_pure (NULL) && (k == M_READ || k == M_WRITE),
- "IO UNIT in %s statement at %C must be "
+ if (gfc_pure (NULL) && (k == M_READ || k == M_WRITE))
+ {
+ gfc_error ("IO UNIT in %s statement at %C must be "
"an internal file in a PURE procedure",
io_kind_name (k));
-
+ return MATCH_ERROR;
+ }
+
if (k == M_READ || k == M_WRITE)
gfc_unset_implicit_pure (NULL);
}

Trailing whitespace on line 3789 (post-patch).

Post by Steve Kargl
PR fortran/87945
* decl.c (var_element): Inquiry parameter cannot be a data object.
(match_data_constant): Inquiry parameter can be a data
in a data statement.

[...]

Post by Steve Kargl
@@ -391,6 +399,14 @@ match_data_constant (gfc_expr **result)
}
else if (m == MATCH_YES)
{
+ /* If a parameter inquiry ends up here, symtree is NULL but **result
+ contains the right constant expression. Check here. */
+ if ((*result)->symtree == NULL
+ && (*result)->expr_type == EXPR_CONSTANT
+ && ((*result)->ts.type == BT_INTEGER
+ || (*result)->ts.type == BT_REAL))
+ return m;
+
/* F2018:R845 data-stmt-constant is initial-data-target.
A data-stmt-constant shall be ... initial-data-target if and
only if the corresponding data-stmt-object has the POINTER

Trailing whitespace on line 406 (post-patch).

Cheers,
Fritz

[Steve Kargl]

Post by Fritz Reese
On Wed, Dec 5, 2018 at 12:00 AM Steve Kargl

Post by Steve Kargl
I intend to commit the attached patch on Saturday.

Thanks for the work. I assume the patch bootstraps and passes
regression tests?

The patch passed regression testing on i586-*-freebsd.
I'll also do regression testing on x86_64-*-freebsd
prior to the commit.

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88228
* expr.c (check_null, check_elemental): Work around -fdec and
initialization with logical operators operating on integers.

I plan to review this section of the patch later today -- though the
patch hides the segfault from the PR, I need more time to determine
whether it is correct and complete.

By the time the gfc_expr is given to check_check and check_elemental,
it has been reduced to a EXPR_CONSTANT, which neither routine expected.
I simply return early in that case.

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

Comment #3 does not contain a patch to fix the problem elsewhere.

In F2003, 15.2.6 "Interoperability of procedures and procedure interfaces",
I cannot find a prohibition on an alternate return in a subroutine
interface with BIND(C).

I'm disinclined to let a patch fester in bugzilla to only attain
the same fate as my patch to PR68544.

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88205
* io.c (gfc_match_open): STATUS must be CHARACTER type.

[...]

Post by Steve Kargl
@@ -2161,6 +2167,12 @@ gfc_match_open (void)
if (!open->file && open->status)
{
+ if (open->status->ts.type != BT_CHARACTER)
+ {
+ gfc_error ("STATUS must be a default character type at %C");
+ goto cleanup;
+ }
+
if (open->status->expr_type == EXPR_CONSTANT
&& gfc_wide_strncasecmp (open->status->value.character.string,
"scratch", 7) != 0)

Both resolve_tag() and is_char_type() actually already catch type
mismatches for STATUS (the latter for constant expressions). The real
problem is the following condition which checks STATUS before it has
been processed yet, since NEWUNIT is processed before STATUS. I think
the correct thing to do is actually to move the NEWUNIT/UNIT if-block
after the STATUS if-block, rather than adding a new phrasing for the
same error.

OK. I'll check to see if this works.

Post by Fritz Reese
open (newunit=n, status=status)
1
Error: STATUS requires a scalar-default-char-expr at (1)

Post by Steve Kargl
PR fortran/88328
* io.c (resolve_tag_format): Detect zero-sized array.

[...]

Post by Steve Kargl
Index: gcc/fortran/io.c
===================================================================
--- gcc/fortran/io.c (revision 266718)
+++ gcc/fortran/io.c (working copy)
@@ -1636,6 +1636,12 @@ resolve_tag_format (gfc_expr *e)
gfc_expr *r;
gfc_char_t *dest, *src;
+ if (e->value.constructor == NULL)
+ {
+ gfc_error ("FORMAT tag at %C cannot be a zero-sized array");
+ return false;
+ }
+
n = 0;
c = gfc_constructor_first (e->value.constructor);
len = c->expr->value.character.length;

[...]

Post by Steve Kargl
@ -3231,12 +3257,21 @@ gfc_resolve_dt (gfc_dt *dt, locus *loc)
{
gfc_expr *e;
io_kind k;
+ locus loc_tmp;
/* This is set in any case. */
gcc_assert (dt->dt_io_kind);
k = dt->dt_io_kind->value.iokind;
- RESOLVE_TAG (&tag_format, dt->format_expr);
+ loc_tmp = gfc_current_locus;
+ gfc_current_locus = *loc;
+ if (!resolve_tag (&tag_format, dt->format_expr))
+ {
+ gfc_current_locus = loc_tmp;
+ return false;
+ }
+ gfc_current_locus = loc_tmp;
+
RESOLVE_TAG (&tag_rec, dt->rec);
RESOLVE_TAG (&tag_spos, dt->pos);
RESOLVE_TAG (&tag_advance, dt->advance);

Is it really true that resolve_tag_format needs the locus at
gfc_resolve_dt::loc instead of e->where as with the other errors in
resolve_tag_format? If so, are the other errors also incorrect in
using e->where? Might it then be better to pass loc from
gfc_resolve_dt down to resolve_tag/RESOLVE_TAG and then
resolve_tag_format, instead of swapping gfc_current_locus?

program p
character(3), parameter :: a(0) = [character(3)::]
print a
end

With the patch using loc I get

a.f90:3:10:

3 | print a
| 1
Error: FORMAT tag at (1) cannot be a zero-sized array

If I used e->where one gets

a.f90:2:32:

2 | character(3), parameter :: a(0) = [character(3)::]
| 1
Error: FORMAT tag at (1) cannot be a zero-sized array

Now, imagine a few hundred lines separating the two statements.
I think the latter error locus is preferable.

I did not audit the other uses of e->where to see where the
locus ends up pointing if those errors are triggered.

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88048
* resolve.c (check_data_variable): Convert gfc_internal_error to
an gfc_error. Add a nearby missing 'return false;'

[...]

Post by Steve Kargl
PR fortran/88025
* expr.c (gfc_apply_init): Remove asserts and check for valid
ts->u.cl->length.

[...]

Post by Steve Kargl
PR fortran/88116
* simplify.c: Remove internal error and return gfc_bad_expr.

These look good.

I'll address these before the commit.

[Thomas Koenig]

Hi Steve,

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

Comment #3 does not contain a patch to fix the problem elsewhere.

I know :-)

Post by Steve Kargl
In F2003, 15.2.6 "Interoperability of procedures and procedure interfaces",
I cannot find a prohibition on an alternate return in a subroutine
interface with BIND(C).

I also does not allow this, and does not offer a valid interpretation
of what it should mean.

If it has a meaning, it should be translatable into something prescribed
by the standard with -fc-prototypes.

I have assigned the error to myself, so I will not forget to fix
it before the gcc 9 release.

Regards

Thomas

[Steve Kargl]

Post by Thomas Koenig
Hi Steve,

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

Comment #3 does not contain a patch to fix the problem elsewhere.

I know :-)

Post by Steve Kargl
In F2003, 15.2.6 "Interoperability of procedures and procedure interfaces",
I cannot find a prohibition on an alternate return in a subroutine
interface with BIND(C).

I also does not allow this, and does not offer a valid interpretation
of what it should mean.
If it has a meaning, it should be translatable into something prescribed
by the standard with -fc-prototypes.
I have assigned the error to myself, so I will not forget to fix
it before the gcc 9 release.

I think it comes down to F2018, 18.3.7, where one has

A Fortran procedure interface is interoperable with a C functioni
prototype if

(1) ...
(2) ...
(3) ...
(4) ...
(5) any dummy argument without the VALUE attribute corresponds to
a formal parameter of the prototype that is of a pointer type,
and either (4 bullets which cannot be satisfied).

I suppose we should check what other compilers do on the
testcase, but I only have access to gfortran.

BTW, write_proc() starts to write out the prototype before the
argument list is checked. If the current gfc_error
is trigger, you get

void foo (Error: Cannot convert %qs to interoperable type...

I think you want to scan the formal argument list for errors
before writing out "void foo (".

[Steve Kargl]

Post by Thomas Koenig

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

Comment #3 does not contain a patch to fix the problem elsewhere.

I know :-)

Post by Steve Kargl
In F2003, 15.2.6 "Interoperability of procedures and procedure interfaces",
I cannot find a prohibition on an alternate return in a subroutine
interface with BIND(C).

I also does not allow this, and does not offer a valid interpretation
of what it should mean.
If it has a meaning, it should be translatable into something prescribed
by the standard with -fc-prototypes.
I have assigned the error to myself, so I will not forget to fix
it before the gcc 9 release.

I have asked on c.l.f. It seems NAG rejects alternate return
mixed with bind(c). FortranFan provided a complete testcase:

subroutine foo(*) bind(C, name='f')
end subroutine foo
program p
interface
subroutine bar(*) bind(C, name='f')
end subroutine bar
end interface
call bar( *10 )
print *, "Return following 'bar' invocation: jumping to 20"
go to 20
10 print *, "THIS IS UNEXPECTED: Alternate return to 10 after bar"
20 continue
stop
end program p

NAG rejects it. Intel, PGI, and gfortran accept it.

[Fritz Reese]

On Wed, Dec 5, 2018 at 7:03 PM Steve Kargl
[...]

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88228
* expr.c (check_null, check_elemental): Work around -fdec and
initialization with logical operators operating on integers.

I plan to review this section of the patch later today -- though the
patch hides the segfault from the PR, I need more time to determine
whether it is correct and complete.

By the time the gfc_expr is given to check_check and check_elemental,
it has been reduced to a EXPR_CONSTANT, which neither routine expected.
I simply return early in that case.

It appears the correct solution is simply the following patch:

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index b2090218d48..775a5c52c65 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -4004,7 +4004,7 @@ resolve_operator (gfc_expr *e)
if (op2->ts.type != e->ts.type || op2->ts.kind != e->ts.kind)
gfc_convert_type (op2, &e->ts, 1);
e = logical_to_bitwise (e);
- return resolve_function (e);
+ break;
}

sprintf (msg, _("Operands of logical operator %%<%s%%> at %%L
are %s/%s"),
@@ -4020,7 +4020,7 @@ resolve_operator (gfc_expr *e)
e->ts.type = BT_INTEGER;
e->ts.kind = op1->ts.kind;
e = logical_to_bitwise (e);
- return resolve_function (e);
+ break;
}

if (op1->ts.type == BT_LOGICAL)

Returning immediately short-circuits various checks and
simplifications which are done in the remainder of resolve_operator,
including gfc_simplify_expr which handles the EXPR_CONSTANT case. The
comments on gfc_reduce_init_expr indicate that check_null and
check_elemental should never get EXPR_CONSTANT anyway if
gfc_resolve_expr is correct. Regression tests verify this patch is
correct. Please use this patch instead for PR 88228, or if you prefer
I can submit/commit the patch myself.

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

Comment #3 does not contain a patch to fix the problem elsewhere.
In F2003, 15.2.6 "Interoperability of procedures and procedure interfaces",
I cannot find a prohibition on an alternate return in a subroutine
interface with BIND(C).
I'm disinclined to let a patch fester in bugzilla to only attain
the same fate as my patch to PR68544.
any dummy argument without the VALUE attribute [...] is interoperable with an entity of the
referenced type (ISO/IEC 9899:1999, 6.2.5, 7.17, and 7.18.1) of the formal parameter

Regardless of whether or not we accept alternate returns in BIND(C)
procedures, the compiler must be at least consistent: if we accept
them (which gfortran currently does), then we should be able to dump
the C prototype (with -fc-prototypes), providing a formal parameter
interoperable with the type of the alternate return dummy argument; if
we reject them, then we should issue the error in parsing (before
handling by -fc-prototypes). In either case, the error message should
not be obscure or meaningless. Even so, the patch here is inconsistent
since we accept the code, but issue an error when attempting to dump
the C prototype.

However, gfortran does not implement alternate return dummy arguments
as actual arguments, but rather using an integer return code
(regardless of the number of alternate return parameters in the
interface). One interpretation of the consequences of this are that
BIND(C) should be rejected, since there is no interoperable formal
parameter which can be used to mirror the dummy argument (required by
15.3.7.2.5 above). An alternate interpretation is that we can continue
to accept BIND(C) with alternate return dummy arguments, but just
ignore the alternate return arguments. The former is perhaps more
"correct"; the latter is perhaps more useful albeit potentially
error-prone.

To patch support for the latter case, rather than issuing an error in
write_proc for procedures with alternate return arguments, we should
output the actual interoperable prototype: in this case we would
output 'int' as the return type (rather than void, as usual for
subroutines) and alternate return dummy arguments would be ignored
(not output). So the output for the example in the PR should really be
'int f()'. Something like this should do it:

diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
index af64588786a..9d6c3945cc5 100644
--- a/gcc/fortran/dump-parse-tree.c
+++ b/gcc/fortran/dump-parse-tree.c
@@ -3239,19 +3239,41 @@ write_proc (gfc_symbol *sym)
gfc_formal_arglist *f;
const char *sym_name;
const char *intent_in;
+ bool has_alternate_returns;

if (sym->binding_label)
sym_name = sym->binding_label;
else
sym_name = sym->name;

- if (sym->ts.type == BT_UNKNOWN)
+ /* Look for alternate return placeholders. */
+ for (f = gfc_sym_get_dummy_args (sym); f; f = f->next)
+ {
+ if (f->sym == NULL)
+ {
+ has_alternate_returns = true;
+ break;
+ }
+ }
+
+ gfc_typespec ts = sym->ts;
+ gfc_array_spec *as = sym->as;
+ if (has_alternate_returns)
+ {
+ /* Alternate returns are implemented as an integer return code from
+ an otherwise void subroutine; override this here. */
+ ts.type = BT_INTEGER;
+ ts.kind = gfc_c_int_kind;
+ as = NULL;
+ }
+
+ if (!has_alternate_returns && sym->ts.type == BT_UNKNOWN)
{
fprintf (dumpfile, "void ");
fputs (sym_name, dumpfile);
}
else
- write_decl (&(sym->ts), sym->as, sym_name, true, &sym->declared_at);
+ write_decl (&ts, as, sym_name, true, &sym->declared_at);

fputs (" (", dumpfile);

@@ -3259,6 +3281,12 @@ write_proc (gfc_symbol *sym)
{
gfc_symbol *s;
s = f->sym;
+
+ /* Ignore alternate return dummy arguments, since they are handled
+ as an integer return value. */
+ if (!s)
+ continue;
+
rok = get_c_type_name (&(s->ts), NULL, &pre, &type_name, &asterisk,
&post, false);
if (rok == T_ERROR)


EDIT:

It appears Thomas beat me to a reply - I believe he's suggested
something like what the above diff should provide. Perhaps this will
be a useful starting point for him.

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88205
* io.c (gfc_match_open): STATUS must be CHARACTER type.

[...]

Post by Steve Kargl
If I used e->where one gets
2 | character(3), parameter :: a(0) = [character(3)::]
| 1
Error: FORMAT tag at (1) cannot be a zero-sized array
Now, imagine a few hundred lines separating the two statements.
I think the latter error locus is preferable.

Yes, I agree.

Swapping gfc_current_locus definitely works, but is possibly less
readable(+maintainable) than my other suggestion of passing loc down
as an argument... But that suggestion touches more code, so there are
merits to either approach. In either case I have no real issue with
this part of the patch regardless of implementation of the locus
workaround.


Fritz

[Steve Kargl]

Post by Fritz Reese
On Wed, Dec 5, 2018 at 7:03 PM Steve Kargl
[...]

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88228
* expr.c (check_null, check_elemental): Work around -fdec and
initialization with logical operators operating on integers.

I plan to review this section of the patch later today -- though the
patch hides the segfault from the PR, I need more time to determine
whether it is correct and complete.

By the time the gfc_expr is given to check_check and check_elemental,
it has been reduced to a EXPR_CONSTANT, which neither routine expected.
I simply return early in that case.

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index b2090218d48..775a5c52c65 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -4004,7 +4004,7 @@ resolve_operator (gfc_expr *e)
if (op2->ts.type != e->ts.type || op2->ts.kind != e->ts.kind)
gfc_convert_type (op2, &e->ts, 1);
e = logical_to_bitwise (e);
- return resolve_function (e);
+ break;
}
sprintf (msg, _("Operands of logical operator %%<%s%%> at %%L
are %s/%s"),
@@ -4020,7 +4020,7 @@ resolve_operator (gfc_expr *e)
e->ts.type = BT_INTEGER;
e->ts.kind = op1->ts.kind;
e = logical_to_bitwise (e);
- return resolve_function (e);
+ break;
}

Intersting. I wonder why resolve_function() appears here.
Hmmm, 'svn annotate' points to r241535 with the committer
being foreese. :-) Log message says you are trying to convert
logical op on integers for DEC. Were trying to catch the
logical functions like NOT()?

I'll include the above in my patch Saturday activities.

Post by Fritz Reese

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

Comment #3 does not contain a patch to fix the problem elsewhere.
In F2003, 15.2.6 "Interoperability of procedures and procedure interfaces",
I cannot find a prohibition on an alternate return in a subroutine
interface with BIND(C).
I'm disinclined to let a patch fester in bugzilla to only attain
the same fate as my patch to PR68544.
any dummy argument without the VALUE attribute [...] is interoperable with an entity of the
referenced type (ISO/IEC 9899:1999, 6.2.5, 7.17, and 7.18.1) of the formal parameter

Yep.

Post by Fritz Reese
Regardless of whether or not we accept alternate returns in BIND(C)
procedures, the compiler must be at least consistent: if we accept
them (which gfortran currently does), then we should be able to dump
the C prototype (with -fc-prototypes), providing a formal parameter
interoperable with the type of the alternate return dummy argument; if
we reject them, then we should issue the error in parsing (before
handling by -fc-prototypes). In either case, the error message should
not be obscure or meaningless. Even so, the patch here is inconsistent
since we accept the code, but issue an error when attempting to dump

I think we should determine what other compilers do with
BIND(C) and alternate return dummy arguments, and follow
suit.

Post by Fritz Reese

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88205
* io.c (gfc_match_open): STATUS must be CHARACTER type.

[...]

Post by Steve Kargl
If I used e->where one gets
2 | character(3), parameter :: a(0) = [character(3)::]
| 1
Error: FORMAT tag at (1) cannot be a zero-sized array
Now, imagine a few hundred lines separating the two statements.
I think the latter error locus is preferable.

Yes, I agree.
Swapping gfc_current_locus definitely works, but is possibly less
readable(+maintainable) than my other suggestion of passing loc down
as an argument... But that suggestion touches more code, so there are
merits to either approach. In either case I have no real issue with
this part of the patch regardless of implementation of the locus
workaround.

I agree that this could get messy, but I'm hoping only
format strings need this special handling. A Fortran
string must contain '(' and ')', so zero-sized arrays
can never appear as a fmt=array.

[Steve Kargl]

Post by Fritz Reese
On Wed, Dec 5, 2018 at 7:03 PM Steve Kargl

Post by Steve Kargl

Post by Fritz Reese

Post by Steve Kargl
PR fortran/88139
* dump-parse-tree.c (write_proc): Alternate return.

I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

Comment #3 does not contain a patch to fix the problem elsewhere.
In F2003, 15.2.6 "Interoperability of procedures and procedure interfaces",
I cannot find a prohibition on an alternate return in a subroutine
interface with BIND(C).
I'm disinclined to let a patch fester in bugzilla to only attain
the same fate as my patch to PR68544.
any dummy argument without the VALUE attribute [...] is interoperable with an entity of the
referenced type (ISO/IEC 9899:1999, 6.2.5, 7.17, and 7.18.1) of the formal parameter

Regardless of whether or not we accept alternate returns in BIND(C)
procedures, the compiler must be at least consistent: if we accept
them (which gfortran currently does), then we should be able to dump
the C prototype (with -fc-prototypes), providing a formal parameter
interoperable with the type of the alternate return dummy argument; if
we reject them, then we should issue the error in parsing (before
handling by -fc-prototypes). In either case, the error message should
not be obscure or meaningless. Even so, the patch here is inconsistent
since we accept the code, but issue an error when attempting to dump
the C prototype.

Here's an alternative patch that would reject a subroutine
with an alternate return dummy argument with the bind(c)
attributes. I'm still trying to determine if the code
should be legal. The c.l.f thread I started isn't helping :(

Index: decl.c
===================================================================
--- decl.c (revision 266766)
+++ decl.c (working copy)
@@ -7467,6 +7467,7 @@ gfc_match_subroutine (void)
match is_bind_c;
char peek_char;
bool allow_binding_name;
+ locus loc;

if (gfc_current_state () != COMP_NONE
&& gfc_current_state () != COMP_INTERFACE
@@ -7532,6 +7533,8 @@ gfc_match_subroutine (void)
/* Here, we are just checking if it has the bind(c) attribute, and if
so, then we need to make sure it's all correct. If it doesn't,
we still need to continue matching the rest of the subroutine line. */
+ gfc_gobble_whitespace ();
+ loc = gfc_current_locus;
is_bind_c = gfc_match_bind_c (sym, allow_binding_name);
if (is_bind_c == MATCH_ERROR)
{
@@ -7543,6 +7546,8 @@ gfc_match_subroutine (void)

if (is_bind_c == MATCH_YES)
{
+ gfc_formal_arglist *arg;
+
/* The following is allowed in the Fortran 2008 draft. */
if (gfc_current_state () == COMP_CONTAINS
&& sym->ns->proc_name->attr.flavor != FL_MODULE
@@ -7556,8 +7561,17 @@ gfc_match_subroutine (void)
gfc_error ("Missing required parentheses before BIND(C) at %C");
return MATCH_ERROR;
}
- if (!gfc_add_is_bind_c (&(sym->attr), sym->name,
- &(sym->declared_at), 1))
+
+ /* Scan the dummy arguments for an alternate return. */
+ for (arg = sym->formal; arg; arg = arg->next)
+ if (!arg->sym)
+ {
+ gfc_error ("Alternate return dummy argument cannot appear in a "
+ "SUBROUTINE with the BIND(C) attribute at %L", &loc);
+ return MATCH_ERROR;
+ }
+
+ if (!gfc_add_is_bind_c (&(sym->attr), sym->name, &(sym->declared_at), 1))
return MATCH_ERROR;
}

[Steve Kargl]

Post by Steve Kargl
Here's an alternative patch that would reject a subroutine
with an alternate return dummy argument with the bind(c)
attributes. I'm still trying to determine if the code
should be legal. The c.l.f thread I started isn't helping :(

I think I have found the restriction. In F2018,

C1554 If proc-language-binding-spec is specified for a procedure, each
of its dummy arguments shall be an interoperable procedure (18.3.7)
or a variable that is interoperable (18.3.5, 18.3.6), assumed-shape,
assumed-rank, assumed-type, of type CHARACTER with assumed length,
or that has the ALLOCATABLE or POINTER attribute.

Post by Steve Kargl
Index: decl.c
===================================================================
--- decl.c (revision 266766)
+++ decl.c (working copy)
@@ -7467,6 +7467,7 @@ gfc_match_subroutine (void)
match is_bind_c;
char peek_char;
bool allow_binding_name;
+ locus loc;
if (gfc_current_state () != COMP_NONE
&& gfc_current_state () != COMP_INTERFACE
@@ -7532,6 +7533,8 @@ gfc_match_subroutine (void)
/* Here, we are just checking if it has the bind(c) attribute, and if
so, then we need to make sure it's all correct. If it doesn't,
we still need to continue matching the rest of the subroutine line. */
+ gfc_gobble_whitespace ();
+ loc = gfc_current_locus;
is_bind_c = gfc_match_bind_c (sym, allow_binding_name);
if (is_bind_c == MATCH_ERROR)
{
@@ -7543,6 +7546,8 @@ gfc_match_subroutine (void)
if (is_bind_c == MATCH_YES)
{
+ gfc_formal_arglist *arg;
+
/* The following is allowed in the Fortran 2008 draft. */
if (gfc_current_state () == COMP_CONTAINS
&& sym->ns->proc_name->attr.flavor != FL_MODULE
@@ -7556,8 +7561,17 @@ gfc_match_subroutine (void)
gfc_error ("Missing required parentheses before BIND(C) at %C");
return MATCH_ERROR;
}
- if (!gfc_add_is_bind_c (&(sym->attr), sym->name,
- &(sym->declared_at), 1))
+
+ /* Scan the dummy arguments for an alternate return. */
+ for (arg = sym->formal; arg; arg = arg->next)
+ if (!arg->sym)
+ {
+ gfc_error ("Alternate return dummy argument cannot appear in a "
+ "SUBROUTINE with the BIND(C) attribute at %L", &loc);
+ return MATCH_ERROR;
+ }
+
+ if (!gfc_add_is_bind_c (&(sym->attr), sym->name, &(sym->declared_at), 1))
return MATCH_ERROR;
}
--
steve