Add a "vexing parse" warning for ambiguity between a variable declaration and a

function-style cast.

This fires for cases such as

  T(x);

... where 'x' was previously declared and T is a type. This construct declares
a variable named 'x' rather than the (probably expected) interpretation of a
function-style cast of 'x' to T.

llvm-svn: 314570

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -295,8 +295,20 @@ def warn_empty_parens_are_function_decl : Warning<
 def warn_parens_disambiguated_as_function_declaration : Warning<
   "parentheses were disambiguated as a function declaration">,
   InGroup<VexingParse>;
+def warn_parens_disambiguated_as_variable_declaration : Warning<
+  "parentheses were disambiguated as redundant parentheses around declaration "
+  "of variable named %0">, InGroup<VexingParse>;
+def warn_redundant_parens_around_declarator : Warning<
+  "redundant parentheses surrounding declarator">,
+  InGroup<DiagGroup<"redundant-parens">>, DefaultIgnore;
 def note_additional_parens_for_variable_declaration : Note<
   "add a pair of parentheses to declare a variable">;
+def note_raii_guard_add_name : Note<
+  "add a variable name to declare a %0 initialized with %1">;
+def note_function_style_cast_add_parentheses : Note<
+  "add enclosing parentheses to perform a function-style cast">;
+def note_remove_parens_for_variable_declaration : Note<
+  "remove parentheses to silence this warning">;
 def note_empty_parens_function_call : Note<
   "change this ',' to a ';' to call %0">;
 def note_empty_parens_default_ctor : Note<

clang/include/clang/Sema/DeclSpec.h

@@ -2301,6 +2301,42 @@ public:
     }
     llvm_unreachable("unknown context kind!");
   }
+
+  /// Determine whether this declaration appears in a context where an
+  /// expression could appear.
+  bool isExpressionContext() const {
+    switch (Context) {
+    case FileContext:
+    case KNRTypeListContext:
+    case MemberContext:
+    case TypeNameContext: // FIXME: sizeof(...) permits an expression.
+    case FunctionalCastContext:
+    case AliasDeclContext:
+    case AliasTemplateContext:
+    case PrototypeContext:
+    case LambdaExprParameterContext:
+    case ObjCParameterContext:
+    case ObjCResultContext:
+    case TemplateParamContext:
+    case CXXNewContext:
+    case CXXCatchContext:
+    case ObjCCatchContext:
+    case BlockLiteralContext:
+    case LambdaExprContext:
+    case ConversionIdContext:
+    case TrailingReturnContext:
+      return false;
+
+    case BlockContext:
+    case ForContext:
+    case InitStmtContext:
+    case ConditionContext:
+    case TemplateTypeArgContext:
+      return true;
+    }
+
+    llvm_unreachable("unknown context kind!");
+  }
   
   /// \brief Return true if a function declarator at this position would be a
   /// function declaration.

clang/lib/Sema/SemaType.cpp

@@ -3065,6 +3065,7 @@ static void warnAboutAmbiguousFunction(Sema &S, Declarator &D,
         S.Diag(D.getCommaLoc(), diag::note_empty_parens_function_call)
           << FixItHint::CreateReplacement(D.getCommaLoc(), ";")
           << D.getIdentifier();
+      Result.suppressDiagnostics();
     }
   }
 
@@ -3106,6 +3107,99 @@ static void warnAboutAmbiguousFunction(Sema &S, Declarator &D,
   }
 }
 
+/// Produce an appropriate diagnostic for a declarator with top-level
+/// parentheses.
+static void warnAboutRedundantParens(Sema &S, Declarator &D, QualType T) {
+  DeclaratorChunk &Paren = D.getTypeObject(D.getNumTypeObjects() - 1);
+  assert(Paren.Kind == DeclaratorChunk::Paren &&
+         "do not have redundant top-level parentheses");
+
+  // This is a syntactic check; we're not interested in cases that arise
+  // during template instantiation.
+  if (S.inTemplateInstantiation())
+    return;
+
+  // Check whether this could be intended to be a construction of a temporary
+  // object in C++ via a function-style cast.
+  bool CouldBeTemporaryObject =
+      S.getLangOpts().CPlusPlus && D.isExpressionContext() &&
+      !D.isInvalidType() && D.getIdentifier() &&
+      D.getDeclSpec().getParsedSpecifiers() == DeclSpec::PQ_TypeSpecifier &&
+      (T->isRecordType() || T->isDependentType()) &&
+      D.getDeclSpec().getTypeQualifiers() == 0 && D.isFirstDeclarator();
+
+  for (auto &C : D.type_objects()) {
+    switch (C.Kind) {
+    case DeclaratorChunk::Pointer:
+    case DeclaratorChunk::Paren:
+      continue;
+
+    case DeclaratorChunk::Array:
+      if (!C.Arr.NumElts)
+        CouldBeTemporaryObject = false;
+      continue;
+
+    case DeclaratorChunk::Reference:
+      // FIXME: Suppress the warning here if there is no initializer; we're
+      // going to give an error anyway.
+      // We assume that something like 'T (&x) = y;' is highly likely to not
+      // be intended to be a temporary object.
+      CouldBeTemporaryObject = false;
+      continue;
+
+    case DeclaratorChunk::Function:
+      // In a new-type-id, function chunks require parentheses.
+      if (D.getContext() == Declarator::CXXNewContext)
+        return;
+      LLVM_FALLTHROUGH;
+    case DeclaratorChunk::BlockPointer:
+    case DeclaratorChunk::MemberPointer:
+    case DeclaratorChunk::Pipe:
+      // These cannot appear in expressions.
+      CouldBeTemporaryObject = false;
+      continue;
+    }
+  }
+
+  // FIXME: If there is an initializer, assume that this is not intended to be
+  // a construction of a temporary object.
+
+  // Check whether the name has already been declared; if not, this is not a
+  // function-style cast.
+  if (CouldBeTemporaryObject) {
+    LookupResult Result(S, D.getIdentifier(), SourceLocation(),
+                        Sema::LookupOrdinaryName);
+    if (!S.LookupName(Result, S.getCurScope()))
+      CouldBeTemporaryObject = false;
+    Result.suppressDiagnostics();
+  }
+
+  SourceRange ParenRange(Paren.Loc, Paren.EndLoc);
+
+  if (!CouldBeTemporaryObject) {
+    S.Diag(Paren.Loc, diag::warn_redundant_parens_around_declarator)
+        << ParenRange << FixItHint::CreateRemoval(Paren.Loc)
+        << FixItHint::CreateRemoval(Paren.EndLoc);
+    return;
+  }
+
+  S.Diag(Paren.Loc, diag::warn_parens_disambiguated_as_variable_declaration)
+      << ParenRange << D.getIdentifier();
+  auto *RD = T->getAsCXXRecordDecl();
+  if (!RD || !RD->hasDefinition() || RD->hasNonTrivialDestructor())
+    S.Diag(Paren.Loc, diag::note_raii_guard_add_name)
+        << FixItHint::CreateInsertion(Paren.Loc, " varname") << T
+        << D.getIdentifier();
+  // FIXME: A cast to void is probably a better suggestion in cases where it's
+  // valid (when there is no initializer and we're not in a condition).
+  S.Diag(D.getLocStart(), diag::note_function_style_cast_add_parentheses)
+      << FixItHint::CreateInsertion(D.getLocStart(), "(")
+      << FixItHint::CreateInsertion(S.getLocForEndOfToken(D.getLocEnd()), ")");
+  S.Diag(Paren.Loc, diag::note_remove_parens_for_variable_declaration)
+      << FixItHint::CreateRemoval(Paren.Loc)
+      << FixItHint::CreateRemoval(Paren.EndLoc);
+}
+
 /// Helper for figuring out the default CC for a function declarator type.  If
 /// this is the outermost chunk, then we can determine the CC from the
 /// declarator context.  If not, then this could be either a member function
@@ -4007,6 +4101,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
     IsQualifiedFunction &= DeclType.Kind == DeclaratorChunk::Paren;
     switch (DeclType.Kind) {
     case DeclaratorChunk::Paren:
+      if (i == 0)
+        warnAboutRedundantParens(S, D, T);
       T = S.BuildParenType(T);
       break;
     case DeclaratorChunk::BlockPointer:

clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp

@@ -73,6 +73,7 @@ void other_contexts() {
   int a;
   {
     X0::X0(a); // expected-error{{qualified reference to 'X0' is a constructor name rather than a type in this context}}
+    // expected-warning@-1 {{redundant parentheses around declaration of variable named 'a'}} expected-note@-1 2{{}}
   }
 }
 

clang/test/FixIt/fixit-vexing-parse.cpp

@@ -106,3 +106,24 @@ namespace N {
     wchar_t wc(); // expected-warning {{function declaration}} expected-note {{replace parentheses with an initializer}}
   }
 }
+
+namespace RedundantParens {
+struct Y {
+  Y();
+  Y(int);
+  ~Y();
+};
+int n;
+
+void test() {
+  // CHECK: add a variable name
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+7]]:4-[[@LINE+7]]:4}:" varname"
+  // CHECK: add enclosing parentheses
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+5]]:3-[[@LINE+5]]:3}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+4]]:7-[[@LINE+4]]:7}:")"
+  // CHECK: remove parentheses
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:4-[[@LINE+2]]:5}:" "
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:7}:""
+  Y(n); // expected-warning {{declaration of variable named 'n'}} expected-note 3{{}}
+}
+}

clang/test/Parser/cxx0x-condition.cpp

@@ -14,11 +14,11 @@ void f() {
   for (; int x = ++a; ) ;
 
   if (S(a)) {} // ok
-  if (S(a) = 0) {} // ok
+  if (S(a) = 0) {} // expected-warning {{redundant parentheses}} expected-note 2{{}}
   if (S(a) == 0) {} // ok
 
   if (S(n)) {} // expected-error {{unexpected type name 'n': expected expression}}
-  if (S(n) = 0) {} // ok
+  if (S(n) = 0) {} // expected-warning {{redundant parentheses}} expected-note 2{{}}
   if (S(n) == 0) {} // expected-error {{unexpected type name 'n': expected expression}}
 
   if (S b(a)) {} // expected-error {{variable declaration in condition cannot have a parenthesized initializer}}

clang/test/Parser/cxx1z-class-template-argument-deduction.cpp

@@ -180,6 +180,7 @@ namespace typename_specifier {
     {(void)(typename T::A)(0);} // expected-error{{refers to class template member}}
     {(void)(typename T::A){0};} // expected-error{{refers to class template member}}
     {typename T::A (parens) = 0;} // expected-error {{refers to class template member in 'typename_specifier::X'; argument deduction not allowed here}}
+    // expected-warning@-1 {{disambiguated as redundant parentheses around declaration of variable named 'parens'}} expected-note@-1 {{add a variable name}} expected-note@-1{{remove parentheses}} expected-note@-1 {{add enclosing parentheses}}
     {typename T::A *p = 0;} // expected-error {{refers to class template member}}
     {typename T::A &r = *p;} // expected-error {{refers to class template member}}
     {typename T::A arr[3] = 0;} // expected-error {{refers to class template member}}

clang/test/SemaCXX/decl-expr-ambiguity.cpp

@@ -48,13 +48,20 @@ void f() {
 
 struct RAII {
   RAII();
+  RAII(int);
   ~RAII();
 };
 
+struct NotRAII {
+  NotRAII();
+  NotRAII(int);
+};
+
 void func();
 void func2(short);
 namespace N {
   struct S;
+  int n;
 
   void emptyParens() {
     RAII raii(); // expected-warning {{function declaration}} expected-note {{remove parentheses to declare a variable}}
@@ -69,6 +76,23 @@ namespace N {
   void nonEmptyParens() {
     int f = 0, // g = 0; expected-note {{change this ',' to a ';' to call 'func2'}}
     func2(short(f)); // expected-warning {{function declaration}} expected-note {{add a pair of parentheses}}
+
+    RAII(n); // expected-warning {{parentheses were disambiguated as redundant parentheses around declaration of variable named 'n'}}
+    // expected-note@-1 {{add a variable name to declare a 'RAII' initialized with 'n'}}
+    // expected-note@-2 {{add enclosing parentheses to perform a function-style cast}}
+    // expected-note@-3 {{remove parentheses to silence this warning}}
+
+    RAII(undeclared1);
+#pragma clang diagnostic push
+#pragma clang diagnostic warning "-Wredundant-parens"
+    RAII(undeclared2); // expected-warning {{redundant parentheses surrounding declarator}}
+#pragma clang diagnostic pop
+
+    {
+      NotRAII(n); // expected-warning {{parentheses were disambiguated as redundant parentheses around declaration of variable named 'n'}}
+      // expected-note@-1 {{add enclosing parentheses to perform a function-style cast}}
+      // expected-note@-2 {{remove parentheses to silence this warning}}
+    }
   }
 }