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
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
 };

Reply via email to