* [PATCH] Better error reporting for missing semicolons after a struct definition
@ 2010-10-29 16:44 Paolo Bonzini
2010-11-05 18:04 ` Joseph S. Myers
0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2010-10-29 16:44 UTC (permalink / raw)
To: gcc-patches
Currently, when a struct, enum or union declaration lacks a terminating
semicolon, GCC usually goes on parsing what follows as a declspec and
prints the infamous "two or more data types in declaration specifiers"
error message. This patch series instead makes GCC treat the declaration
separately if it includes a tagged type definition, and print a clearer
"expected ';', identifier or '('" error message.
An additional benefit is that when presented with something like
struct f {}
int a;
The variables will be given type "int" in the compiler instead of
"struct f", thus removing further cascading errors.
It should be possible to do something like this for C++ too, but I'm
not attempting it right now. This patch depends on the -Wc++-compat
patch I posted because it also uses the typespec_kind field of struct
c_declspecs.
Bootstrapped and regtested x86_64-pc-linux-gnu, okay for mainline?
Paolo
2010-10-28 Paolo Bonzini <bonzini@gnu.org>
* c-parser.c (c_token_is_qualifier,
c_parser_next_token_is_qualifier): New.
(c_parser_declaration_or_fndef, c_parser_struct_declaration):
Improve error message on specs->tagdef_seen_p.
(c_parser_struct_or_union_specifier): Improve error recovery.
(c_parser_declspecs): Move exit condition on C_ID_ID early.
Reorganize exit condition for C_ID_TYPENAME/C_ID_CLASSNAME
using c_parser_next_token_is_qualifier, and extend it to
cover !typespec_ok in general, and also specs->tagdef_seen_p.
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c (branch diag-2)
+++ gcc/c-parser.c (working copy)
@@ -506,6 +506,47 @@ c_parser_next_token_starts_typename (c_p
return c_token_starts_typename (token);
}
+/* Return true if TOKEN is a type qualifier, false otherwise. */
+static bool
+c_token_is_qualifier (c_token *token)
+{
+ switch (token->type)
+ {
+ case CPP_NAME:
+ switch (token->id_kind)
+ {
+ case C_ID_ADDRSPACE:
+ return true;
+ default:
+ return false;
+ }
+ case CPP_KEYWORD:
+ switch (token->keyword)
+ {
+ case RID_CONST:
+ case RID_VOLATILE:
+ case RID_RESTRICT:
+ case RID_ATTRIBUTE:
+ return true;
+ default:
+ return false;
+ }
+ case CPP_LESS:
+ return false;
+ default:
+ gcc_unreachable ();
+ }
+}
+
+/* Return true if the next token from PARSER is a type qualifier,
+ false otherwise. */
+static inline bool
+c_parser_next_token_is_qualifier (c_parser *parser)
+{
+ c_token *token = c_parser_peek_token (parser);
+ return c_token_is_qualifier (token);
+}
+
/* Return true if TOKEN can start declaration specifiers, false
otherwise. */
static bool
@@ -1349,6 +1390,19 @@ c_parser_declaration_or_fndef (c_parser
c_parser_consume_token (parser);
return;
}
+
+ /* Provide better error recovery. Note that a type name here is usually
+ better diagnosed as a redeclaration. */
+ if (empty_ok
+ && specs->typespec_kind == ctsk_tagdef
+ && c_parser_next_token_starts_declspecs (parser)
+ && !c_parser_next_token_is (parser, CPP_NAME))
+ {
+ c_parser_error (parser, "expected %<;%>, identifier or %<(%>");
+ parser->error = false;
+ shadow_tag_warned (specs, 1);
+ return;
+ }
else if (c_dialect_objc ())
{
/* Prefix attributes are an error on method decls. */
@@ -1812,13 +1866,28 @@ c_parser_declspecs (c_parser *parser, st
{
bool attrs_ok = start_attr_ok;
bool seen_type = (specs->typespec_kind != ctsk_none);
- while (c_parser_next_token_is (parser, CPP_NAME)
+ while ((c_parser_next_token_is (parser, CPP_NAME)
+ && c_parser_peek_token (parser)->id_kind != C_ID_ID)
|| c_parser_next_token_is (parser, CPP_KEYWORD)
|| (c_dialect_objc () && c_parser_next_token_is (parser, CPP_LESS)))
{
struct c_typespec t;
tree attrs;
location_t loc = c_parser_peek_token (parser)->location;
+
+ /* If we already have seen a tagged definition, or cannot accept a
+ type, and the next token must start a new type, exit. Exit for
+ TYPENAMEs after any type because they can appear as a field name. */
+ if (!c_parser_next_token_is_qualifier (parser))
+ {
+ if (seen_type && c_parser_next_token_is (parser, CPP_NAME))
+ break;
+
+ if ((!typespec_ok || specs->typespec_kind == ctsk_tagdef)
+ && c_parser_next_token_starts_typename (parser))
+ break;
+ }
+
if (c_parser_next_token_is (parser, CPP_NAME))
{
tree value = c_parser_peek_token (parser)->value;
@@ -1834,12 +1903,7 @@ c_parser_declspecs (c_parser *parser, st
continue;
}
- /* This finishes the specifiers unless a type name is OK, it
- is declared as a type name and a type name hasn't yet
- been seen. */
- if (!typespec_ok || seen_type
- || (kind != C_ID_TYPENAME && kind != C_ID_CLASSNAME))
- break;
+ /* Now at a C_ID_TYPENAME or C_ID_CLASSNAME. */
c_parser_consume_token (parser);
seen_type = true;
attrs_ok = true;
@@ -1857,7 +1921,7 @@ c_parser_declspecs (c_parser *parser, st
else
{
tree proto = NULL_TREE;
- gcc_assert (c_dialect_objc ());
+ gcc_assert (kind == C_ID_CLASSNAME && c_dialect_objc ());
t.kind = ctsk_objc;
if (c_parser_next_token_is (parser, CPP_LESS))
proto = c_parser_objc_protocol_refs (parser);
@@ -2280,12 +2344,17 @@ c_parser_struct_or_union_specifier (c_pa
if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
pedwarn (c_parser_peek_token (parser)->location, 0,
"no semicolon at end of struct or union");
- else
+ else if (parser->error
+ || !c_parser_next_token_starts_declspecs (parser))
{
c_parser_error (parser, "expected %<;%>");
c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, NULL);
break;
}
+
+ /* If we come here, we have already emitted an error
+ for an expected `;', identifier or `(', and we also
+ recovered already. Go on with the next field. */
}
}
postfix_attrs = c_parser_attributes (parser);
@@ -2400,6 +2469,21 @@ c_parser_struct_declaration (c_parser *p
}
return ret;
}
+
+ /* Provide better error recovery. Note that a type name here is valid,
+ and will be treated as a field name. */
+ if (specs->typespec_kind == ctsk_tagdef
+ && TREE_CODE (specs->type) != ENUMERAL_TYPE
+ && c_parser_next_token_starts_declspecs (parser)
+ && !c_parser_next_token_is (parser, CPP_NAME))
+ {
+ c_parser_error (parser, "expected %<;%>, identifier or %<(%>");
+ parser->error = false;
+ return NULL_TREE;
+ }
+ if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
+ return NULL_TREE;
+
pending_xref_error ();
prefix_attrs = specs->attrs;
all_prefix_attrs = prefix_attrs;
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
typedef int x, y;
x y z; /* { dg-error "" "" } */
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
typedef int x, y;
x struct f z; /* { dg-error "two types " "" } */
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
struct f {
}
int z(); /* { dg-error "expected ';', identifier or " "" { target *-*-* } } */
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
int f()
{
struct f {
}
int z; /* { dg-error "expected ';', identifier or " "" } */
}
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
struct f {}
struct g {} /* { dg-error "expected ';', identifier or " "" } */
int f(); /* { dg-error "expected ';', identifier or " "" } */
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
struct s {
struct f {} /* dg-warning "does not declare anything" "" } */
struct g {} x; /* { dg-error "expected ';', identifier or " "" } */
};
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
struct s {
struct f {}
enum a { X } /* { dg-error "expected ';', identifier or " "" } */
struct g {} /* { dg-error "expected identifier " "" } */
}; /* { dg-warning "no semicolon" "" } */
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
enum x { XYZ }
struct g { enum x a; }; /* { dg-error "expected ';', identifier or " "" } */
int f(struct g *x)
{
return x->a == XYZ; /* { dg-bogus " has no member " "" } */
}
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
struct f {}
static int a, b; /* { dg-error "expected ';', identifier or " "" } */
int f()
{
return a - b; /* { dg-bogus "invalid operands " "" } */
}
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
typedef enum a { XYZ } a; /* { dg-message "previous declaration" } */
enum a a; /* { dg-error "redeclared" } */
struct b { enum a a : 8; };
/* { dg-do compile } */
/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
@interface foo
struct f {}
struct g { int a; }; /* { dg-error "expected ';', identifier or " "" } */
- (struct f *) a;
- (struct g *) b;
@end
int f(struct g *x)
{
return x->a; /* { dg-bogus " has no member " "" } */
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Better error reporting for missing semicolons after a struct definition
2010-10-29 16:44 [PATCH] Better error reporting for missing semicolons after a struct definition Paolo Bonzini
@ 2010-11-05 18:04 ` Joseph S. Myers
2010-11-08 10:20 ` Paolo Bonzini
2010-11-09 16:44 ` Paolo Bonzini
0 siblings, 2 replies; 8+ messages in thread
From: Joseph S. Myers @ 2010-11-05 18:04 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: gcc-patches
On Fri, 29 Oct 2010, Paolo Bonzini wrote:
> @@ -2400,6 +2469,21 @@ c_parser_struct_declaration (c_parser *p
> }
> return ret;
> }
> +
> + /* Provide better error recovery. Note that a type name here is valid,
> + and will be treated as a field name. */
> + if (specs->typespec_kind == ctsk_tagdef
> + && TREE_CODE (specs->type) != ENUMERAL_TYPE
> + && c_parser_next_token_starts_declspecs (parser)
> + && !c_parser_next_token_is (parser, CPP_NAME))
> + {
> + c_parser_error (parser, "expected %<;%>, identifier or %<(%>");
> + parser->error = false;
> + return NULL_TREE;
> + }
> + if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
> + return NULL_TREE;
I don't follow why you are returning NULL_TREE here (or why it is OK to do
so). It appears to me that it will quietly return NULL_TREE if you have
declaration specifiers with no following declarator or semicolon.
Consider the following testcase.
struct s { struct { int a; } };
int *f(struct s *p) { return &p->a; }
Your patch doesn't apply cleanly to the sources I have, either because of
subsequent changes or because of missing prerequisites, but it appears to
me that with your patch the parser will accept the struct s declaration
(diagnosing the missing semicolon at end of structure with a pedwarn) but
not actually do anything with the anonymous structure element because the
code above only handles anonymous structs with following semicolons. The
current compiler gives parse errors on this code. The *correct* behavior
I think is neither - it should be accepted with the anonymous structure
processed (so the check above should allow for CPP_CLOSE_BRACE as well as
CPP_SEMICOLON), like 4.0 and earlier did with the old parser.
Otherwise the changes in this patch look correct.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Better error reporting for missing semicolons after a struct definition
2010-11-05 18:04 ` Joseph S. Myers
@ 2010-11-08 10:20 ` Paolo Bonzini
2010-11-09 16:44 ` Paolo Bonzini
1 sibling, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2010-11-08 10:20 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: gcc-patches
On 11/05/2010 07:04 PM, Joseph S. Myers wrote:
> struct s { struct { int a; } };
> int *f(struct s *p) { return &p->a; }
>
> The *correct* behavior I think is neither - it should be accepted
> with the anonymous structure processed (so the check above should
> allow for CPP_CLOSE_BRACE as well as CPP_SEMICOLON), like 4.0 and
> earlier did with the old parser.
I agree, I'll take a look at this testcase.
Thanks for the review!
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Better error reporting for missing semicolons after a struct definition
2010-11-05 18:04 ` Joseph S. Myers
2010-11-08 10:20 ` Paolo Bonzini
@ 2010-11-09 16:44 ` Paolo Bonzini
2010-11-10 2:13 ` Joseph S. Myers
1 sibling, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2010-11-09 16:44 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 603 bytes --]
On 11/05/2010 07:04 PM, Joseph S. Myers wrote:
> Consider the following testcase.
>
> struct s { struct { int a; } };
> int *f(struct s *p) { return &p->a; }
The attached patch (mostly untested) fixes this testcase. This will also pedwarn:
struct t { int };
like this:
f.c:3:16: warning: declaration does not declare anything [enabled by default]
f.c:3:16: warning: no semicolon at end of struct or union [enabled by default]
while GCC 4.5 gives
f.c:3:16: error: expected identifier or ‘(’ before ‘}’ token
f.c:3:16: error: expected specifier-qualifier-list at end of input
Paolo
[-- Attachment #2: c-parser-corner-case.patch --]
[-- Type: text/plain, Size: 6333 bytes --]
fix corner case of unnamed struct and missing final semicolon
* c-parser.c (enum c_dtr_syn): Change to a 3-bit mask.
(c_parser_declarator, c_parser_direct_declarator): Adjust tests, change
c_dtr_syn argument to int.
(c_parser_struct_declaration): Remove wrong code from previous patch.
Accept an abstract declarator, usually an unnamed struct, and do not
crash when other kinds of abstract declarator are found in a struct.
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c (branch diag-2)
+++ gcc/c-parser.c (working copy)
@@ -1030,9 +1030,9 @@ restore_extension_diagnostics (int flags
/* Possibly kinds of declarator to parse. */
typedef enum c_dtr_syn {
/* A normal declarator with an identifier. */
- C_DTR_NORMAL,
+ C_DTR_NORMAL = 1,
/* An abstract declarator (maybe empty). */
- C_DTR_ABSTRACT,
+ C_DTR_ABSTRACT = 2,
/* A parameter declarator: may be either, but after a type name does
not redeclare a typedef name as an identifier if it can
alternatively be interpreted as a typedef name; see DR#009,
@@ -1043,7 +1043,7 @@ typedef enum c_dtr_syn {
abstract declarators rather than involving redundant parentheses;
the same applies with attributes inside the parentheses before
"T". */
- C_DTR_PARM
+ C_DTR_PARM = 4
} c_dtr_syn;
static void c_parser_external_declaration (c_parser *);
@@ -1058,10 +1058,10 @@ static struct c_typespec c_parser_enum_s
static struct c_typespec c_parser_struct_or_union_specifier (c_parser *);
static tree c_parser_struct_declaration (c_parser *);
static struct c_typespec c_parser_typeof_specifier (c_parser *);
-static struct c_declarator *c_parser_declarator (c_parser *, bool, c_dtr_syn,
+static struct c_declarator *c_parser_declarator (c_parser *, bool, int,
bool *);
static struct c_declarator *c_parser_direct_declarator (c_parser *, bool,
- c_dtr_syn, bool *);
+ int, bool *);
static struct c_declarator *c_parser_direct_declarator_inner (c_parser *,
bool,
struct c_declarator *);
@@ -2516,8 +2516,6 @@ c_parser_struct_declaration (c_parser *p
parser->error = false;
return NULL_TREE;
}
- if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
- return NULL_TREE;
pending_xref_error ();
prefix_attrs = specs->attrs;
@@ -2526,7 +2524,9 @@ c_parser_struct_declaration (c_parser *p
decls = NULL_TREE;
while (true)
{
- /* Declaring one or more declarators or un-named bit-fields. */
+ /* Declaring one or more declarators or un-named bit-fields. The
+ identifier in a declarator is optional, since we could have an
+ unnamed struct. */
struct c_declarator *declarator;
bool dummy = false;
if (c_parser_next_token_is (parser, CPP_COLON))
@@ -2534,7 +2534,7 @@ c_parser_struct_declaration (c_parser *p
else
declarator = c_parser_declarator (parser,
specs->typespec_kind != ctsk_none,
- C_DTR_NORMAL, &dummy);
+ C_DTR_NORMAL|C_DTR_ABSTRACT, &dummy);
if (declarator == NULL)
{
c_parser_skip_to_end_of_block_or_statement (parser);
@@ -2558,10 +2558,13 @@ c_parser_struct_declaration (c_parser *p
postfix_attrs = c_parser_attributes (parser);
d = grokfield (c_parser_peek_token (parser)->location,
declarator, specs, width, &all_prefix_attrs);
- decl_attributes (&d, chainon (postfix_attrs,
- all_prefix_attrs), 0);
- DECL_CHAIN (d) = decls;
- decls = d;
+ if (d)
+ {
+ decl_attributes (&d, chainon (postfix_attrs,
+ all_prefix_attrs), 0);
+ DECL_CHAIN (d) = decls;
+ decls = d;
+ }
if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
all_prefix_attrs = chainon (c_parser_attributes (parser),
prefix_attrs);
@@ -2734,7 +2737,7 @@ c_parser_typeof_specifier (c_parser *par
an abstract declarator, although not part of the formal syntax. */
static struct c_declarator *
-c_parser_declarator (c_parser *parser, bool type_seen_p, c_dtr_syn kind,
+c_parser_declarator (c_parser *parser, bool type_seen_p, int kind,
bool *seen_id)
{
/* Parse any initial pointer part. */
@@ -2759,7 +2762,7 @@ c_parser_declarator (c_parser *parser, b
as c_parser_declarator. */
static struct c_declarator *
-c_parser_direct_declarator (c_parser *parser, bool type_seen_p, c_dtr_syn kind,
+c_parser_direct_declarator (c_parser *parser, bool type_seen_p, int kind,
bool *seen_id)
{
/* The direct declarator must start with an identifier (possibly
@@ -2796,7 +2799,7 @@ c_parser_direct_declarator (c_parser *pa
??? Also following the old parser, typedef names may be
redeclared in declarators, but not Objective-C class names. */
- if (kind != C_DTR_ABSTRACT
+ if ((kind & ~C_DTR_ABSTRACT)
&& c_parser_next_token_is (parser, CPP_NAME)
&& ((type_seen_p
&& (c_parser_peek_token (parser)->id_kind == C_ID_TYPENAME
@@ -2811,7 +2814,7 @@ c_parser_direct_declarator (c_parser *pa
return c_parser_direct_declarator_inner (parser, *seen_id, inner);
}
- if (kind != C_DTR_NORMAL
+ if ((kind & ~C_DTR_NORMAL)
&& c_parser_next_token_is (parser, CPP_OPEN_SQUARE))
{
struct c_declarator *inner = build_id_declarator (NULL_TREE);
@@ -2827,13 +2830,12 @@ c_parser_direct_declarator (c_parser *pa
struct c_declarator *inner;
c_parser_consume_token (parser);
attrs = c_parser_attributes (parser);
- if (kind != C_DTR_NORMAL
+ if ((kind & ~C_DTR_NORMAL)
&& (c_parser_next_token_starts_declspecs (parser)
|| c_parser_next_token_is (parser, CPP_CLOSE_PAREN)))
{
struct c_arg_info *args
- = c_parser_parms_declarator (parser, kind == C_DTR_NORMAL,
- attrs);
+ = c_parser_parms_declarator (parser, false, attrs);
if (args == NULL)
return NULL;
else
@@ -2866,13 +2868,13 @@ c_parser_direct_declarator (c_parser *pa
}
else
{
- if (kind == C_DTR_NORMAL)
+ if (kind & ~C_DTR_NORMAL)
+ return build_id_declarator (NULL_TREE);
+ else
{
c_parser_error (parser, "expected identifier or %<(%>");
return NULL;
}
- else
- return build_id_declarator (NULL_TREE);
}
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Better error reporting for missing semicolons after a struct definition
2010-11-09 16:44 ` Paolo Bonzini
@ 2010-11-10 2:13 ` Joseph S. Myers
2010-11-10 10:10 ` Paolo Bonzini
2010-11-13 11:18 ` Paolo Bonzini
0 siblings, 2 replies; 8+ messages in thread
From: Joseph S. Myers @ 2010-11-10 2:13 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: gcc-patches
On Tue, 9 Nov 2010, Paolo Bonzini wrote:
> On 11/05/2010 07:04 PM, Joseph S. Myers wrote:
> > Consider the following testcase.
> >
> > struct s { struct { int a; } };
> > int *f(struct s *p) { return &p->a; }
>
> The attached patch (mostly untested) fixes this testcase. This will also
> pedwarn:
That patch seems much more complicated than I expected and I don't see
why most of the changes in it should be needed. I've applied this
patch (bootstrapped with no regressions on x86_64-unknown-linux-gnu)
to fix the testcase I gave in what seems to me to be the natural way.
Perhaps you could post an updated version of your patch to improve
diagnostics for missing semicolons, relative to current trunk?
2010-11-09 Joseph Myers <joseph@codesourcery.com>
* c-parser.c (c_parser_struct_declaration): Handle declaration
specifiers followed by CPP_CLOSE_BRACE.
testsuite:
2010-11-09 Joseph Myers <joseph@codesourcery.com>
* gcc.dg/struct-semi-4.c: New test.
Index: gcc/testsuite/gcc.dg/struct-semi-4.c
===================================================================
--- gcc/testsuite/gcc.dg/struct-semi-4.c (revision 0)
+++ gcc/testsuite/gcc.dg/struct-semi-4.c (revision 0)
@@ -0,0 +1,7 @@
+/* Test for missing semicolons in structures: anonymous structures and
+ similar cases. */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+struct s { struct { int a; } }; /* { dg-warning "no semicolon" } */
+int *f (struct s *p) { return &p->a; }
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c (revision 166433)
+++ gcc/c-parser.c (working copy)
@@ -2395,7 +2395,8 @@ c_parser_struct_declaration (c_parser *p
return NULL_TREE;
}
finish_declspecs (specs);
- if (c_parser_next_token_is (parser, CPP_SEMICOLON))
+ if (c_parser_next_token_is (parser, CPP_SEMICOLON)
+ || c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
{
tree ret;
if (!specs->type_seen_p)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Better error reporting for missing semicolons after a struct definition
2010-11-10 2:13 ` Joseph S. Myers
@ 2010-11-10 10:10 ` Paolo Bonzini
2010-11-13 11:18 ` Paolo Bonzini
1 sibling, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2010-11-10 10:10 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: gcc-patches
On 11/10/2010 03:05 AM, Joseph S. Myers wrote:
> Perhaps you could post an updated version of your patch to improve
> diagnostics for missing semicolons, relative to current trunk?
Yes, I'll commit the c++-compat patch first so that there will be no
prerequisite.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Better error reporting for missing semicolons after a struct definition
2010-11-10 2:13 ` Joseph S. Myers
2010-11-10 10:10 ` Paolo Bonzini
@ 2010-11-13 11:18 ` Paolo Bonzini
2010-11-15 16:49 ` Joseph S. Myers
1 sibling, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2010-11-13 11:18 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 232 bytes --]
On 11/10/2010 03:05 AM, Joseph S. Myers wrote:
> Perhaps you could post an updated version of your patch to improve
> diagnostics for missing semicolons, relative to current trunk?
Here it is, bootstrap/regtest in progress.
Paolo
[-- Attachment #2: semi.patch --]
[-- Type: text/plain, Size: 10942 bytes --]
2010-11-13 Paolo Bonzini <bonzini@gnu.org>
* c-parser.c (c_token_is_qualifier,
c_parser_next_token_is_qualifier): New.
(c_parser_declaration_or_fndef, c_parser_struct_declaration):
Improve error message on ctsk_tagdef typespecs.
(c_parser_struct_or_union_specifier): Improve error recovery.
2010-11-13 Paolo Bonzini <bonzini@gnu.org>
* gcc.dg/two-types-1.c: New test.
* gcc.dg/two-types-2.c: New test.
* gcc.dg/two-types-3.c: New test.
* gcc.dg/two-types-4.c: New test.
* gcc.dg/two-types-5.c: New test.
* gcc.dg/two-types-6.c: New test.
* gcc.dg/two-types-7.c: New test.
* gcc.dg/two-types-8.c: New test.
* gcc.dg/two-types-9.c: New test.
* gcc.dg/two-types-10.c: New test.
* objc.dg/two-types-1.m: New test.
Index: c-parser.c
===================================================================
--- c-parser.c (revision 166700)
+++ c-parser.c (working copy)
@@ -506,6 +506,47 @@ c_parser_next_token_starts_typename (c_p
return c_token_starts_typename (token);
}
+/* Return true if TOKEN is a type qualifier, false otherwise. */
+static bool
+c_token_is_qualifier (c_token *token)
+{
+ switch (token->type)
+ {
+ case CPP_NAME:
+ switch (token->id_kind)
+ {
+ case C_ID_ADDRSPACE:
+ return true;
+ default:
+ return false;
+ }
+ case CPP_KEYWORD:
+ switch (token->keyword)
+ {
+ case RID_CONST:
+ case RID_VOLATILE:
+ case RID_RESTRICT:
+ case RID_ATTRIBUTE:
+ return true;
+ default:
+ return false;
+ }
+ case CPP_LESS:
+ return false;
+ default:
+ gcc_unreachable ();
+ }
+}
+
+/* Return true if the next token from PARSER is a type qualifier,
+ false otherwise. */
+static inline bool
+c_parser_next_token_is_qualifier (c_parser *parser)
+{
+ c_token *token = c_parser_peek_token (parser);
+ return c_token_is_qualifier (token);
+}
+
/* Return true if TOKEN can start declaration specifiers, false
otherwise. */
static bool
@@ -1404,6 +1445,19 @@ c_parser_declaration_or_fndef (c_parser
c_parser_consume_token (parser);
return;
}
+
+ /* Provide better error recovery. Note that a type name here is usually
+ better diagnosed as a redeclaration. */
+ if (empty_ok
+ && specs->typespec_kind == ctsk_tagdef
+ && c_parser_next_token_starts_declspecs (parser)
+ && !c_parser_next_token_is (parser, CPP_NAME))
+ {
+ c_parser_error (parser, "expected %<;%>, identifier or %<(%>");
+ parser->error = false;
+ shadow_tag_warned (specs, 1);
+ return;
+ }
else if (c_dialect_objc ())
{
/* Prefix attributes are an error on method decls. */
@@ -1867,13 +1921,31 @@ c_parser_declspecs (c_parser *parser, st
{
bool attrs_ok = start_attr_ok;
bool seen_type = specs->typespec_kind != ctsk_none;
- while (c_parser_next_token_is (parser, CPP_NAME)
+ while ((c_parser_next_token_is (parser, CPP_NAME)
+ && c_parser_peek_token (parser)->id_kind != C_ID_ID)
|| c_parser_next_token_is (parser, CPP_KEYWORD)
|| (c_dialect_objc () && c_parser_next_token_is (parser, CPP_LESS)))
{
struct c_typespec t;
tree attrs;
location_t loc = c_parser_peek_token (parser)->location;
+
+ if (!c_parser_next_token_is_qualifier (parser))
+ {
+ /* Exit for TYPENAMEs after any type because they can appear as a
+ field name. */
+ if (seen_type && c_parser_next_token_is (parser, CPP_NAME))
+ break;
+
+ /* If we cannot accept a type, and the next token must start one,
+ exit. Do the same if we already have seen a tagged definition,
+ since it would be an error anyway and likely the user has simply
+ forgotten a semicolon. */
+ if ((!typespec_ok || specs->typespec_kind == ctsk_tagdef)
+ && c_parser_next_token_starts_typename (parser))
+ break;
+ }
+
if (c_parser_next_token_is (parser, CPP_NAME))
{
tree value = c_parser_peek_token (parser)->value;
@@ -1889,12 +1961,7 @@ c_parser_declspecs (c_parser *parser, st
continue;
}
- /* This finishes the specifiers unless a type name is OK, it
- is declared as a type name and a type name hasn't yet
- been seen. */
- if (!typespec_ok || seen_type
- || (kind != C_ID_TYPENAME && kind != C_ID_CLASSNAME))
- break;
+ /* Now at a C_ID_TYPENAME or C_ID_CLASSNAME. */
c_parser_consume_token (parser);
seen_type = true;
attrs_ok = true;
@@ -2335,12 +2402,17 @@ c_parser_struct_or_union_specifier (c_pa
if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
pedwarn (c_parser_peek_token (parser)->location, 0,
"no semicolon at end of struct or union");
- else
+ else if (parser->error
+ || !c_parser_next_token_starts_declspecs (parser))
{
c_parser_error (parser, "expected %<;%>");
c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, NULL);
break;
}
+
+ /* If we come here, we have already emitted an error
+ for an expected `;', identifier or `(', and we also
+ recovered already. Go on with the next field. */
}
}
postfix_attrs = c_parser_attributes (parser);
@@ -2456,6 +2528,19 @@ c_parser_struct_declaration (c_parser *p
}
return ret;
}
+
+ /* Provide better error recovery. Note that a type name here is valid,
+ and will be treated as a field name. */
+ if (specs->typespec_kind == ctsk_tagdef
+ && TREE_CODE (specs->type) != ENUMERAL_TYPE
+ && c_parser_next_token_starts_declspecs (parser)
+ && !c_parser_next_token_is (parser, CPP_NAME))
+ {
+ c_parser_error (parser, "expected %<;%>, identifier or %<(%>");
+ parser->error = false;
+ return NULL_TREE;
+ }
+
pending_xref_error ();
prefix_attrs = specs->attrs;
all_prefix_attrs = prefix_attrs;
Index: testsuite/gcc.dg/two-types-4.c
===================================================================
--- testsuite/gcc.dg/two-types-4.c (revision 0)
+++ testsuite/gcc.dg/two-types-4.c (revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+int f()
+{
+ struct f {
+ }
+ int z; /* { dg-error "expected ';', identifier or " "" } */
+}
Index: testsuite/gcc.dg/two-types-8.c
===================================================================
--- testsuite/gcc.dg/two-types-8.c (revision 0)
+++ testsuite/gcc.dg/two-types-8.c (revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+enum x { XYZ }
+struct g { enum x a; }; /* { dg-error "expected ';', identifier or " "" } */
+
+int f(struct g *x)
+{
+ return x->a == XYZ; /* { dg-bogus " has no member " "" } */
+}
Index: testsuite/gcc.dg/two-types-1.c
===================================================================
--- testsuite/gcc.dg/two-types-1.c (revision 0)
+++ testsuite/gcc.dg/two-types-1.c (revision 0)
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+typedef int x, y;
+x y z; /* { dg-error "" "" } */
Index: testsuite/gcc.dg/two-types-5.c
===================================================================
--- testsuite/gcc.dg/two-types-5.c (revision 0)
+++ testsuite/gcc.dg/two-types-5.c (revision 0)
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct f {}
+struct g {} /* { dg-error "expected ';', identifier or " "" } */
+int f(); /* { dg-error "expected ';', identifier or " "" } */
Index: testsuite/gcc.dg/two-types-9.c
===================================================================
--- testsuite/gcc.dg/two-types-9.c (revision 0)
+++ testsuite/gcc.dg/two-types-9.c (revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct f {}
+static int a, b; /* { dg-error "expected ';', identifier or " "" } */
+
+int f()
+{
+ return a - b; /* { dg-bogus "invalid operands " "" } */
+}
Index: testsuite/gcc.dg/two-types-10.c
===================================================================
--- testsuite/gcc.dg/two-types-10.c (revision 0)
+++ testsuite/gcc.dg/two-types-10.c (revision 0)
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+typedef enum a { XYZ } a; /* { dg-message "previous declaration" } */
+enum a a; /* { dg-error "redeclared" } */
+struct b { enum a a : 8; };
Index: testsuite/gcc.dg/two-types-2.c
===================================================================
--- testsuite/gcc.dg/two-types-2.c (revision 0)
+++ testsuite/gcc.dg/two-types-2.c (revision 0)
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+typedef int x, y;
+x struct f z; /* { dg-error "two or more " "" } */
Index: testsuite/gcc.dg/two-types-6.c
===================================================================
--- testsuite/gcc.dg/two-types-6.c (revision 0)
+++ testsuite/gcc.dg/two-types-6.c (revision 0)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct s {
+ struct f {} /* dg-warning "does not declare anything" "" } */
+ struct g {} x; /* { dg-error "expected ';', identifier or " "" } */
+};
Index: testsuite/gcc.dg/two-types-3.c
===================================================================
--- testsuite/gcc.dg/two-types-3.c (revision 0)
+++ testsuite/gcc.dg/two-types-3.c (revision 0)
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct f {
+}
+int z(); /* { dg-error "expected ';', identifier or " "" { target *-*-* } } */
Index: testsuite/gcc.dg/two-types-7.c
===================================================================
--- testsuite/gcc.dg/two-types-7.c (revision 0)
+++ testsuite/gcc.dg/two-types-7.c (revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+struct s {
+ struct f {}
+ enum a { X } /* { dg-error "expected ';', identifier or " "" } */
+ struct g {} /* { dg-error "expected identifier " "" } */
+}; /* { dg-warning "no semicolon" "" } */
Index: testsuite/objc.dg/two-types-1.m
===================================================================
--- testsuite/objc.dg/two-types-1.m (revision 0)
+++ testsuite/objc.dg/two-types-1.m (revision 0)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89" } // suppress default -pedantic-errors */
+
+@interface foo
+struct f {}
+struct g { int a; }; /* { dg-error "expected ';', identifier or " "" } */
+
+- (struct f *) a;
+- (struct g *) b;
+@end
+
+int f(struct g *x)
+{
+ return x->a; /* { dg-bogus " has no member " "" } */
+}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Better error reporting for missing semicolons after a struct definition
2010-11-13 11:18 ` Paolo Bonzini
@ 2010-11-15 16:49 ` Joseph S. Myers
0 siblings, 0 replies; 8+ messages in thread
From: Joseph S. Myers @ 2010-11-15 16:49 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: gcc-patches
On Sat, 13 Nov 2010, Paolo Bonzini wrote:
> On 11/10/2010 03:05 AM, Joseph S. Myers wrote:
> > Perhaps you could post an updated version of your patch to improve
> > diagnostics for missing semicolons, relative to current trunk?
>
> Here it is, bootstrap/regtest in progress.
This is OK if it passed the testing with no regressions.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-11-15 16:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-29 16:44 [PATCH] Better error reporting for missing semicolons after a struct definition Paolo Bonzini
2010-11-05 18:04 ` Joseph S. Myers
2010-11-08 10:20 ` Paolo Bonzini
2010-11-09 16:44 ` Paolo Bonzini
2010-11-10 2:13 ` Joseph S. Myers
2010-11-10 10:10 ` Paolo Bonzini
2010-11-13 11:18 ` Paolo Bonzini
2010-11-15 16:49 ` Joseph S. Myers
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).