Author: rsmith Date: Fri Sep 29 16:57:25 2017 New Revision: 314570 URL: http://llvm.org/viewvc/llvm-project?rev=314570&view=rev Log: 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. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/DeclSpec.h cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp cfe/trunk/test/FixIt/fixit-vexing-parse.cpp cfe/trunk/test/Parser/cxx0x-condition.cpp cfe/trunk/test/Parser/cxx1z-class-template-argument-deduction.cpp cfe/trunk/test/SemaCXX/decl-expr-ambiguity.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314570&r1=314569&r2=314570&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep 29 16:57:25 2017 @@ -295,8 +295,20 @@ def warn_empty_parens_are_function_decl 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< Modified: cfe/trunk/include/clang/Sema/DeclSpec.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=314570&r1=314569&r2=314570&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/DeclSpec.h (original) +++ cfe/trunk/include/clang/Sema/DeclSpec.h Fri Sep 29 16:57:25 2017 @@ -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. Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=314570&r1=314569&r2=314570&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Fri Sep 29 16:57:25 2017 @@ -3065,6 +3065,7 @@ static void warnAboutAmbiguousFunction(S 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(S } } +/// 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 *GetFullTypeForDec 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: Modified: cfe/trunk/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp?rev=314570&r1=314569&r2=314570&view=diff ============================================================================== --- cfe/trunk/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp (original) +++ cfe/trunk/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp Fri Sep 29 16:57:25 2017 @@ -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{{}} } } Modified: cfe/trunk/test/FixIt/fixit-vexing-parse.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit-vexing-parse.cpp?rev=314570&r1=314569&r2=314570&view=diff ============================================================================== --- cfe/trunk/test/FixIt/fixit-vexing-parse.cpp (original) +++ cfe/trunk/test/FixIt/fixit-vexing-parse.cpp Fri Sep 29 16:57:25 2017 @@ -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{{}} +} +} Modified: cfe/trunk/test/Parser/cxx0x-condition.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx0x-condition.cpp?rev=314570&r1=314569&r2=314570&view=diff ============================================================================== --- cfe/trunk/test/Parser/cxx0x-condition.cpp (original) +++ cfe/trunk/test/Parser/cxx0x-condition.cpp Fri Sep 29 16:57:25 2017 @@ -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}} Modified: cfe/trunk/test/Parser/cxx1z-class-template-argument-deduction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx1z-class-template-argument-deduction.cpp?rev=314570&r1=314569&r2=314570&view=diff ============================================================================== --- cfe/trunk/test/Parser/cxx1z-class-template-argument-deduction.cpp (original) +++ cfe/trunk/test/Parser/cxx1z-class-template-argument-deduction.cpp Fri Sep 29 16:57:25 2017 @@ -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}} Modified: cfe/trunk/test/SemaCXX/decl-expr-ambiguity.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/decl-expr-ambiguity.cpp?rev=314570&r1=314569&r2=314570&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/decl-expr-ambiguity.cpp (original) +++ cfe/trunk/test/SemaCXX/decl-expr-ambiguity.cpp Fri Sep 29 16:57:25 2017 @@ -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}} + } } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits