* [Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration
@ 2018-06-09 12:13 Janus Weil
2018-06-09 22:31 ` Thomas Koenig
0 siblings, 1 reply; 4+ messages in thread
From: Janus Weil @ 2018-06-09 12:13 UTC (permalink / raw)
To: gfortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 653 bytes --]
Hi all,
attached is a small patch that approves some diagnostics for INTENT
declarations. It also takes care of a TODO note in decl.c.
I had regtested a previous version of the patch without problems and
will do another run with this one before checking in. Ok for trunk?
Cheers,
Janus
2018-06-09 Janus Weil <janus@gcc.gnu.org>
PR fortran/85088
* decl.c (match_attr_spec): Synchronize the DECL_* enum values with the
INTENT_* values from the enum 'sym_intent'. Call 'match_intent_spec'
and remove a TODO note.
2018-06-09 Janus Weil <janus@gcc.gnu.org>
PR fortran/85088
* gfortran.dg/intent_decl_1.f90: New test case.
[-- Attachment #2: pr85088.diff --]
[-- Type: text/x-patch, Size: 1392 bytes --]
Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c (revision 261358)
+++ gcc/fortran/decl.c (working copy)
@@ -4716,9 +4716,10 @@ match_attr_spec (void)
{
/* Modifiers that can exist in a type statement. */
enum
- { GFC_DECL_BEGIN = 0,
- DECL_ALLOCATABLE = GFC_DECL_BEGIN, DECL_DIMENSION, DECL_EXTERNAL,
- DECL_IN, DECL_OUT, DECL_INOUT, DECL_INTRINSIC, DECL_OPTIONAL,
+ { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
+ DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
+ DECL_DIMENSION, DECL_EXTERNAL,
+ DECL_INTRINSIC, DECL_OPTIONAL,
DECL_PARAMETER, DECL_POINTER, DECL_PROTECTED, DECL_PRIVATE,
DECL_STATIC, DECL_AUTOMATIC,
DECL_PUBLIC, DECL_SAVE, DECL_TARGET, DECL_VALUE, DECL_VOLATILE,
@@ -4846,13 +4847,12 @@ match_attr_spec (void)
if (match_string_p ("nt"))
{
/* Matched "intent". */
- /* TODO: Call match_intent_spec from here. */
- if (gfc_match (" ( in out )") == MATCH_YES)
- d = DECL_INOUT;
- else if (gfc_match (" ( in )") == MATCH_YES)
- d = DECL_IN;
- else if (gfc_match (" ( out )") == MATCH_YES)
- d = DECL_OUT;
+ d = match_intent_spec ();
+ if (d == INTENT_UNKNOWN)
+ {
+ m = MATCH_ERROR;
+ goto cleanup;
+ }
}
}
else if (ch == 'r')
[-- Attachment #3: intent_decl_1.f90 --]
[-- Type: text/x-fortran, Size: 366 bytes --]
! { dg-do compile }
!
! PR 85088: improve diagnostic for bad INTENT declaration
!
! Contributed by Janus Weil <janus@gcc.gnu.org>
subroutine s(x, y, z)
integer, intent(int) :: x ! { dg-error "Bad INTENT specification" }
integer, intent :: y ! { dg-error "Bad INTENT specification" }
integer, inten :: z ! { dg-error "Invalid character" }
end
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration
2018-06-09 12:13 [Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration Janus Weil
@ 2018-06-09 22:31 ` Thomas Koenig
2018-06-10 12:09 ` Janus Weil
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Koenig @ 2018-06-09 22:31 UTC (permalink / raw)
To: Janus Weil, gfortran, gcc-patches
Hi Janus,
I like what the patch does. However, I have one concern.
> * decl.c (match_attr_spec): Synchronize the DECL_* enum values with the
> INTENT_* values from the enum 'sym_intent'.
This part
+ { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
+ DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
+ DECL_DIMENSION, DECL_EXTERNAL,
+ DECL_INTRINSIC, DECL_OPTIONAL,
brings a dependency of the values of sym_intent into this enum.
I know that the order of sym_intent is not likely to change, but I would
still prefer if you could add a comment to the sym_intent definition in
gfortran.h noting that INTENT_IN cannot be zero, and add a
gcc_assert (INTENT_IN > 0);
somewhere (which will be optimized away) with an explanatory comment.
OK with that change.
Thanks for the patch!
Thomas
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration
2018-06-09 22:31 ` Thomas Koenig
@ 2018-06-10 12:09 ` Janus Weil
2018-06-10 12:25 ` Janus Weil
0 siblings, 1 reply; 4+ messages in thread
From: Janus Weil @ 2018-06-10 12:09 UTC (permalink / raw)
To: Thomas Koenig; +Cc: gfortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]
Hi Thomas,
> I like what the patch does. However, I have one concern.
>
>> * decl.c (match_attr_spec): Synchronize the DECL_* enum values with
>> the
>> INTENT_* values from the enum 'sym_intent'.
>
>
> This part
>
> + { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
> + DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
> + DECL_DIMENSION, DECL_EXTERNAL,
> + DECL_INTRINSIC, DECL_OPTIONAL,
>
> brings a dependency of the values of sym_intent into this enum.
>
> I know that the order of sym_intent is not likely to change, but I would
> still prefer if you could add a comment to the sym_intent definition in
> gfortran.h noting that INTENT_IN cannot be zero, and add a
>
> gcc_assert (INTENT_IN > 0);
>
> somewhere (which will be optimized away) with an explanatory comment.
good point. I have added the assert and some comments. Updated patch attached.
> OK with that change.
Thanks. I'm doing another regtest now and will commit if that goes well ...
Cheers,
Janus
[-- Attachment #2: pr85088_v2.diff --]
[-- Type: text/x-patch, Size: 2203 bytes --]
Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c (revision 261358)
+++ gcc/fortran/decl.c (working copy)
@@ -4716,9 +4716,10 @@ match_attr_spec (void)
{
/* Modifiers that can exist in a type statement. */
enum
- { GFC_DECL_BEGIN = 0,
- DECL_ALLOCATABLE = GFC_DECL_BEGIN, DECL_DIMENSION, DECL_EXTERNAL,
- DECL_IN, DECL_OUT, DECL_INOUT, DECL_INTRINSIC, DECL_OPTIONAL,
+ { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
+ DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
+ DECL_DIMENSION, DECL_EXTERNAL,
+ DECL_INTRINSIC, DECL_OPTIONAL,
DECL_PARAMETER, DECL_POINTER, DECL_PROTECTED, DECL_PRIVATE,
DECL_STATIC, DECL_AUTOMATIC,
DECL_PUBLIC, DECL_SAVE, DECL_TARGET, DECL_VALUE, DECL_VOLATILE,
@@ -4729,6 +4730,9 @@ match_attr_spec (void)
/* GFC_DECL_END is the sentinel, index starts at 0. */
#define NUM_DECL GFC_DECL_END
+ /* Make sure that values from sym_intent are safe to be used here. */
+ gcc_assert (INTENT_IN > 0);
+
locus start, seen_at[NUM_DECL];
int seen[NUM_DECL];
unsigned int d;
@@ -4846,13 +4850,12 @@ match_attr_spec (void)
if (match_string_p ("nt"))
{
/* Matched "intent". */
- /* TODO: Call match_intent_spec from here. */
- if (gfc_match (" ( in out )") == MATCH_YES)
- d = DECL_INOUT;
- else if (gfc_match (" ( in )") == MATCH_YES)
- d = DECL_IN;
- else if (gfc_match (" ( out )") == MATCH_YES)
- d = DECL_OUT;
+ d = match_intent_spec ();
+ if (d == INTENT_UNKNOWN)
+ {
+ m = MATCH_ERROR;
+ goto cleanup;
+ }
}
}
else if (ch == 'r')
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h (revision 261358)
+++ gcc/fortran/gfortran.h (working copy)
@@ -291,7 +291,8 @@ enum procedure_type
PROC_INTRINSIC, PROC_ST_FUNCTION, PROC_EXTERNAL
};
-/* Intent types. */
+/* Intent types. Note that these values are also used in another enum in
+ decl.c (match_attr_spec). */
enum sym_intent
{ INTENT_UNKNOWN = 0, INTENT_IN, INTENT_OUT, INTENT_INOUT
};
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration
2018-06-10 12:09 ` Janus Weil
@ 2018-06-10 12:25 ` Janus Weil
0 siblings, 0 replies; 4+ messages in thread
From: Janus Weil @ 2018-06-10 12:25 UTC (permalink / raw)
To: Thomas Koenig; +Cc: gfortran, gcc-patches
2018-06-10 9:02 GMT+02:00 Janus Weil <janus@gcc.gnu.org>:
> Hi Thomas,
>
>> I like what the patch does. However, I have one concern.
>>
>>> * decl.c (match_attr_spec): Synchronize the DECL_* enum values with
>>> the
>>> INTENT_* values from the enum 'sym_intent'.
>>
>>
>> This part
>>
>> + { GFC_DECL_BEGIN = 0, DECL_ALLOCATABLE = GFC_DECL_BEGIN,
>> + DECL_IN = INTENT_IN, DECL_OUT = INTENT_OUT, DECL_INOUT = INTENT_INOUT,
>> + DECL_DIMENSION, DECL_EXTERNAL,
>> + DECL_INTRINSIC, DECL_OPTIONAL,
>>
>> brings a dependency of the values of sym_intent into this enum.
>>
>> I know that the order of sym_intent is not likely to change, but I would
>> still prefer if you could add a comment to the sym_intent definition in
>> gfortran.h noting that INTENT_IN cannot be zero, and add a
>>
>> gcc_assert (INTENT_IN > 0);
>>
>> somewhere (which will be optimized away) with an explanatory comment.
>
> good point. I have added the assert and some comments. Updated patch attached.
>
>
>> OK with that change.
>
> Thanks. I'm doing another regtest now and will commit if that goes well ...
No failures observed. Committed as r261386.
Cheers,
Janus
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-06-10 8:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-09 12:13 [Patch, Fortran] PR 85088: improve diagnostic for bad INTENT declaration Janus Weil
2018-06-09 22:31 ` Thomas Koenig
2018-06-10 12:09 ` Janus Weil
2018-06-10 12:25 ` Janus Weil
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).