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

[Richard Smith]

On Mon, Jul 16, 2012 at 8:13 AM, Nikola Smiljanic <popizdeh at gmail.com>wrote:

> I kindly request another review.
>
> while (int f()=0) ; // condition.cpp
> This should be the same as your ConditionContext example so I removed
> warnings.
>

Perfect.



> T(*d)(int(p)); // decl-expr-ambiguity.cpp
> This should be unambiguous, always a function pointer. Maybe I should
> delete the entire line since this file is explicitly testing ambiguity?
>

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.


+++ include/clang/Sema/DeclSpec.h (working copy)
@@ -1488,6 +1488,9 @@
   /// \brief Is this Declarator a redeclaration?
   bool Redeclaration : 1;

+  /// \brief Can this declaration be a constructor-style initializer?
+  bool Ambiguous : 1;

I think this flag really belongs on DeclaratorChunk::FunctionTypeInfo.
Sorry for suggesting putting it here!


+++ lib/Sema/SemaDecl.cpp (working copy)
@@ -5261,10 +5261,25 @@
+      if (!T->isVoidType() && C.Fun.NumArgs != 0 && D.isAmbiguous() &&
[...]
+      if (!T->isVoidType() && C.Fun.NumArgs == 0 && D.isAmbiguous()) {

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


+++ lib/Parse/ParseTentative.cpp (working copy)
@@ -671,7 +671,8 @@
+      bool Ambiuous = false;
+      if (!mayBeAbstract && !isCXXFunctionDeclarator(Ambiuous))

Typo (missing 'g'). Or even, drop this change, and instead...


+++ include/clang/Parse/Parser.h (working copy)
@@ -1643,11 +1643,11 @@
-  bool isCXXFunctionDeclarator(bool warnIfAmbiguous);
+  bool isCXXFunctionDeclarator(bool &isAmbiguous);

... use (bool *isAmbiguous = 0). Also, since you're touching this variable
anyway, would you mind renaming to IsAmbiguous to conform with the coding
standard?


+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -158,6 +158,11 @@
+def warn_parens_disambiguated_as_function_decl : Warning<
+  "parentheses were disambiguated as a function declarator">,
+  InGroup<VexingParse>;

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


Thanks for working on this!
Richard
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120716/8860207b/attachment.html>