| Message ID | 91253db3-6c8e-2b2f-0672-73b804b26ac2@ramsayjones.plus.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | misc sparse patches | expand |
On Mon, Nov 19, 2018 at 08:51:33PM +0000, Ramsay Jones wrote: > > The dump_macros() function fails to correctly output the definition of > macros that have a variable argument list. For example, the following > macros: > > #define unlocks(...) annotate(unlock_func(__VA_ARGS__)) > #define apply(x,...) x(__VA_ARGS__) > > are output like so: > > #define unlocks(__VA_ARGS__) annotate(unlock_func(__VA_ARGS__)) > #define apply(x,__VA_ARGS__) x(__VA_ARGS__) > > Add the code necessary to print the ellipsis in the argument list to the > dump_macros() function and add the above macros to the 'dump-macros.c' > test file. > > Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> > --- > pre-process.c | 11 ++++++++++- > validation/preprocessor/dump-macros.c | 5 +++++ > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/pre-process.c b/pre-process.c > index 7b76c65..418b8ee 100644 > --- a/pre-process.c > +++ b/pre-process.c > @@ -2167,6 +2167,12 @@ struct token * preprocess(struct token *token) > return token; > } > > +static int is_VA_ARGS_token(struct token *token) > +{ > + return (token_type(token) == TOKEN_IDENT) && > + (token->ident == &__VA_ARGS___ident); > +} > + > static void dump_macro(struct symbol *sym) > { > int nargs = sym->arglist ? sym->arglist->count.normal : 0; > @@ -2182,7 +2188,10 @@ static void dump_macro(struct symbol *sym) > for (; !eof_token(token); token = token->next) { > if (token_type(token) == TOKEN_ARG_COUNT) > continue; > - printf("%s%s", sep, show_token(token)); > + if (is_VA_ARGS_token(token)) > + printf("%s...", sep); > + else > + printf("%s%s", sep, show_token(token)); I'm wondering what is displayed by: show_ident(&__VA_ARGS___ident); I would much prefer to adjust show_ident()/show_token() and keep the code here generic. -- Luc
On Tue, Nov 20, 2018 at 01:06:23AM +0100, Luc Van Oostenryck wrote: > On Mon, Nov 19, 2018 at 08:51:33PM +0000, Ramsay Jones wrote: > > > > The dump_macros() function fails to correctly output the definition of > > macros that have a variable argument list. For example, the following > > macros: > > > > #define unlocks(...) annotate(unlock_func(__VA_ARGS__)) > > #define apply(x,...) x(__VA_ARGS__) > > > > are output like so: > > > > #define unlocks(__VA_ARGS__) annotate(unlock_func(__VA_ARGS__)) > > #define apply(x,__VA_ARGS__) x(__VA_ARGS__) > > > > Add the code necessary to print the ellipsis in the argument list to the > > dump_macros() function and add the above macros to the 'dump-macros.c' > > test file. > > > > Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> > > --- > > pre-process.c | 11 ++++++++++- > > validation/preprocessor/dump-macros.c | 5 +++++ > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/pre-process.c b/pre-process.c > > index 7b76c65..418b8ee 100644 > > --- a/pre-process.c > > +++ b/pre-process.c > > @@ -2167,6 +2167,12 @@ struct token * preprocess(struct token *token) > > return token; > > } > > > > +static int is_VA_ARGS_token(struct token *token) > > +{ > > + return (token_type(token) == TOKEN_IDENT) && > > + (token->ident == &__VA_ARGS___ident); > > +} > > + > > static void dump_macro(struct symbol *sym) > > { > > int nargs = sym->arglist ? sym->arglist->count.normal : 0; > > @@ -2182,7 +2188,10 @@ static void dump_macro(struct symbol *sym) > > for (; !eof_token(token); token = token->next) { > > if (token_type(token) == TOKEN_ARG_COUNT) > > continue; > > - printf("%s%s", sep, show_token(token)); > > + if (is_VA_ARGS_token(token)) > > + printf("%s...", sep); > > + else > > + printf("%s%s", sep, show_token(token)); > > I'm wondering what is displayed by: > show_ident(&__VA_ARGS___ident); > > I would much prefer to adjust show_ident()/show_token() and keep > the code here generic. Mmmm, I looked at this one and what you've done here is the best. I don't understand exactly why but for arguments the token SPECIAL_ELLIPSIS is, at some stage, changed into a VA_ARGS ident. This need then to be reversed when dumping the macros. -- Luc
On 21/11/2018 00:12, Luc Van Oostenryck wrote: > On Tue, Nov 20, 2018 at 01:06:23AM +0100, Luc Van Oostenryck wrote: >> On Mon, Nov 19, 2018 at 08:51:33PM +0000, Ramsay Jones wrote: >>> >>> The dump_macros() function fails to correctly output the definition of >>> macros that have a variable argument list. For example, the following >>> macros: >>> >>> #define unlocks(...) annotate(unlock_func(__VA_ARGS__)) >>> #define apply(x,...) x(__VA_ARGS__) >>> >>> are output like so: >>> >>> #define unlocks(__VA_ARGS__) annotate(unlock_func(__VA_ARGS__)) >>> #define apply(x,__VA_ARGS__) x(__VA_ARGS__) >>> >>> Add the code necessary to print the ellipsis in the argument list to the >>> dump_macros() function and add the above macros to the 'dump-macros.c' >>> test file. >>> >>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> >>> --- >>> pre-process.c | 11 ++++++++++- >>> validation/preprocessor/dump-macros.c | 5 +++++ >>> 2 files changed, 15 insertions(+), 1 deletion(-) >>> >>> diff --git a/pre-process.c b/pre-process.c >>> index 7b76c65..418b8ee 100644 >>> --- a/pre-process.c >>> +++ b/pre-process.c >>> @@ -2167,6 +2167,12 @@ struct token * preprocess(struct token *token) >>> return token; >>> } >>> >>> +static int is_VA_ARGS_token(struct token *token) >>> +{ >>> + return (token_type(token) == TOKEN_IDENT) && >>> + (token->ident == &__VA_ARGS___ident); >>> +} >>> + >>> static void dump_macro(struct symbol *sym) >>> { >>> int nargs = sym->arglist ? sym->arglist->count.normal : 0; >>> @@ -2182,7 +2188,10 @@ static void dump_macro(struct symbol *sym) >>> for (; !eof_token(token); token = token->next) { >>> if (token_type(token) == TOKEN_ARG_COUNT) >>> continue; >>> - printf("%s%s", sep, show_token(token)); >>> + if (is_VA_ARGS_token(token)) >>> + printf("%s...", sep); >>> + else >>> + printf("%s%s", sep, show_token(token)); >> >> I'm wondering what is displayed by: >> show_ident(&__VA_ARGS___ident); >> >> I would much prefer to adjust show_ident()/show_token() and keep >> the code here generic. > > Mmmm, I looked at this one and what you've done here is the best. > I don't understand exactly why but for arguments the token > SPECIAL_ELLIPSIS is, at some stage, changed into a VA_ARGS ident. Heh, I'm having a weird flashback! I don't quite remember the details, but I was convinced the parse_arguments() (pre-process.c #1087) function was wrong, but I got lost several times in the debugger, so gave up! I thought a normal return would come from the condition at line 1123, which would leave the SPECIAL_ELLIPSIS token alone, rather than from the conditional at line 1143, which replaces it with VA_ARGS. ;-) > This need then to be reversed when dumping the macros. It is possible that SPECIAL_ELLIPSIS is not _always_ replaced with VA_ARGS, and the above should still work. However, I may have given up too easily. :) ATB, Ramsay Jones
On Wed, Nov 21, 2018 at 01:26:13AM +0000, Ramsay Jones wrote: > > > On 21/11/2018 00:12, Luc Van Oostenryck wrote: > > On Tue, Nov 20, 2018 at 01:06:23AM +0100, Luc Van Oostenryck wrote: > >> On Mon, Nov 19, 2018 at 08:51:33PM +0000, Ramsay Jones wrote: > >>> > >>> The dump_macros() function fails to correctly output the definition of > >>> macros that have a variable argument list. For example, the following > >>> macros: > >>> > >>> #define unlocks(...) annotate(unlock_func(__VA_ARGS__)) > >>> #define apply(x,...) x(__VA_ARGS__) > >>> > >>> are output like so: > >>> > >>> #define unlocks(__VA_ARGS__) annotate(unlock_func(__VA_ARGS__)) > >>> #define apply(x,__VA_ARGS__) x(__VA_ARGS__) > >>> > >>> Add the code necessary to print the ellipsis in the argument list to the > >>> dump_macros() function and add the above macros to the 'dump-macros.c' > >>> test file. > >>> > >>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> > >>> --- > >>> pre-process.c | 11 ++++++++++- > >>> validation/preprocessor/dump-macros.c | 5 +++++ > >>> 2 files changed, 15 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/pre-process.c b/pre-process.c > >>> index 7b76c65..418b8ee 100644 > >>> --- a/pre-process.c > >>> +++ b/pre-process.c > >>> @@ -2167,6 +2167,12 @@ struct token * preprocess(struct token *token) > >>> return token; > >>> } > >>> > >>> +static int is_VA_ARGS_token(struct token *token) > >>> +{ > >>> + return (token_type(token) == TOKEN_IDENT) && > >>> + (token->ident == &__VA_ARGS___ident); > >>> +} > >>> + > >>> static void dump_macro(struct symbol *sym) > >>> { > >>> int nargs = sym->arglist ? sym->arglist->count.normal : 0; > >>> @@ -2182,7 +2188,10 @@ static void dump_macro(struct symbol *sym) > >>> for (; !eof_token(token); token = token->next) { > >>> if (token_type(token) == TOKEN_ARG_COUNT) > >>> continue; > >>> - printf("%s%s", sep, show_token(token)); > >>> + if (is_VA_ARGS_token(token)) > >>> + printf("%s...", sep); > >>> + else > >>> + printf("%s%s", sep, show_token(token)); > >> > >> I'm wondering what is displayed by: > >> show_ident(&__VA_ARGS___ident); > >> > >> I would much prefer to adjust show_ident()/show_token() and keep > >> the code here generic. > > > > Mmmm, I looked at this one and what you've done here is the best. > > I don't understand exactly why but for arguments the token > > SPECIAL_ELLIPSIS is, at some stage, changed into a VA_ARGS ident. > > Heh, I'm having a weird flashback! I don't quite remember the details, > but I was convinced the parse_arguments() (pre-process.c #1087) function > was wrong, but I got lost several times in the debugger, so gave up! > > I thought a normal return would come from the condition at line 1123, > which would leave the SPECIAL_ELLIPSIS token alone, rather than from > the conditional at line 1143, which replaces it with VA_ARGS. ;-) > > It is possible that SPECIAL_ELLIPSIS is not _always_ replaced with > VA_ARGS, and the above should still work. However, I may have given up > too easily. :) Yes, but this condition is only reached if you have something like: #define m(x ...) and then either it's an error (no ending ')' ) or the ellipsis is replaced by TOKEN_ARG_COUNT. OTOH, the condition at line 1143 is reached when you have something like: #define m(x,...) or #define m(...) So, I think the function is correct and the ellipses are always replaced in non-erroneous cases (and the replacement is needed for current processing). -- Luc
On 21/11/2018 02:01, Luc Van Oostenryck wrote: > On Wed, Nov 21, 2018 at 01:26:13AM +0000, Ramsay Jones wrote: >> >> >> On 21/11/2018 00:12, Luc Van Oostenryck wrote: >>> On Tue, Nov 20, 2018 at 01:06:23AM +0100, Luc Van Oostenryck wrote: >>>> On Mon, Nov 19, 2018 at 08:51:33PM +0000, Ramsay Jones wrote: >>>>> >>>>> The dump_macros() function fails to correctly output the definition of >>>>> macros that have a variable argument list. For example, the following >>>>> macros: >>>>> >>>>> #define unlocks(...) annotate(unlock_func(__VA_ARGS__)) >>>>> #define apply(x,...) x(__VA_ARGS__) >>>>> >>>>> are output like so: >>>>> >>>>> #define unlocks(__VA_ARGS__) annotate(unlock_func(__VA_ARGS__)) >>>>> #define apply(x,__VA_ARGS__) x(__VA_ARGS__) >>>>> >>>>> Add the code necessary to print the ellipsis in the argument list to the >>>>> dump_macros() function and add the above macros to the 'dump-macros.c' >>>>> test file. >>>>> >>>>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> >>>>> --- >>>>> pre-process.c | 11 ++++++++++- >>>>> validation/preprocessor/dump-macros.c | 5 +++++ >>>>> 2 files changed, 15 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/pre-process.c b/pre-process.c >>>>> index 7b76c65..418b8ee 100644 >>>>> --- a/pre-process.c >>>>> +++ b/pre-process.c >>>>> @@ -2167,6 +2167,12 @@ struct token * preprocess(struct token *token) >>>>> return token; >>>>> } >>>>> >>>>> +static int is_VA_ARGS_token(struct token *token) >>>>> +{ >>>>> + return (token_type(token) == TOKEN_IDENT) && >>>>> + (token->ident == &__VA_ARGS___ident); >>>>> +} >>>>> + >>>>> static void dump_macro(struct symbol *sym) >>>>> { >>>>> int nargs = sym->arglist ? sym->arglist->count.normal : 0; >>>>> @@ -2182,7 +2188,10 @@ static void dump_macro(struct symbol *sym) >>>>> for (; !eof_token(token); token = token->next) { >>>>> if (token_type(token) == TOKEN_ARG_COUNT) >>>>> continue; >>>>> - printf("%s%s", sep, show_token(token)); >>>>> + if (is_VA_ARGS_token(token)) >>>>> + printf("%s...", sep); >>>>> + else >>>>> + printf("%s%s", sep, show_token(token)); >>>> >>>> I'm wondering what is displayed by: >>>> show_ident(&__VA_ARGS___ident); >>>> >>>> I would much prefer to adjust show_ident()/show_token() and keep >>>> the code here generic. >>> >>> Mmmm, I looked at this one and what you've done here is the best. >>> I don't understand exactly why but for arguments the token >>> SPECIAL_ELLIPSIS is, at some stage, changed into a VA_ARGS ident. >> >> Heh, I'm having a weird flashback! I don't quite remember the details, >> but I was convinced the parse_arguments() (pre-process.c #1087) function >> was wrong, but I got lost several times in the debugger, so gave up! >> >> I thought a normal return would come from the condition at line 1123, >> which would leave the SPECIAL_ELLIPSIS token alone, rather than from >> the conditional at line 1143, which replaces it with VA_ARGS. ;-) >> >> It is possible that SPECIAL_ELLIPSIS is not _always_ replaced with >> VA_ARGS, and the above should still work. However, I may have given up >> too easily. :) > > Yes, but this condition is only reached if you have something like: > #define m(x ...) > and then either it's an error (no ending ')' ) or the ellipsis is > replaced by TOKEN_ARG_COUNT. OTOH, the condition at line 1143 is > reached when you have something like: > #define m(x,...) > or > #define m(...) > > So, I think the function is correct and the ellipses are always > replaced in non-erroneous cases (and the replacement is needed > for current processing). Ah, yes, you are (of course) correct. Having been prompted, I now remember that the penny dropped when I was trying to follow a (cut- down) version of one of the preprocessorXX.c tests in the debugger. I was still a little concerned about the discrepancy in the token mangling and made a note to come back and look at that again (which I haven't done, obviously!). However, even if the dump-macros code would have been presented with either token type, the (new) code can handle both, so ... I am a little busy at the moment, but I will try to send out a new version of the patches soon. Well, I don't quite know what to do about patch #8. Shall we just drop that one for now (I can un-comment the SPARSE_FLAGS in my config.mak file again), or do you have an idea to improve the implementation of that patch? ATB, Ramsay Jones
On Wed, Nov 21, 2018 at 07:38:11PM +0000, Ramsay Jones wrote: > > I am a little busy at the moment, but I will try to send out a new > version of the patches soon. No worries. I can take #1 & #2 as is and I would like to take #9 quickly. #3-#6 don't need changes IIRC. I would like a small change in #7 but it can tweak it myself (I've more time now than I had in the past weeks and much probably more than you have now). > Well, I don't quite know what to do > about patch #8. Shall we just drop that one for now (I can un-comment > the SPARSE_FLAGS in my config.mak file again), or do you have an > idea to improve the implementation of that patch? Yes, this #8 is annoying. I'll think a bit about what can be done. Best regards, -- Luc
diff --git a/pre-process.c b/pre-process.c index 7b76c65..418b8ee 100644 --- a/pre-process.c +++ b/pre-process.c @@ -2167,6 +2167,12 @@ struct token * preprocess(struct token *token) return token; } +static int is_VA_ARGS_token(struct token *token) +{ + return (token_type(token) == TOKEN_IDENT) && + (token->ident == &__VA_ARGS___ident); +} + static void dump_macro(struct symbol *sym) { int nargs = sym->arglist ? sym->arglist->count.normal : 0; @@ -2182,7 +2188,10 @@ static void dump_macro(struct symbol *sym) for (; !eof_token(token); token = token->next) { if (token_type(token) == TOKEN_ARG_COUNT) continue; - printf("%s%s", sep, show_token(token)); + if (is_VA_ARGS_token(token)) + printf("%s...", sep); + else + printf("%s%s", sep, show_token(token)); args[narg++] = token; sep = ","; } diff --git a/validation/preprocessor/dump-macros.c b/validation/preprocessor/dump-macros.c index f8983d8..6940a20 100644 --- a/validation/preprocessor/dump-macros.c +++ b/validation/preprocessor/dump-macros.c @@ -9,6 +9,9 @@ #define STRING(x) #x #define CONCAT(x,y) x ## y + +#define unlocks(...) annotate(unlock_func(__VA_ARGS__)) +#define apply(x,...) x(__VA_ARGS__) /* * check-name: dump-macros * check-command: sparse -E -dD -DIJK=ijk -UNDEF -UNYDEF $file @@ -20,4 +23,6 @@ check-output-contains: #define DEF xyz check-output-contains: #define NYDEF ydef check-output-contains: #define STRING(x) #x check-output-contains: #define CONCAT(x,y) x ## y +check-output-contains: #define unlocks(...) annotate(unlock_func(__VA_ARGS__)) +check-output-contains: #define apply(x,...) x(__VA_ARGS__) */
The dump_macros() function fails to correctly output the definition of macros that have a variable argument list. For example, the following macros: #define unlocks(...) annotate(unlock_func(__VA_ARGS__)) #define apply(x,...) x(__VA_ARGS__) are output like so: #define unlocks(__VA_ARGS__) annotate(unlock_func(__VA_ARGS__)) #define apply(x,__VA_ARGS__) x(__VA_ARGS__) Add the code necessary to print the ellipsis in the argument list to the dump_macros() function and add the above macros to the 'dump-macros.c' test file. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> --- pre-process.c | 11 ++++++++++- validation/preprocessor/dump-macros.c | 5 +++++ 2 files changed, 15 insertions(+), 1 deletion(-)