Tobias Schlüter - [FORTRAN, fix PR30478] Slightly overhaul ENUM parsing (original) (raw)

This is the mail archive of the gcc-patches@gcc.gnu.orgmailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

From: Tobias Schlüter
To: Fortran List , gcc-patches
Date: Sat, 10 Feb 2007 21:23:41 +0100
Subject: [FORTRAN, fix PR30478] Slightly overhaul ENUM parsing

PR30478 is due to the decision to use the normal variable matching code also when matching ENUMERATORs. This leads to the curious situation that something like
ENUM, BIND(C)
INTEGER :: x
END ENUM
will make the compiler choke much later than necessary. Since our enum handling determines the values of the ENUMERATORs during parsing, and due to unforeseen interactions in the error case, this also leads to the additional complication that the machinery which determines the ENUMERATOR values, gets confused, which explains the invalid memory accesses reported in the PR.This patch fixes this by separating the parsing of ENUMERATOR declarations from that of variable declarations. A possible follow-up is to move the determination of the ENUMERATOR values to the resolution stage. I can by no means remember why I didn't insist that this be done when I reviewed the original patch, the only thing I remember is that I thought that there was a good reason not to (besides laziness). If anybody can think of something, please let me know.The diff is against 4.1 because I couldn't get 4.3 to build on the fast Linux boxes I have access to, due to issues with a GMP upgraded by the sysop. My personal old Linux box is currently working its way through the compile, but it will take until tomorrow.Built and tested on i686-linux, I verified with valgrind that that the enum_* testcases stay clear of memory error. Is this ok for 4.3, 4.2 and 4.1 once I manage to build and test this on all three versions? I can split out the cleanups, and only apply them to 4.3, in case they weren't included in Steve's whitespace works.

Cheers,

Tobi

:ADDPATCH fortran:2007-02-10 Tobias Schl¸ter tobi@gcc.gnu.org

PR fortran/30478

fortran/ * decl.c (create_enum_history, gfc_free_enum_history): Formatting fixes. (add_init_expr_to_sym): Remove ENUM-specific code-path. (variable_decl): Likewise. Formatting fix. (match_attr_spec): Remove ENUM-specific codepath. (gfc_match_enum): Fix typo in error message. (enumerator_decl): New. (gfc_match_enumerator_def): Strip down to code necessary for ENUMs, use enumerator_decl. testsuite/ * gfortran.dg/enum_4.f90: Update expected error message.

2007-02-10 Tobias Schlüter tobi@gcc.gnu.org

PR fortran/30478

Index: gcc/fortran/decl.c

--- gcc/fortran/decl.c (revision 121799) +++ gcc/fortran/decl.c (working copy) @@ -758,7 +758,7 @@ gfc_set_constant_character_len (int len, INIT points to its enumerator value. / static void -create_enum_history(gfc_symbol sym, gfc_expr init) +create_enum_history (gfc_symbol sym, gfc_expr init) { enumerator_history new_enum_history; gcc_assert (sym != NULL && init != NULL); @@ -789,7 +789,7 @@ create_enum_history(gfc_symbol sym, gfc / Function to free enum kind history. / void -gfc_free_enum_history(void) +gfc_free_enum_history (void) { enumerator_history current = enum_history;
enumerator_history next;
@@ -911,10 +911,6 @@ add_init_expr_to_sym (const char name, initp = NULL; } - / Maintain enumerator history. */ - if (gfc_current_state () == COMP_ENUM) - create_enum_history (sym, init);

return SUCCESS; } @@ -1073,14 +1069,6 @@ variable_decl (int elem) if (m == MATCH_NO) as = gfc_copy_array_spec (current_as); - else if (gfc_current_state () == COMP_ENUM) - { - gfc_error ("Enumerator cannot be array at %C"); - gfc_free_enum_history (); - m = MATCH_ERROR; - goto cleanup; - }

char_len = NULL; cl = NULL; @@ -1179,10 +1167,11 @@ variable_decl (int elem) goto cleanup; } - /* An interface body specifies all of the procedure's characteristics and these - shall be consistent with those specified in the procedure definition, except - that the interface may specify a procedure that is not pure if the procedure - is defined to be pure(12.3.2). / + / An interface body specifies all of the procedure's + characteristics and these shall be consistent with those + specified in the procedure definition, except that the interface + may specify a procedure that is not pure if the procedure is + defined to be pure(12.3.2). / if (current_ts.type == BT_DERIVED && gfc_current_ns->proc_name->attr.if_source == IFSRC_IFBODY && current_ts.derived->ns != gfc_current_ns) @@ -1288,30 +1277,6 @@ variable_decl (int elem) } } - / Check if we are parsing an enumeration and if the current enumerator - variable has an initializer or not. If it does not have an - initializer, the initialization value of the previous enumerator - (stored in last_initializer) is incremented by 1 and is used to - initialize the current enumerator. */ - if (gfc_current_state () == COMP_ENUM) - { - if (initializer == NULL) - initializer = gfc_enum_initializer (last_initializer, old_locus);

 if (initializer == NULL || initializer->ts.type != BT_INTEGER)

```
   {
```

     gfc_error("ENUMERATOR %L not initialized with integer expression",

```
      &var_locus);
```
```
     m = MATCH_ERROR; 
```
```
     gfc_free_enum_history ();
```
```
     goto cleanup;
```
```
   }
```

 /* Store this current initializer, for the next enumerator

```
variable to be parsed.  */
```
```
 last_initializer = initializer;
```
}
/* Add the initializer. Note that it is fine if initializer is NULL here, because we sometimes also need to check if a declaration must have an initialization expression. */

@@ -2033,12 +1998,6 @@ match_attr_spec (void) if (d == DECL_NONE || d == DECL_COLON) break;
- if (gfc_current_state () == COMP_ENUM) - { - gfc_error ("Enumerator cannot have attributes %C"); - return MATCH_ERROR; - }

   seen[d]++;
   seen_at[d] = gfc_current_locus;

@@ -2057,18 +2016,6 @@ match_attr_spec (void) } } - /* If we are parsing an enumeration and have ensured that no other - attributes are present we can now set the parameter attribute. */ - if (gfc_current_state () == COMP_ENUM) - { - t = gfc_add_flavor (&current_attr, FL_PARAMETER, NULL, NULL); - if (t == FAILURE) - { - m = MATCH_ERROR; - goto cleanup; - } - }

/* No double colon, so assume that we've been looking at something else the whole time. */ if (d == DECL_NONE) @@ -4081,7 +4028,7 @@ gfc_match_enum (void) return m;

if (gfc_notify_std (GFC_STD_F2003, - "New in Fortran 2003: ENUM AND ENUMERATOR at %C") + "New in Fortran 2003: ENUM and ENUMERATOR at %C") == FAILURE) return MATCH_ERROR;

@@ -4089,19 +4036,116 @@ gfc_match_enum (void) }

+/* Match a variable name with an optional initializer. When this

subroutine is called, a variable is expected to be parsed next.
Depending on what is happening at the moment, updates either the
symbol table or the current interface. */
+static match +enumerator_decl (void) +{
char name[GFC_MAX_SYMBOL_LEN + 1];
gfc_expr *initializer;
gfc_array_spec *as = NULL;
gfc_symbol *sym;
locus var_locus;
match m;
try t;
locus old_locus;
initializer = NULL;
old_locus = gfc_current_locus;
/* When we get here, we've just matched a list of attributes and

maybe a type and a double colon.  The next thing we expect to see

```
is the name of the symbol.  */
```
m = gfc_match_name (name);
if (m != MATCH_YES)
goto cleanup;
var_locus = gfc_current_locus;
/* OK, we've successfully matched the declaration. Now put the

symbol in the current namespace. If we fail to create the symbol,

```
bail out.  */
```
if (build_sym (name, NULL, &as, &var_locus) == FAILURE)
{
```
 m = MATCH_ERROR;
```
```
 goto cleanup;
```
}
/* The double colon must be present in order to have initializers.

Otherwise the statement is ambiguous with an assignment statement.  */

if (colon_seen)
{
```
 if (gfc_match_char ('=') == MATCH_YES)
```
{

 m = gfc_match_init_expr (&initializer);

```
 if (m == MATCH_NO)
```
```
   {
```

     gfc_error ("Expected an initialization expression at %C");

```
     m = MATCH_ERROR;
```
```
   }
```
```
 if (m != MATCH_YES)
```
```
   goto cleanup;
```
}
}
/* If we do not have an initializer, the initialization value of the

previous enumerator (stored in last_initializer) is incremented

by 1 and is used to initialize the current enumerator.  */

if (initializer == NULL)
initializer = gfc_enum_initializer (last_initializer, old_locus);
if (initializer == NULL || initializer->ts.type != BT_INTEGER)
{

 gfc_error("ENUMERATOR %L not initialized with integer expression",

```
  &var_locus);
```
```
 m = MATCH_ERROR; 
```
```
 gfc_free_enum_history ();
```
```
 goto cleanup;
```
}
/* Store this current initializer, for the next enumerator variable

to be parsed.  add_init_expr_to_sym() zeros initializer, so we

```
use last_initializer below.  */
```
last_initializer = initializer;
t = add_init_expr_to_sym (name, &initializer, &var_locus);
/* Maintain enumerator history. */
gfc_find_symbol (name, NULL, 0, &sym);
create_enum_history (sym, last_initializer);
return (t == SUCCESS) ? MATCH_YES : MATCH_ERROR;
+cleanup:
/* Free stuff up and return. */
gfc_free_expr (initializer);
return m; +}
/* Match the enumerator definition statement. */

match gfc_match_enumerator_def (void) { match m;

int elem;

try t;

gfc_clear_ts (&current_ts);

m = gfc_match (" enumerator"); if (m != MATCH_YES) return m;
m = gfc_match (" :: ");
if (m == MATCH_ERROR)
return m;
colon_seen = (m == MATCH_YES);

if (gfc_current_state () != COMP_ENUM) {

@@ -4113,17 +4157,17 @@ gfc_match_enumerator_def (void) (&current_ts)->type = BT_INTEGER; (&current_ts)->kind = gfc_c_int_kind;

m = match_attr_spec ();
if (m == MATCH_ERROR)

gfc_clear_attr (&current_attr);
t = gfc_add_flavor (&current_attr, FL_PARAMETER, NULL, NULL);
if (t == FAILURE) {

```
 m = MATCH_NO;
```

```
 m = MATCH_ERROR;
 goto cleanup;
```
}

elem = 1; for (;;) {
```
 m = variable_decl (elem++);
```

 m = enumerator_decl ();
 if (m == MATCH_ERROR)

goto cleanup; if (m == MATCH_NO)

Index: gcc/testsuite/gfortran.dg/enum_4.f90

--- gcc/testsuite/gfortran.dg/enum_4.f90 (revision 121799) +++ gcc/testsuite/gfortran.dg/enum_4.f90 (working copy) @@ -5,12 +5,12 @@ program main implicit none enum, bind (c) enumerator :: red, black = 2

enumerator :: blue = 1, red ! { dg-error "already" }

enumerator :: blue = 1, red ! { dg-error "already has basic type" }

end enum

enum, bind (c)

enumerator :: r, b(10) = 2 ! { dg-error "cannot be array" }
enumerator , save :: g = 1 ! { dg-error "cannot have attributes" }

enumerator :: r, b(10) = 2 ! { dg-error "Syntax error" }
enumerator , save :: g = 1 ! { dg-error "Syntax error" }

end ! { dg-error " END ENUM" }

end program main ! { dg-excess-errors "" }

Follow-Ups:
- Re: [FORTRAN, fix PR30478] Slightly overhaul ENUM parsing
  * From: Steve Kargl

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]

Tobias Schlüter - [FORTRAN, fix PR30478] Slightly overhaul ENUM parsing (original) (raw)

Index: gcc/fortran/decl.c

return SUCCESS; } @@ -1073,14 +1069,6 @@ variable_decl (int elem) if (m == MATCH_NO) as = gfc_copy_array_spec (current_as); - else if (gfc_current_state () == COMP_ENUM) - { - gfc_error ("Enumerator cannot be array at %C"); - gfc_free_enum_history (); - m = MATCH_ERROR; - goto cleanup; - }

@@ -2033,12 +1998,6 @@ match_attr_spec (void) if (d == DECL_NONE || d == DECL_COLON) break; - if (gfc_current_state () == COMP_ENUM) - { - gfc_error ("Enumerator cannot have attributes %C"); - return MATCH_ERROR; - }

Index: gcc/testsuite/gfortran.dg/enum_4.f90

@@ -2033,12 +1998,6 @@ match_attr_spec (void) if (d == DECL_NONE || d == DECL_COLON) break;
- if (gfc_current_state () == COMP_ENUM) - { - gfc_error ("Enumerator cannot have attributes %C"); - return MATCH_ERROR; - }