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

[Nikola Smiljanic]

>
>
> We should still be warning on this line, because it could either be
> declaring d to be a pointer to a function which returns T, or to be a
> pointer to T initialized by int(p). The place to handle this is probably
> GetFullTypeForDeclarator in SemaType.cpp. I don't know if you can move all
> of this checking to there, but if you can, that would be ideal. If not,
> splitting it up is OK.
>
>
I moved everything to SemaType.cpp but I have a number of questions:


> Please factor the D.isAmbiguous() && !T->isVoidType() tests into an outer
> 'if'.
>

I couldn't do this inside SemaDecl.cpp as the call to D.getTypeObject(0)
crashed when I moved it outside the 'if'. Now that it's in SemaType.cpp, it
seems to be working even outside the 'if''. Hope this isn't by chance, but
tests show no problems.


>
> It would be great to say "as a function declaration" here (but still use
> the "as a function declarator" in the pointer-to-function case).
>
>
I added the message text, but I don't know how to differentiate between the
two cases. I can see that D.getNumTypeObjects() returns 3 for the 'pointer
to function' case, but I don't really understand what this function does.
Same goes for D.getNumTypeObjects() == 1 condition.

Checking for FunctionTemplate would be another issue. These warnings don't
fire at file scope, I'm guessing that this is because function declarations
are much more likely here than global variables? It seems that template
declarations are not allowed at block scope so this check is not really
needed. I might be wrong.

Function return type used to be obtained from TypeSourceInfo but this
parameter is 0 inside GetFullTypeForDeclaration. I saw that the rest of
code uses declSpecType for checks about the return type so this is what I
used too.

One of the diagnostics used NewFD variable (FunctionDecl *) in the message
output, but I used the identifier that was used to do a lookup. I'm
guessing that this works the same and only outputs the function name. The
test case for this passes with my modification.

ActOnFuctionDeclaration had a 'Scope *' parameter but
GetFullTypeForDeclaration doesn't.  I used S.getCurScope() to get the
scope, but I'm not sure that this is entirely correct because I saw this in
Sema::HandleDeclarator:

  // The scope passed in may not be a decl scope.  Zip up the scope tree
until
  // we find one that is.
  while ((S->getFlags() & Scope::DeclScope) == 0 ||
         (S->getFlags() & Scope::TemplateParamScope) != 0)
    S = S->getParent();


Comments are breaking the 80 width policy by one character and I have no
idea what to do about them. I'm also not sure if we need the 'We don't
catch this case earlier, since there is no ambiguity here.' comment.

This was a bit long but since I moved a lot of code it might be easier for
you to check every change this way. All test are passing except the one
with function pointer that now expects different wording.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120718/4f72aaf6/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vexing.patch
Type: application/octet-stream
Size: 32039 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120718/4f72aaf6/attachment.obj>