| Message ID | E1LYJZx-0001ls-Nn@ZenIV.linux.org.uk (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
On Sat, Feb 14, 2009 at 4:25 AM, Al Viro <viro@ftp.linux.org.uk> wrote: > > The rule for ident-less declaration is wow, great. The series applies to the tree fine. First impression of the series looks fine. I am still working on it. Thanks for the patches. BTW, this is not the "lazy type evaluation" you are talking about right? Chris > declaration -> declaration-specifiers ; > not > declaration -> declaration-specifiers abstract-declarator; > > IOW, struct foo; is OK and so's struct foo {int x; int y;} (and even > simply int; is allowed by syntax - it's rejected by constraints, but > that's a separate story), but not struct foo (void); and its ilk. > > See C99 6.7p1 for syntax; C90 is the same in that area and gcc also > behaves the same way. Unlike gcc I've made it a warning (gcc produces > a hard error for those). > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > parse.c | 10 +++++++++- > validation/missing-ident.c | 18 ++++++++++++++++++ > 2 files changed, 27 insertions(+), 1 deletions(-) > create mode 100644 validation/missing-ident.c > > diff --git a/parse.c b/parse.c > index 73e7b65..6100fc2 100644 > --- a/parse.c > +++ b/parse.c > @@ -2265,14 +2265,22 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis > token = declaration_specifiers(token, &ctype, 0); > decl = alloc_symbol(token->pos, SYM_NODE); > decl->ctype = ctype; > + /* Just a type declaration? */ > + if (match_op(token, ';')) { > + apply_modifiers(token->pos, &decl->ctype); > + return token->next; > + } > + > token = declarator(token, decl, &ident, 0); > apply_modifiers(token->pos, &decl->ctype); > > decl->endpos = token->pos; > > /* Just a type declaration? */ > - if (!ident) > + if (!ident) { > + warning(token->pos, "missing identifier in declaration"); > return expect(token, ';', "end of type declaration"); > + } > > /* type define declaration? */ > is_typedef = (ctype.modifiers & MOD_TYPEDEF) != 0; > diff --git a/validation/missing-ident.c b/validation/missing-ident.c > new file mode 100644 > index 0000000..ce73983 > --- /dev/null > +++ b/validation/missing-ident.c > @@ -0,0 +1,18 @@ > +int [2]; > +int *; > +int (*); > +int (); > +int; > +struct foo; > +union bar {int x; int y;}; > +struct baz {int x, :3, y:2;}; > +/* > + * check-name: handling of identifier-less declarations > + * > + * check-error-start > +missing-ident.c:1:8: warning: missing identifier in declaration > +missing-ident.c:2:6: warning: missing identifier in declaration > +missing-ident.c:3:8: warning: missing identifier in declaration > +missing-ident.c:4:7: warning: missing identifier in declaration > + * check-error-end > + */ > -- > 1.5.6.6 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sparse" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 14, 2009 at 10:45:41AM -0800, Christopher Li wrote: > On Sat, Feb 14, 2009 at 4:25 AM, Al Viro <viro@ftp.linux.org.uk> wrote: > > > > The rule for ident-less declaration is > > wow, great. > > The series applies to the tree fine. First impression of the series > looks fine. I am still working on it. > > Thanks for the patches. BTW, this is not the "lazy type evaluation" > you are talking about right? No, just the first batch of declaration parser fixes... The next group is about separating ctype-as-part-of-symbol from ctype-as-parser-state uses. After that - getting declaration_specifiers() to sane shape, which, BTW, will relieve the situation with mode bits. Then - cleaning up the type handling... -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 14, 2009 at 07:08:36PM +0000, Al Viro wrote: > On Sat, Feb 14, 2009 at 10:45:41AM -0800, Christopher Li wrote: > > On Sat, Feb 14, 2009 at 4:25 AM, Al Viro <viro@ftp.linux.org.uk> wrote: > > > > > > The rule for ident-less declaration is > > > > wow, great. > > > > The series applies to the tree fine. First impression of the series > > looks fine. I am still working on it. > > > > Thanks for the patches. BTW, this is not the "lazy type evaluation" > > you are talking about right? > > No, just the first batch of declaration parser fixes... The next group is > about separating ctype-as-part-of-symbol from ctype-as-parser-state uses. > After that - getting declaration_specifiers() to sane shape, which, BTW, > will relieve the situation with mode bits. Then - cleaning up the type > handling... BTW, I wonder whether if it would be better to just scan for the end of attributes after ( when we are deciding whether it's a function or nested declarator, leaving the actual handling of these guys to after the decision. The thing is, unlike gcc we have the token list anyway, and it's easier to not bother with passing that crap to parameter_type_list(), etc. I'll try to do that and see what falls out; potentially that replaces 7/7 in this series. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 14, 2009 at 08:31:54PM +0000, Al Viro wrote: > On Sat, Feb 14, 2009 at 07:08:36PM +0000, Al Viro wrote: > > On Sat, Feb 14, 2009 at 10:45:41AM -0800, Christopher Li wrote: > > > On Sat, Feb 14, 2009 at 4:25 AM, Al Viro <viro@ftp.linux.org.uk> wrote: > > > > > > > > The rule for ident-less declaration is > > > > > > wow, great. > > > > > > The series applies to the tree fine. First impression of the series > > > looks fine. I am still working on it. > > > > > > Thanks for the patches. BTW, this is not the "lazy type evaluation" > > > you are talking about right? > > > > No, just the first batch of declaration parser fixes... The next group is > > about separating ctype-as-part-of-symbol from ctype-as-parser-state uses. > > After that - getting declaration_specifiers() to sane shape, which, BTW, > > will relieve the situation with mode bits. Then - cleaning up the type > > handling... > > BTW, I wonder whether if it would be better to just scan for the end > of attributes after ( when we are deciding whether it's a function or > nested declarator, leaving the actual handling of these guys to after > the decision. The thing is, unlike gcc we have the token list anyway, > and it's easier to not bother with passing that crap to parameter_type_list(), > etc. > > I'll try to do that and see what falls out; potentially that replaces > 7/7 in this series. ... no, it doesn't ;-/ Too much PITA with error recovery, so the original version still stands. Alas. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 15, 2009 at 5:42 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > On Sun, Feb 15, 2009 at 04:33:16PM -0800, Christopher Li wrote: >> All applied and pushed. I update some of the validations error because >> of the tab width change. > > BTW, I'm not sure that this messing with tabstops is a good idea. > Note that tokenizer is _still_ the hottest part of the entire > thing, so we need to be damn careful around it. You are getting the > slow path of nextchar() on \t now, which can add up to a lot of > extra overhead... How about the attached patch? I move white space handling out side of the nextchar_slow(). Let nextchar_slow() only handle two case: possible EOF and '\\'. Chris
diff --git a/parse.c b/parse.c index 73e7b65..6100fc2 100644 --- a/parse.c +++ b/parse.c @@ -2265,14 +2265,22 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis token = declaration_specifiers(token, &ctype, 0); decl = alloc_symbol(token->pos, SYM_NODE); decl->ctype = ctype; + /* Just a type declaration? */ + if (match_op(token, ';')) { + apply_modifiers(token->pos, &decl->ctype); + return token->next; + } + token = declarator(token, decl, &ident, 0); apply_modifiers(token->pos, &decl->ctype); decl->endpos = token->pos; /* Just a type declaration? */ - if (!ident) + if (!ident) { + warning(token->pos, "missing identifier in declaration"); return expect(token, ';', "end of type declaration"); + } /* type define declaration? */ is_typedef = (ctype.modifiers & MOD_TYPEDEF) != 0; diff --git a/validation/missing-ident.c b/validation/missing-ident.c new file mode 100644 index 0000000..ce73983 --- /dev/null +++ b/validation/missing-ident.c @@ -0,0 +1,18 @@ +int [2]; +int *; +int (*); +int (); +int; +struct foo; +union bar {int x; int y;}; +struct baz {int x, :3, y:2;}; +/* + * check-name: handling of identifier-less declarations + * + * check-error-start +missing-ident.c:1:8: warning: missing identifier in declaration +missing-ident.c:2:6: warning: missing identifier in declaration +missing-ident.c:3:8: warning: missing identifier in declaration +missing-ident.c:4:7: warning: missing identifier in declaration + * check-error-end + */
The rule for ident-less declaration is declaration -> declaration-specifiers ; not declaration -> declaration-specifiers abstract-declarator; IOW, struct foo; is OK and so's struct foo {int x; int y;} (and even simply int; is allowed by syntax - it's rejected by constraints, but that's a separate story), but not struct foo (void); and its ilk. See C99 6.7p1 for syntax; C90 is the same in that area and gcc also behaves the same way. Unlike gcc I've made it a warning (gcc produces a hard error for those). Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- parse.c | 10 +++++++++- validation/missing-ident.c | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletions(-) create mode 100644 validation/missing-ident.c