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@xxxxxxxxxxxxxxxxxxxx>
>>>>> ---
>>>>> 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
