* [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).