Review Request: Make C macro variadic argument optional

[Milian Wolff]

> 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.

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".


- Milian


-----------------------------------------------------------
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/20120719/5bc8b00c/attachment.html>