| Message ID | 20170218203048.22276-5-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Sun, Feb 19, 2017 at 4:30 AM, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > After parsing and validation, the symbols in the declaration > are added to the list given in argument, *if* they are not extern > symbols. The symbols that are extern are them not added to the list. > > This is what is needed for usual declarations but ignoring extern > symbols make it impossible to emit a diagnostic in less usual > situation. I have other situation want to use this as well. > --- a/parse.h > +++ b/parse.h > @@ -129,7 +129,8 @@ extern int show_statement(struct statement *); > extern void show_statement_list(struct statement_list *, const char *); > extern int show_expression(struct expression *); > > -extern struct token *external_declaration(struct token *token, struct symbol_list **list); > +typedef void (*add_decl_t)(struct symbol_list **list, struct symbol *decl); > +extern struct token *external_declaration(struct token *token, add_decl_t add_decl, struct symbol_list **list); > I think the logic should be, "external_declaration" accept token as input. For each newly declared symbol, it call to the callbacks function to receive the symbol. The receive behavior is depend on the callback function. The default function can be adding the symbol to a list. So the struct symbol_list **list should turn into transparent argument as context for the call back. > --- a/lib.c > +++ b/lib.c > @@ -1080,7 +1080,7 @@ static struct symbol_list *sparse_tokenstream(struct token *token) > > // Parse the resulting C code > while (!eof_token(token)) > - token = external_declaration(token, &translation_unit_used_list); > + token = external_declaration(token, NULL, &translation_unit_used_list); I prefer here just provide the default call back which is add_stmt_decl in your case. > return translation_unit_used_list; > } > > diff --git a/parse.c b/parse.c > index a4a126720..866186fd2 100644 > --- a/parse.c > +++ b/parse.c > @@ -2230,7 +2230,7 @@ static struct token *parse_for_statement(struct token *token, struct statement * > e1 = NULL; > /* C99 variable declaration? */ > if (lookup_type(token)) { > - token = external_declaration(token, &syms); > + token = external_declaration(token, NULL, &syms); Same here. > } else { > token = parse_expression(token, &e1); > token = expect(token, ';', "in 'for'"); > @@ -2457,7 +2457,7 @@ static struct token * statement_list(struct token *token, struct statement_list > seen_statement = 0; > } > stmt = alloc_statement(token->pos, STMT_DECLARATION); > - token = external_declaration(token, &stmt->declaration); > + token = external_declaration(token, NULL, &stmt->declaration); And here. > + if (!add_decl) > + add_decl = add_stmt_decl; > + We don't need this if we ask the caller to provide the call back. > for (;;) { > if (!is_typedef && match_op(token, '=')) { > if (decl->ctype.modifiers & MOD_EXTERN) { > @@ -2873,10 +2884,7 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis > token = initializer(&decl->initializer, token->next); > } > if (!is_typedef) { > - if (!(decl->ctype.modifiers & (MOD_EXTERN | MOD_INLINE))) { > - add_symbol(list, decl); > - fn_local_symbol(decl); > - } > + add_decl(list, decl); change this to some thing like: if (add_decl) add_decl(decl, decl_args); > } > check_declaration(decl); > if (decl->same_symbol) { Chris -- 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 Mon, Feb 27, 2017 at 4:37 PM, Christopher Li <sparse@chrisli.org> wrote: > On Sun, Feb 19, 2017 at 4:30 AM, Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: >> After parsing and validation, the symbols in the declaration >> are added to the list given in argument, *if* they are not extern >> symbols. The symbols that are extern are them not added to the list. >> >> This is what is needed for usual declarations but ignoring extern >> symbols make it impossible to emit a diagnostic in less usual >> situation. > > I have other situation want to use this as well. > >> --- a/parse.h >> +++ b/parse.h >> @@ -129,7 +129,8 @@ extern int show_statement(struct statement *); >> extern void show_statement_list(struct statement_list *, const char *); >> extern int show_expression(struct expression *); >> >> -extern struct token *external_declaration(struct token *token, struct symbol_list **list); >> +typedef void (*add_decl_t)(struct symbol_list **list, struct symbol *decl); >> +extern struct token *external_declaration(struct token *token, add_decl_t add_decl, struct symbol_list **list); >> > > I think the logic should be, "external_declaration" accept token as input. > For each newly declared symbol, it call to the callbacks function > to receive the symbol. The receive behavior is depend on the callback > function. The default function can be adding the symbol to a list. > > So the struct symbol_list **list should turn into transparent argument as > context for the call back. Yes, it can certainly be more general. By 'transparent' you mean a void pointer? For the two case currently concerned the callback argument can be a statement pointer because the two symbol list belong to a statement and I fact I hesitated to use it or the list. I choose the smallest change. If for the other situation you talked here above, we can also use a statement pointer, I would prefrer to do so instead than using a void pointer but well it doesn't matter much anyway. >> --- a/lib.c >> +++ b/lib.c >> @@ -1080,7 +1080,7 @@ static struct symbol_list *sparse_tokenstream(struct token *token) >> >> // Parse the resulting C code >> while (!eof_token(token)) >> - token = external_declaration(token, &translation_unit_used_list); >> + token = external_declaration(token, NULL, &translation_unit_used_list); > > I prefer here just provide the default call back which is > add_stmt_decl in your case. Sure, I can do this. Luc -- 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 Mon, Feb 27, 2017 at 11:37:09PM +0800, Christopher Li wrote: > On Sun, Feb 19, 2017 at 4:30 AM, Luc Van Oostenryck wrote: > > > > This is what is needed for usual declarations but ignoring extern > > symbols make it impossible to emit a diagnostic in less usual > > situation. > > I have other situation want to use this as well. ... > So the struct symbol_list **list should turn into transparent argument as > context for the call back. > ... > I prefer here just provide the default call back which is > add_stmt_decl in your case. OK. I've made the changes related to this and also prepared for something more generic (like the name of the method). I've left the arg type as 'symbol_list **' for the moment though, it will be easy enough to change when/if needed. Luc -- 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
This serie adds scope & storage validations of C99-style for-loop initializers. Patch 1 replaces the current indirect test by a direct one Patch 2-3 add test cases for the scope & storage Patch 4-5 add missing storage validation Patch 6-7 move some checks into default_process_decl(). Changes since v1: - better log message for patch 1, thanks to Ramsay Jones Changes since v2: - patches 1-3 are unchanged. - do not use 'NULL' for the default method - limit changes to parse.c, leaving external_declaration() untouched - use a more generic and exact name for the method - move a check to default_process_decl() Luc Van Oostenryck (7): replace test for c99 for-loop initializers add test case for scope of C99 for-loop declarations add test cases for storage of c99 for-loop declarations add a method to external_declaration() check the storage of C99 for-loop initializers make process_decl() aware of the presence of an initializer move check extern with initializer to default_process_decl() parse.c | 54 ++++++++++++++++++++++++++++++++---------- validation/c99-for-loop-decl.c | 40 +++++++++++++++++++++++++++++++ validation/c99-for-loop.c | 36 ++++++++++------------------ 3 files changed, 94 insertions(+), 36 deletions(-) create mode 100644 validation/c99-for-loop-decl.c
On Tue, Feb 28, 2017 at 6:03 PM, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > This serie adds scope & storage validations of C99-style for-loop > initializers. Just a heads up I am traveling in the next few days. My time and internet access is unpredictable. I will try to apply them if I got a chance. In the worse case, I will work on it on Sunday. Chris -- 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 Tue, Feb 28, 2017 at 5:34 PM, Christopher Li <sparse@chrisli.org> wrote: > On Tue, Feb 28, 2017 at 6:03 PM, Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: >> This serie adds scope & storage validations of C99-style for-loop >> initializers. > > Just a heads up I am traveling in the next few days. My time and internet access > is unpredictable. I will try to apply them if I got a chance. In the > worse case, I will > work on it on Sunday. > > Chris OK. There is no urgency anyway but thanks for letting known. Have a nice travel. -- 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
diff --git a/lib.c b/lib.c index d47325243..9badc09b3 100644 --- a/lib.c +++ b/lib.c @@ -1080,7 +1080,7 @@ static struct symbol_list *sparse_tokenstream(struct token *token) // Parse the resulting C code while (!eof_token(token)) - token = external_declaration(token, &translation_unit_used_list); + token = external_declaration(token, NULL, &translation_unit_used_list); return translation_unit_used_list; } diff --git a/parse.c b/parse.c index a4a126720..866186fd2 100644 --- a/parse.c +++ b/parse.c @@ -2230,7 +2230,7 @@ static struct token *parse_for_statement(struct token *token, struct statement * e1 = NULL; /* C99 variable declaration? */ if (lookup_type(token)) { - token = external_declaration(token, &syms); + token = external_declaration(token, NULL, &syms); } else { token = parse_expression(token, &e1); token = expect(token, ';', "in 'for'"); @@ -2457,7 +2457,7 @@ static struct token * statement_list(struct token *token, struct statement_list seen_statement = 0; } stmt = alloc_statement(token->pos, STMT_DECLARATION); - token = external_declaration(token, &stmt->declaration); + token = external_declaration(token, NULL, &stmt->declaration); } else { seen_statement = Wdeclarationafterstatement; token = statement(token, &stmt); @@ -2785,7 +2785,15 @@ static struct token *toplevel_asm_declaration(struct token *token, struct symbol return token; } -struct token *external_declaration(struct token *token, struct symbol_list **list) +static void add_stmt_decl(struct symbol_list **list, struct symbol *decl) +{ + if (!(decl->ctype.modifiers & (MOD_EXTERN | MOD_INLINE))) { + add_symbol(list, decl); + fn_local_symbol(decl); + } +} + +struct token *external_declaration(struct token *token, add_decl_t add_decl, struct symbol_list **list) { struct ident *ident = NULL; struct symbol *decl; @@ -2864,6 +2872,9 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis sparse_error(token->pos, "void declaration"); } + if (!add_decl) + add_decl = add_stmt_decl; + for (;;) { if (!is_typedef && match_op(token, '=')) { if (decl->ctype.modifiers & MOD_EXTERN) { @@ -2873,10 +2884,7 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis token = initializer(&decl->initializer, token->next); } if (!is_typedef) { - if (!(decl->ctype.modifiers & (MOD_EXTERN | MOD_INLINE))) { - add_symbol(list, decl); - fn_local_symbol(decl); - } + add_decl(list, decl); } check_declaration(decl); if (decl->same_symbol) { diff --git a/parse.h b/parse.h index a2b3e3889..d3199727e 100644 --- a/parse.h +++ b/parse.h @@ -129,7 +129,8 @@ extern int show_statement(struct statement *); extern void show_statement_list(struct statement_list *, const char *); extern int show_expression(struct expression *); -extern struct token *external_declaration(struct token *token, struct symbol_list **list); +typedef void (*add_decl_t)(struct symbol_list **list, struct symbol *decl); +extern struct token *external_declaration(struct token *token, add_decl_t add_decl, struct symbol_list **list); extern struct symbol *ctype_integer(int size, int want_unsigned);
After parsing and validation, the symbols in the declaration are added to the list given in argument, *if* they are not extern symbols. The symbols that are extern are them not added to the list. This is what is needed for usual declarations but ignoring extern symbols make it impossible to emit a diagnostic in less usual situation. This is motivated by the validation of variable declaration inside a for-loop initializer, which valid in C99 but only for variable with local storage. The changes consist in moving the part 'add the symbol to the list if the symbol is not extern' to a separate function which will be called by default. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- lib.c | 2 +- parse.c | 22 +++++++++++++++------- parse.h | 3 ++- 3 files changed, 18 insertions(+), 9 deletions(-)