[cfe-commits] Note and FixIt for additional case of vexing parse

[Nikola Smiljanic]

Thanks for your insightful comments Richard, I've been really busy but
I'll hopefully find some time to finish this. Here's a new patch just
to check that I'm on track:

I moved the warning completely to Sema, I'm not sure if this is the
right approach?

I don't know how to differentiate between int foo(int) which is
obviously a function declaration and int foo(int()) which could be a
variable declaration. What I'm looking for is a check whether the
function declaration is ambiguous?

> Your FixIt will cause these declarations to use vector's initializer_list
> constructor, which is presumably not what the author of the code intended.
> We can't detect these cases from within the parser, so this check should
> either always use the C++98 note, or (preferably, but more work) be moved to
> Sema (alongside the "int n()" vexing parse diagnostic code) and use the
> braces note only if there is no initializer_list constructor.

I'm starting to think that this note doesn't make sense even in that
case, because what if somebody suppresses the warning and later on
adds a constructor with an initializer list? Adding a pair of braces
could change the meaning of the code, and we don't want this, right?

> Moving this check to Sema would also allow us to easily suppress
> the bogus warning on code like:
>
>   typedef void V;
>   void f() {
>     V g(int (*p)[4]);
>   }
>
> ... on the basis that the return type is 'void', in the same way we suppress
> it for V g();

This should be covered now by checking the return type.

>
> This change will also produce an unhelpful note for this case (in C++98
> mode)...
>
>   if (int n(int())) {
>
> ... because a direct-initializer isn't actually permitted on this
> declaration, but we perform a tentative parse anyway in order to produce the
> 'variable declaration in condition cannot have a parenthesized initializer'
> diagnostic. For now, I suggest you just set warnIfAmbiguous to false in
> ParseDirectDeclarator when we're in a Declarator::ConditionContext.

What would be the right way to check for this case in the Sema? I can
do D.Context != ConditionContext inside my if statement?

> This fixit isn't correct if multiple parameters are declared: you're
> suggesting adding a single set of parentheses around all the parameters;
> instead, you should suggest adding them around just one of the parameters.

This should be correct now, but I have a question regarding fixit
insertion. The fixit is inserted before the location it points to,
just like iterators in stl? My fixit for "int foo(int());" points to
"i" of the parameter, and to the last closing paren (the one before
the semicolon). But I could achieve the same result without calling
PP.getLocForEndOfToken at all, but the insertion points wouldn't be
100% accurate. The first one would point to foo's open paren and the
second one would point to first parameter's closing paren. I'm not
sure what the convention here is?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vexing.patch
Type: application/octet-stream
Size: 6312 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120714/02e4409e/attachment.obj>