| Message ID | 20160218213902.GV3017@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/18/2016 02:39 PM, Jakub Jelinek wrote: > Hi! > > Here is an attempt to fix up the token reclassification after for statement, > where we lexed the next token with the declaration from for in scope and > need to undo that if it wasn't else. > > If token->id_kind is C_ID_CLASSNAME (ObjC only), then token->value has > changed already, but in that case I think it means we can just keep it as > is, it can't be shadowed by the for init declaration (because it would be > C_ID_ID or C_ID_TYPENAME? otherwise). > Otherwise, this patch tries to model the handling closer to what > c_lex_one_token does, i.e. set it to C_ID_ID, except when decl is non-NULL > and TYPE_DECL, or for the ObjC case where for init declaration shadowed > a class declaration. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2016-02-18 Jakub Jelinek <jakub@redhat.com> > > PR objc/69844 > * c-parser.c (c_parser_for_statement): Properly handle ObjC classes > in id_kind reclassification. > > * objc.dg/pr69844.m: New test. I'm having trouble following the issue and your fix. Marek, you're probably more familiar with these interactions, can you take a look at Jakub's patch? Thanks, jeff > > --- gcc/c/c-parser.c.jj 2016-02-16 16:29:54.000000000 +0100 > +++ gcc/c/c-parser.c 2016-02-18 17:36:55.025067859 +0100 > @@ -5887,12 +5887,27 @@ c_parser_for_statement (c_parser *parser > { > c_token *token = c_parser_peek_token (parser); > tree decl = lookup_name (token->value); > - if (decl == NULL_TREE || VAR_P (decl)) > - /* If DECL is null, we don't know what this token might be. Treat > - it as an ID for better diagnostics; we'll error later on. */ > - token->id_kind = C_ID_ID; > - else if (TREE_CODE (decl) == TYPE_DECL) > - token->id_kind = C_ID_TYPENAME; > + if (token->id_kind != C_ID_CLASSNAME) > + { > + token->id_kind = C_ID_ID; > + if (decl) > + { > + if (TREE_CODE (decl) == TYPE_DECL) > + token->id_kind = C_ID_TYPENAME; > + } > + else if (c_dialect_objc ()) > + { > + tree objc_interface_decl = objc_is_class_name (token->value); > + /* Objective-C class names are in the same namespace as > + variables and typedefs, and hence are shadowed by local > + declarations. */ > + if (objc_interface_decl) > + { > + token->value = objc_interface_decl; > + token->id_kind = C_ID_CLASSNAME; > + } > + } > + } > } > > token_indent_info next_tinfo > --- gcc/testsuite/objc.dg/pr69844.m.jj 2016-02-18 17:44:40.896624579 +0100 > +++ gcc/testsuite/objc.dg/pr69844.m 2016-02-18 17:45:20.465078855 +0100 > @@ -0,0 +1,24 @@ > +/* PR objc/69844 */ > +/* { dg-do compile } */ > +/* { dg-options "-std=gnu99" } */ > + > +@class D; > + > +void > +foo (void) > +{ > + for (;;) > + ; > + D *d = (id) 0; > + (void) d; > +} > + > +void > +bar (void) > +{ > + for (int D = 0; D < 30; D++) > + if (1) > + ; > + D *d = (id) 0; > + (void) d; > +} > > Jakub >
On Thu, Feb 18, 2016 at 10:39:02PM +0100, Jakub Jelinek wrote: > Hi! > > Here is an attempt to fix up the token reclassification after for statement, > where we lexed the next token with the declaration from for in scope and > need to undo that if it wasn't else. > > If token->id_kind is C_ID_CLASSNAME (ObjC only), then token->value has > changed already, but in that case I think it means we can just keep it as > is, it can't be shadowed by the for init declaration (because it would be > C_ID_ID or C_ID_TYPENAME? otherwise). > Otherwise, this patch tries to model the handling closer to what > c_lex_one_token does, i.e. set it to C_ID_ID, except when decl is non-NULL > and TYPE_DECL, or for the ObjC case where for init declaration shadowed > a class declaration. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2016-02-18 Jakub Jelinek <jakub@redhat.com> > > PR objc/69844 > * c-parser.c (c_parser_for_statement): Properly handle ObjC classes > in id_kind reclassification. > > * objc.dg/pr69844.m: New test. > > --- gcc/c/c-parser.c.jj 2016-02-16 16:29:54.000000000 +0100 > +++ gcc/c/c-parser.c 2016-02-18 17:36:55.025067859 +0100 > @@ -5887,12 +5887,27 @@ c_parser_for_statement (c_parser *parser > { > c_token *token = c_parser_peek_token (parser); > tree decl = lookup_name (token->value); > - if (decl == NULL_TREE || VAR_P (decl)) > - /* If DECL is null, we don't know what this token might be. Treat > - it as an ID for better diagnostics; we'll error later on. */ > - token->id_kind = C_ID_ID; > - else if (TREE_CODE (decl) == TYPE_DECL) > - token->id_kind = C_ID_TYPENAME; > + if (token->id_kind != C_ID_CLASSNAME) > + { > + token->id_kind = C_ID_ID; I think let's sink the lookup_name call here. If id_kind is C_ID_CLASSNAME we're not looking at decl at all. > + if (decl) > + { > + if (TREE_CODE (decl) == TYPE_DECL) > + token->id_kind = C_ID_TYPENAME; > + } > + else if (c_dialect_objc ()) > + { > + tree objc_interface_decl = objc_is_class_name (token->value); This objc_is_class_name is a weird stub that always returns NULL_TREE but I know that the same code is in c_lex_one_token so let's keep it this way. I've tried a bunch of invalid ObjC testcases and saw no ICEs and from the C point of view this patch is safe. Ok with that lookup_name change, thanks. Marek
On Tue, Feb 23, 2016 at 08:24:06PM +0100, Marek Polacek wrote: > > --- gcc/c/c-parser.c.jj 2016-02-16 16:29:54.000000000 +0100 > > +++ gcc/c/c-parser.c 2016-02-18 17:36:55.025067859 +0100 > > @@ -5887,12 +5887,27 @@ c_parser_for_statement (c_parser *parser > > { > > c_token *token = c_parser_peek_token (parser); > > tree decl = lookup_name (token->value); > > - if (decl == NULL_TREE || VAR_P (decl)) > > - /* If DECL is null, we don't know what this token might be. Treat > > - it as an ID for better diagnostics; we'll error later on. */ > > - token->id_kind = C_ID_ID; > > - else if (TREE_CODE (decl) == TYPE_DECL) > > - token->id_kind = C_ID_TYPENAME; > > + if (token->id_kind != C_ID_CLASSNAME) > > + { > > + token->id_kind = C_ID_ID; > > I think let's sink the lookup_name call here. If id_kind is C_ID_CLASSNAME > we're not looking at decl at all. Done (and committed). Thanks for review. > > + if (decl) > > + { > > + if (TREE_CODE (decl) == TYPE_DECL) > > + token->id_kind = C_ID_TYPENAME; > > + } > > + else if (c_dialect_objc ()) > > + { > > + tree objc_interface_decl = objc_is_class_name (token->value); > > This objc_is_class_name is a weird stub that always returns NULL_TREE but It is a weird stub only in the cc1 binary, in cc1obj binary it comes from objc/objc-act.c and does various ObjC magic. Jakub
--- gcc/c/c-parser.c.jj 2016-02-16 16:29:54.000000000 +0100 +++ gcc/c/c-parser.c 2016-02-18 17:36:55.025067859 +0100 @@ -5887,12 +5887,27 @@ c_parser_for_statement (c_parser *parser { c_token *token = c_parser_peek_token (parser); tree decl = lookup_name (token->value); - if (decl == NULL_TREE || VAR_P (decl)) - /* If DECL is null, we don't know what this token might be. Treat - it as an ID for better diagnostics; we'll error later on. */ - token->id_kind = C_ID_ID; - else if (TREE_CODE (decl) == TYPE_DECL) - token->id_kind = C_ID_TYPENAME; + if (token->id_kind != C_ID_CLASSNAME) + { + token->id_kind = C_ID_ID; + if (decl) + { + if (TREE_CODE (decl) == TYPE_DECL) + token->id_kind = C_ID_TYPENAME; + } + else if (c_dialect_objc ()) + { + tree objc_interface_decl = objc_is_class_name (token->value); + /* Objective-C class names are in the same namespace as + variables and typedefs, and hence are shadowed by local + declarations. */ + if (objc_interface_decl) + { + token->value = objc_interface_decl; + token->id_kind = C_ID_CLASSNAME; + } + } + } } token_indent_info next_tinfo --- gcc/testsuite/objc.dg/pr69844.m.jj 2016-02-18 17:44:40.896624579 +0100 +++ gcc/testsuite/objc.dg/pr69844.m 2016-02-18 17:45:20.465078855 +0100 @@ -0,0 +1,24 @@ +/* PR objc/69844 */ +/* { dg-do compile } */ +/* { dg-options "-std=gnu99" } */ + +@class D; + +void +foo (void) +{ + for (;;) + ; + D *d = (id) 0; + (void) d; +} + +void +bar (void) +{ + for (int D = 0; D < 30; D++) + if (1) + ; + D *d = (id) 0; + (void) d; +}