* [patch, fortran] Fix PR 44960, accepts invalid reference on function
@ 2020-01-16 23:00 Thomas Koenig
2020-01-17 7:03 ` Steve Kargl
2020-01-17 8:31 ` Tobias Burnus
0 siblings, 2 replies; 11+ messages in thread
From: Thomas Koenig @ 2020-01-16 23:00 UTC (permalink / raw)
To: fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 659 bytes --]
Hello world,
here is my first patch made from the git world. It certainly took
enough time to work out how to to this, but I think I have it
figured out now...
Anyway, the fix is rather straightforward. We cannot have a reference
on a function. If this slipped through on parsing, let's issue the
error during resolution.
Regression-tested. OK for trunk?
Regards
Thomas
2020-01-16 Thomas König <tkoenig@gcc.gnu.org>
PR fortran/44960
* resolve.c (resolve_function): Issue error when a
function call contains a reference.
2020-01-16 Thomas König <tkoenig@gcc.gnu.org>
PR fortran/44960
* gfortran.dg/function_reference_1.f90: New test.
[-- Attachment #2: p1.diff --]
[-- Type: text/x-patch, Size: 1030 bytes --]
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 6f2a4c4d65a..1525c00ea4c 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -3129,6 +3129,13 @@ resolve_function (gfc_expr *expr)
|| sym->intmod_sym_id == GFC_ISYM_CAF_SEND))
return true;
+ if (expr->ref)
+ {
+ gfc_error ("Function call can not contain a reference at %L",
+ &expr->where);
+ return false;
+ }
+
if (sym && sym->attr.intrinsic
&& !gfc_resolve_intrinsic (sym, &expr->where))
return false;
diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90
new file mode 100644
index 00000000000..78c92a6f20d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR 44960 - this was erroneusly accepted.
+! Original test case by Daniel Franke.
+
+type t
+ integer :: a
+end type t
+type(t) :: foo
+print *, foo(1)%a ! { dg-error "Function call can not contain a reference" }
+end
+
[-- Attachment #3: function_reference_1.f90 --]
[-- Type: text/x-fortran, Size: 233 bytes --]
! { dg-do compile }
! PR 44960 - this was erroneusly accepted.
! Original test case by Daniel Franke.
type t
integer :: a
end type t
type(t) :: foo
print *, foo(1)%a ! { dg-error "Function call can not contain a reference" }
end
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-16 23:00 [patch, fortran] Fix PR 44960, accepts invalid reference on function Thomas Koenig
@ 2020-01-17 7:03 ` Steve Kargl
2020-01-17 8:43 ` Tobias Burnus
2020-01-17 8:31 ` Tobias Burnus
1 sibling, 1 reply; 11+ messages in thread
From: Steve Kargl @ 2020-01-17 7:03 UTC (permalink / raw)
To: Thomas Koenig; +Cc: fortran, gcc-patches
On Thu, Jan 16, 2020 at 11:34:43PM +0100, Thomas Koenig wrote:
> diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90
> new file mode 100644
> index 00000000000..78c92a6f20d
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90
> @@ -0,0 +1,11 @@
> +! { dg-do compile }
> +! PR 44960 - this was erroneusly accepted.
> +! Original test case by Daniel Franke.
> +
> +type t
> + integer :: a
> +end type t
> +type(t) :: foo
> +print *, foo(1)%a ! { dg-error "Function call can not contain a reference" }
> +end
> +
I do not understand this error message, and find it to be confusing.
foo(1)%a looks like an invalid array reference. That is, foo is scalar
and foo(1) is an array element.
PS: s/can not/cannot
--
Steve
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-16 23:00 [patch, fortran] Fix PR 44960, accepts invalid reference on function Thomas Koenig
2020-01-17 7:03 ` Steve Kargl
@ 2020-01-17 8:31 ` Tobias Burnus
1 sibling, 0 replies; 11+ messages in thread
From: Tobias Burnus @ 2020-01-17 8:31 UTC (permalink / raw)
To: Thomas Koenig, fortran, gcc-patches
Hello Thomas, hi all,
On 1/16/20 11:34 PM, Thomas Koenig wrote:
> Anyway, the fix is rather straightforward. We cannot have a reference
> on a function. If this slipped through on parsing, let's issue the
> error during resolution.
>
> Regression-tested. OK for trunk?
I think I am fine with the check itself â but â¦
> + if (expr->ref)
> + {
> + gfc_error ("Function call can not contain a reference at %L",
I have issues with the wording of the error message.
First, when reading the error message without context, I thought it is
about something regarding the arguments ("contain") or maybe the
reference of a function iself.
It is clear that we do not want to have anything after the
function-reference (âR1521). And we do lack a good wording for that what
one can (but shan't) put after the function-reference as it can be quite
a lot (cf. âR901): a following '%' to attempt to create a
structure-component or complex-part-component access or "(" to get a
substring or array-element/section access or '[' to get something coindexed.
Not that I like the wording in particular, but we use quite often
"Unexpected junk" in the parser. Hence: "Unexpected junk after function
reference at %L" would work.
Or maybe clearer:
char ref_char = '%'; // REF_INQUIRY or REF_COMPONENT
if (expr->ref->type == REF_ARRAY || expr->ref->type == REF_SUBSTRING)
ref_char = ((expr->ref->type == REF_ARRAY && !expr->ref->u.ar.dimen)
'[' : '(';
 ⦠("Unexpected %qc after function reference at %L", ref_char, â¦);
Although, I am not sure the '[' (coindex) can occur.
Cheers,
Tobias
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-17 7:03 ` Steve Kargl
@ 2020-01-17 8:43 ` Tobias Burnus
2020-01-17 16:26 ` Steve Kargl
0 siblings, 1 reply; 11+ messages in thread
From: Tobias Burnus @ 2020-01-17 8:43 UTC (permalink / raw)
To: sgk, Thomas Koenig; +Cc: fortran, gcc-patches
On 1/17/20 4:49 AM, Steve Kargl wrote:
> On Thu, Jan 16, 2020 at 11:34:43PM +0100, Thomas Koenig wrote:
>> +type(t) :: foo
>> +print *, foo(1)%a ! { dg-error "Function call can not contain a reference" }
> I do not understand this error message, and find it to be confusing.
> foo(1)%a looks like an invalid array reference. That is, foo is scalar
> and foo(1) is an array element.
Well, we simply do not know whether "external" or "dimension" has been
forgotten. As "external" can also be determined by the use, we end up
regarding it as function referenceâ¦
Another example:
character(len=4):: str
print *, str(1)(1:4)
end
Maybe a more helpful error message is: "Unexpected junk after function
reference or missing dimension declaration for %s", sym->name)
(Or instead of "junk" the fancier variant of my previous email.)
Cheers,
Tobias
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-17 8:43 ` Tobias Burnus
@ 2020-01-17 16:26 ` Steve Kargl
2020-01-17 23:14 ` Thomas Koenig
0 siblings, 1 reply; 11+ messages in thread
From: Steve Kargl @ 2020-01-17 16:26 UTC (permalink / raw)
To: Tobias Burnus; +Cc: Thomas Koenig, fortran, gcc-patches
On Fri, Jan 17, 2020 at 09:29:33AM +0100, Tobias Burnus wrote:
> On 1/17/20 4:49 AM, Steve Kargl wrote:
> > On Thu, Jan 16, 2020 at 11:34:43PM +0100, Thomas Koenig wrote:
> >> +type(t) :: foo
> >> +print *, foo(1)%a ! { dg-error "Function call can not contain a reference" }
> > I do not understand this error message, and find it to be confusing.
> > foo(1)%a looks like an invalid array reference. That is, foo is scalar
> > and foo(1) is an array element.
>
> Well, we simply do not know whether "external" or "dimension" has been
> forgotten. As "external" can also be determined by the use, we end up
> regarding it as function referenceâ¦
>
> Another example:
>
> character(len=4):: str
> print *, str(1)(1:4)
> end
>
> Maybe a more helpful error message is: "Unexpected junk after function
> reference or missing dimension declaration for %s", sym->name)
>
> (Or instead of "junk" the fancier variant of my previous email.)
>
Gfortran probably should not try to guess what the user
thought s/he wanted. The generic "Syntax error" would
seem to apply here. To me, foo(1)%a looks much more like
an array reference rather than a function reference.
--
Steve
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-17 16:26 ` Steve Kargl
@ 2020-01-17 23:14 ` Thomas Koenig
2020-01-18 0:06 ` Steve Kargl
2020-01-18 12:09 ` Tobias Burnus
0 siblings, 2 replies; 11+ messages in thread
From: Thomas Koenig @ 2020-01-17 23:14 UTC (permalink / raw)
To: sgk, Tobias Burnus; +Cc: fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 724 bytes --]
Am 17.01.20 um 15:42 schrieb Steve Kargl:
> Gfortran probably should not try to guess what the user
> thought s/he wanted. The generic "Syntax error" would
> seem to apply here. To me, foo(1)%a looks much more like
> an array reference rather than a function reference.
OK, so here's a patch which does just that.
The error message low looks like
function_reference_1.f90:9:8:
9 | print *, foo(1)%a ! { dg-error "Syntax error" }
| 1
Error: Syntax error in expression at (1)
The location information is a bit off, but in the absence of location
information for the reference (which we do not collect), I think this
is the best I can do.
So, OK for trunk (with the old ChangeLog)?
Regards
Thomas
[-- Attachment #2: p2.diff --]
[-- Type: text/x-patch, Size: 982 bytes --]
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 6f2a4c4d65a..a846677b770 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -3129,6 +3129,12 @@ resolve_function (gfc_expr *expr)
|| sym->intmod_sym_id == GFC_ISYM_CAF_SEND))
return true;
+ if (expr->ref)
+ {
+ gfc_error ("Syntax error in expression at %L", &expr->where);
+ return false;
+ }
+
if (sym && sym->attr.intrinsic
&& !gfc_resolve_intrinsic (sym, &expr->where))
return false;
diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90
new file mode 100644
index 00000000000..1b7f4809c5c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR 44960 - this was erroneusly accepted.
+! Original test case by Daniel Franke.
+
+type t
+ integer :: a
+end type t
+type(t) :: foo
+print *, foo(1)%a ! { dg-error "Syntax error" }
+end
+
[-- Attachment #3: function_reference_1.f90 --]
[-- Type: text/x-fortran, Size: 204 bytes --]
! { dg-do compile }
! PR 44960 - this was erroneusly accepted.
! Original test case by Daniel Franke.
type t
integer :: a
end type t
type(t) :: foo
print *, foo(1)%a ! { dg-error "Syntax error" }
end
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-17 23:14 ` Thomas Koenig
@ 2020-01-18 0:06 ` Steve Kargl
2020-01-18 12:09 ` Tobias Burnus
1 sibling, 0 replies; 11+ messages in thread
From: Steve Kargl @ 2020-01-18 0:06 UTC (permalink / raw)
To: Thomas Koenig; +Cc: Tobias Burnus, fortran, gcc-patches
On Fri, Jan 17, 2020 at 11:20:06PM +0100, Thomas Koenig wrote:
> Am 17.01.20 um 15:42 schrieb Steve Kargl:
> > Gfortran probably should not try to guess what the user
> > thought s/he wanted. The generic "Syntax error" would
> > seem to apply here. To me, foo(1)%a looks much more like
> > an array reference rather than a function reference.
>
> OK, so here's a patch which does just that.
>
> The error message low looks like
>
> function_reference_1.f90:9:8:
>
> 9 | print *, foo(1)%a ! { dg-error "Syntax error" }
> | 1
> Error: Syntax error in expression at (1)
>
> The location information is a bit off, but in the absence of location
> information for the reference (which we do not collect), I think this
> is the best I can do.
>
> So, OK for trunk (with the old ChangeLog)?
>
It's fine with me. May want to give Tobias a chance to comment.
> + if (expr->ref)
> + {
> + gfc_error ("Syntax error in expression at %L", &expr->where);
I assume that %C puts the locus at the end of the line. I haven't
spent to much time trying to understand expressions in an output IO
list, but as you state, it seems that gfortran loose the locus.
--
Steve
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-17 23:14 ` Thomas Koenig
2020-01-18 0:06 ` Steve Kargl
@ 2020-01-18 12:09 ` Tobias Burnus
2020-01-18 12:55 ` Thomas Koenig
2020-01-19 1:58 ` Thomas Koenig
1 sibling, 2 replies; 11+ messages in thread
From: Tobias Burnus @ 2020-01-18 12:09 UTC (permalink / raw)
To: Thomas Koenig, sgk, Tobias Burnus; +Cc: fortran, gcc-patches
On 1/17/20 11:20 PM, Thomas Koenig wrote:
> function_reference_1.f90:9:8:
>
> Â Â Â 9 | print *, foo(1)%a ! { dg-error "Syntax error" }
> Â Â Â Â Â |Â Â Â Â Â Â Â 1
> Error: Syntax error in expression at (1)
The error message is not helpful at all. I was recently struggling to
understand why/where some code was failing with syntax error â and it
took me a while to find it. And with this message, I had also to find
out what did go wrong.
How about: ("Unexpected junk after %s at %L",
expr->n.symtree->sym->name, &expr->where)? â or "Unexpected junk in
reference to %s at %L"?
Or deviating from your current error messages: ""Inconsistent use of %s
at %L"
(Side note: I think we have the general problem that expr->where does
not start after the white space, which can be slightly confusing.)
Cheers,
Tobias
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-18 12:09 ` Tobias Burnus
@ 2020-01-18 12:55 ` Thomas Koenig
2020-01-19 1:58 ` Thomas Koenig
1 sibling, 0 replies; 11+ messages in thread
From: Thomas Koenig @ 2020-01-18 12:55 UTC (permalink / raw)
To: Tobias Burnus, sgk, Tobias Burnus; +Cc: fortran, gcc-patches
Am 18.01.20 um 12:44 schrieb Tobias Burnus:
>> function_reference_1.f90:9:8:
>>
>> Â Â Â 9 | print *, foo(1)%a ! { dg-error "Syntax error" }
>> Â Â Â Â Â |Â Â Â Â Â Â Â 1
>> Error: Syntax error in expression at (1)
>
> The error message is not helpful at all.
Well, yes. It is no better than what we currently have for the slightly
different test case
type t
integer :: a
end type t
type(t) :: foo
external foo
print *, foo(1)%a
end
which is
a.f90:6:16:
6 | print *, foo(1)%a
| 1
Error: Syntax error in PRINT statement at (1)
(but at least that points to the right place).
I can see if I can also replace this with something more useful
(have to find the place where this is issued first, though).
Regards
Thomas
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-18 12:09 ` Tobias Burnus
2020-01-18 12:55 ` Thomas Koenig
@ 2020-01-19 1:58 ` Thomas Koenig
2020-01-19 2:17 ` Tobias Burnus
1 sibling, 1 reply; 11+ messages in thread
From: Thomas Koenig @ 2020-01-19 1:58 UTC (permalink / raw)
To: Tobias Burnus, sgk, Tobias Burnus; +Cc: fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 656 bytes --]
Hello world,
I found an ommission in primary.c which prevented issuing a
more specific error instead of "syntax error" for the case
when a function was declared in an EXTERNAL statement,
and I have now gone for the "Unexpected junk after foo"
variant.
Regression-tested. OK for trunk?
Regards
Thomas
2020-01-18 Thomas König <tkoenig@gcc.gnu.org>
PR fortran/44960
* primary.c (gfc_match_rvalue): Break after setting MATCH_ERROR.
* resolve.c (resolve_function): Issue error when a
function call contains a reference.
2020-01-18 Thomas König <tkoenig@gcc.gnu.org>
PR fortran/44960
* gfortran.dg/function_reference_1.f90: New test.
[-- Attachment #2: p3.diff --]
[-- Type: text/x-patch, Size: 1890 bytes --]
diff --git a/gcc/fortran/primary.c b/gcc/fortran/primary.c
index 07b8ac08ba2..bd50827bb15 100644
--- a/gcc/fortran/primary.c
+++ b/gcc/fortran/primary.c
@@ -3661,6 +3661,7 @@ gfc_match_rvalue (gfc_expr **result)
gfc_error ("The leftmost part-ref in a data-ref cannot be a "
"function reference at %C");
m = MATCH_ERROR;
+ break;
}
m = MATCH_YES;
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 6f2a4c4d65a..697afadb378 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -3129,6 +3129,13 @@ resolve_function (gfc_expr *expr)
|| sym->intmod_sym_id == GFC_ISYM_CAF_SEND))
return true;
+ if (expr->ref)
+ {
+ gfc_error ("Unexpected junk after %qs at %L", expr->symtree->n.sym->name,
+ &expr->where);
+ return false;
+ }
+
if (sym && sym->attr.intrinsic
&& !gfc_resolve_intrinsic (sym, &expr->where))
return false;
diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90
new file mode 100644
index 00000000000..be634c9dd4b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR 44960 - this was erroneusly accepted.
+! Original test case by Daniel Franke.
+
+type t
+ integer :: a
+end type t
+type(t) :: foo
+print *, foo(1)%a ! { dg-error "Unexpected junk" }
+end
+
diff --git a/gcc/testsuite/gfortran.dg/function_reference_2.f90 b/gcc/testsuite/gfortran.dg/function_reference_2.f90
new file mode 100644
index 00000000000..375c58bb6d2
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/function_reference_2.f90
@@ -0,0 +1,10 @@
+! { dg-do compile }
+! PR 44960 - improve the error message
+program main
+ type t
+ integer :: a
+end type t
+type(t) :: foo
+external foo
+i = foo(1)%1 ! { dg-error "leftmost part-ref in a data-ref cannot be a function reference" }
+end
[-- Attachment #3: function_reference_2.f90 --]
[-- Type: text/x-fortran, Size: 232 bytes --]
! { dg-do compile }
! PR 44960 - improve the error message
program main
type t
integer :: a
end type t
type(t) :: foo
external foo
i = foo(1)%1 ! { dg-error "leftmost part-ref in a data-ref cannot be a function reference" }
end
[-- Attachment #4: function_reference_1.f90 --]
[-- Type: text/x-fortran, Size: 207 bytes --]
! { dg-do compile }
! PR 44960 - this was erroneusly accepted.
! Original test case by Daniel Franke.
type t
integer :: a
end type t
type(t) :: foo
print *, foo(1)%a ! { dg-error "Unexpected junk" }
end
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
2020-01-19 1:58 ` Thomas Koenig
@ 2020-01-19 2:17 ` Tobias Burnus
0 siblings, 0 replies; 11+ messages in thread
From: Tobias Burnus @ 2020-01-19 2:17 UTC (permalink / raw)
To: Thomas Koenig, sgk, Tobias Burnus; +Cc: fortran, gcc-patches
Hello Thomas,
looks good to me. Nice catch the missing "break".
Tobias
PS: I think it does not exercise a differ code path, but instead of
derived types, using a character string works as well. The following is
accepted with the unmodified trunk:
character(len=4):: str
print *, str(1)(1:4)
end
On 1/18/20 7:04 PM, Thomas Koenig wrote:
> Hello world,
>
> I found an ommission in primary.c which prevented issuing a
> more specific error instead of "syntax error" for the case
> when a function was declared in an EXTERNAL statement,
> and I have now gone for the "Unexpected junk after foo"
> variant.
>
> Regression-tested. OK for trunk?
>
> Regards
>
> Â Â Â Â Thomas
>
> 2020-01-18 Thomas König <tkoenig@gcc.gnu.org>
>
> Â Â Â Â PR fortran/44960
> Â Â Â Â * primary.c (gfc_match_rvalue): Break after setting MATCH_ERROR.
> Â Â Â Â * resolve.c (resolve_function): Issue error when a
> Â Â Â Â function call contains a reference.
>
> 2020-01-18 Thomas König <tkoenig@gcc.gnu.org>
>
> Â Â Â Â PR fortran/44960
> Â Â Â Â * gfortran.dg/function_reference_1.f90: New test.
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-01-18 18:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16 23:00 [patch, fortran] Fix PR 44960, accepts invalid reference on function Thomas Koenig
2020-01-17 7:03 ` Steve Kargl
2020-01-17 8:43 ` Tobias Burnus
2020-01-17 16:26 ` Steve Kargl
2020-01-17 23:14 ` Thomas Koenig
2020-01-18 0:06 ` Steve Kargl
2020-01-18 12:09 ` Tobias Burnus
2020-01-18 12:55 ` Thomas Koenig
2020-01-19 1:58 ` Thomas Koenig
2020-01-19 2:17 ` Tobias Burnus
2020-01-17 8:31 ` Tobias Burnus
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).