Review Request: Make C macro variadic argument optional

[Alexandre Courbot]

> On July 18, 2012, 12:32 p.m., Milian Wolff wrote:
> > languages/cpp/parser/tests/test_parser.cpp, line 523
> > <http://git.reviewboard.kde.org/r/105307/diff/1/?file=69936#file69936line523>
> >
> >     I don't get this comment: I thought the variadic argument *can* be left empty? what is actually happening now in kdevelop?
> 
> Alexandre Courbot wrote:
>     Yes, that's precisely what the comment says: variadic arguments normally can be left empty, and this comment is about the error condition, which happens when they cannot.
> 
> Milian Wolff wrote:
>     I'm sorry but I need some more info as I'm still not understanding this :-S
>     
>     a) I understand from your review description, that this patch will allow KDevelop to properly handle empty variadic macro arguments
>     b) you add a test but expect it to fail, with the explanation that variadic args *cannot* be left empty
>     c) what is the actual result of the test you added there, if not "kdevelop"? which should be the result if we properly handle the empty variadic macro, no?
>     
>     or is your patch only "fixing" the error reporting, i.e. empty variadic macro arguments don't report an error anymore, but still don't work properly hence the result is not "kdevelop"? If so, then please change the test fail comment to something like "Empty variadic argument not properly handled".
>     
>

The key is that sentence in http://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html: "GNU CPP has a pair of extensions which deal with this problem. First, you are allowed to leave the variable argument out entirely".

This behavior was not properly implemented - i.e. if you left the variable argument empty in the (args...) form, the parser would trigger an error. The (args, ...) form was correctly handled though.

So this is just what this patch fixes. The additional test case also attempts to use the same macro with its variable argument and without.


- Alexandre


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105307/#review16070
-----------------------------------------------------------


On June 20, 2012, 1:51 p.m., Alexandre Courbot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105307/
> -----------------------------------------------------------
> 
> (Updated June 20, 2012, 1:51 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> In GNU C, the variadic argument of a macro can be completely left out
> without triggering an error (as explained in
> http://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html)
> 
> 
> Diffs
> -----
> 
>   languages/cpp/parser/rpp/pp-engine.cpp ca566cb 
>   languages/cpp/parser/tests/test_parser.cpp 804f379 
> 
> Diff: http://git.reviewboard.kde.org/r/105307/diff/
> 
> 
> Testing
> -------
> 
> Checked that the faulty case was not triggering a parsing error anymore. Checked that no regression was introduced in the testPreprocessor() test case.
> 
> 
> Thanks,
> 
> Alexandre Courbot
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120723/faa9a44d/attachment.html>