From 69f90dce49124c98838ad80914c0c6642311167c Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 5 Jan 2012 04:12:21 +0000 Subject: [PATCH] PR10828: Produce a warning when a no-arguments function is declared in block scope, when no other indication is provided that the user intended to declare a function rather than a variable. Remove some false positives from the existing 'parentheses disambiguated as a function' warning by suppressing it when the declaration is marked as 'typedef' or 'extern'. Add a new warning group -Wvexing-parse containing both of these warnings. The new warning is enabled by default; despite a number of false positives (and one bug) in clang's test-suite, I have only found genuine bugs with it when running it over a significant quantity of real C++ code. llvm-svn: 147599 --- clang/include/clang/Basic/DiagnosticGroups.td | 1 + clang/include/clang/Basic/DiagnosticParseKinds.td | 6 +++++- clang/include/clang/Sema/DeclSpec.h | 7 +++++++ clang/lib/Parse/ParseDecl.cpp | 15 +++++++++++++++ clang/test/CXX/basic/basic.link/p9.cpp | 3 +-- .../basic.namespace/namespace.udecl/p11.cpp | 13 ++++++------- clang/test/Misc/warning-flags.c | 3 +-- clang/test/SemaCXX/conditional-expr.cpp | 4 ++-- clang/test/SemaCXX/decl-expr-ambiguity.cpp | 9 +++++++-- .../class-template-ctor-initializer.cpp | 2 +- 10 files changed, 46 insertions(+), 17 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index c54abaa1dc40..9c0358041331 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -231,6 +231,7 @@ def OverridingMethodMismatch : DiagGroup<"overriding-method-mismatch">; def : DiagGroup<"variadic-macros">; def VariadicMacros : DiagGroup<"variadic-macros">; def VectorConversions : DiagGroup<"vector-conversions">; // clang specific +def VexingParse : DiagGroup<"vexing-parse">; def VLA : DiagGroup<"vla">; def VolatileRegisterVar : DiagGroup<"volatile-register-var">; diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 90d4721603fc..bd30f486b7a6 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -387,7 +387,11 @@ def err_expected_lparen_after_type : Error< def err_expected_equal_after_declarator : Error< "expected '=' after declarator">; def warn_parens_disambiguated_as_function_decl : Warning< - "parentheses were disambiguated as a function declarator">; + "parentheses were disambiguated as a function declarator">, + InGroup; +def warn_empty_parens_are_function_decl : Warning< + "empty parentheses interpreted as a function declaration">, + InGroup; def warn_dangling_else : Warning< "add explicit braces to avoid dangling else">, InGroup; diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h index 44823c8f93e4..a601210bf4b6 100644 --- a/clang/include/clang/Sema/DeclSpec.h +++ b/clang/include/clang/Sema/DeclSpec.h @@ -1626,6 +1626,13 @@ public: bool mayBeFollowedByCXXDirectInit() const { if (hasGroupingParens()) return false; + if (getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_typedef) + return false; + + if (getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_extern && + Context != FileContext) + return false; + switch (Context) { case FileContext: case BlockContext: diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index ee4a51e184a2..d3c821137239 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -1111,6 +1111,21 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, if (FirstDecl) DeclsInGroup.push_back(FirstDecl); + // In C++, "T var();" at block scope is probably an attempt to initialize a + // variable, not a function declaration. We don't catch this case earlier, + // since there is no ambiguity here. + if (getLang().CPlusPlus && Context != Declarator::FileContext && + D.getNumTypeObjects() == 1 && D.isFunctionDeclarator() && + D.getDeclSpec().getStorageClassSpecAsWritten() + == DeclSpec::SCS_unspecified && + D.getDeclSpec().getTypeSpecType() != DeclSpec::TST_void) { + DeclaratorChunk &C = D.getTypeObject(0); + if (C.Fun.NumArgs == 0 && !C.Fun.isVariadic && !C.Fun.TrailingReturnType && + C.Fun.getExceptionSpecType() == EST_None) + Diag(C.Loc, diag::warn_empty_parens_are_function_decl) + << SourceRange(C.Loc, C.EndLoc); + } + bool ExpectSemi = Context != Declarator::ForContext; // If we don't have a comma, it is either the end of the list (a ';') or an diff --git a/clang/test/CXX/basic/basic.link/p9.cpp b/clang/test/CXX/basic/basic.link/p9.cpp index bd16b02d7bad..c895253c68fb 100644 --- a/clang/test/CXX/basic/basic.link/p9.cpp +++ b/clang/test/CXX/basic/basic.link/p9.cpp @@ -6,6 +6,5 @@ namespace N { } // expected-note{{here}} // First bullet: two names with external linkage that refer to // different kinds of entities. void f() { - int N(); // expected-error{{redefinition}} + int N(); // expected-error{{redefinition}} expected-warning{{interpreted as a function declaration}} } - diff --git a/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp b/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp index 63b302265ea0..b1dcf4d01543 100644 --- a/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp +++ b/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp @@ -25,13 +25,13 @@ namespace test1 { namespace test2 { namespace ns { void foo(); } // expected-note 2 {{target of using declaration}} void test0() { - int foo(); // expected-note {{conflicting declaration}} + int foo(); // expected-note {{conflicting declaration}} expected-warning{{function declaration}} using ns::foo; // expected-error {{target of using declaration conflicts with declaration already in scope}} } void test1() { using ns::foo; //expected-note {{using declaration}} - int foo(); // expected-error {{declaration conflicts with target of using declaration already in scope}} + int foo(); // expected-error {{declaration conflicts with target of using declaration already in scope}} expected-warning{{function declaration}} } } @@ -39,7 +39,7 @@ namespace test3 { namespace ns { void foo(); } // expected-note 2 {{target of using declaration}} class Test0 { void test() { - int foo(); // expected-note {{conflicting declaration}} + int foo(); // expected-note {{conflicting declaration}} expected-warning{{function declaration}} using ns::foo; // expected-error {{target of using declaration conflicts with declaration already in scope}} } }; @@ -47,7 +47,7 @@ namespace test3 { class Test1 { void test() { using ns::foo; //expected-note {{using declaration}} - int foo(); // expected-error {{declaration conflicts with target of using declaration already in scope}} + int foo(); // expected-error {{declaration conflicts with target of using declaration already in scope}} expected-warning{{function declaration}} } }; } @@ -56,7 +56,7 @@ namespace test4 { namespace ns { void foo(); } // expected-note 2 {{target of using declaration}} template class Test0 { void test() { - int foo(); // expected-note {{conflicting declaration}} + int foo(); // expected-note {{conflicting declaration}} expected-warning{{function declaration}} using ns::foo; // expected-error {{target of using declaration conflicts with declaration already in scope}} } }; @@ -64,7 +64,7 @@ namespace test4 { template class Test1 { void test() { using ns::foo; //expected-note {{using declaration}} - int foo(); // expected-error {{declaration conflicts with target of using declaration already in scope}} + int foo(); // expected-error {{declaration conflicts with target of using declaration already in scope}} expected-warning{{function declaration}} } }; } @@ -91,4 +91,3 @@ namespace test5 { template class Test0; template class Test1; // expected-note {{in instantiation of member function}} } - diff --git a/clang/test/Misc/warning-flags.c b/clang/test/Misc/warning-flags.c index 5e05e0b87a08..42c09cbe7d1b 100644 --- a/clang/test/Misc/warning-flags.c +++ b/clang/test/Misc/warning-flags.c @@ -17,7 +17,7 @@ This test serves two purposes: The list of warnings below should NEVER grow. It should gradually shrink to 0. -CHECK: Warnings without flags (268): +CHECK: Warnings without flags (267): CHECK-NEXT: ext_anon_param_requires_type_specifier CHECK-NEXT: ext_anonymous_struct_union_qualified CHECK-NEXT: ext_array_init_copy @@ -211,7 +211,6 @@ CHECK-NEXT: warn_octal_escape_too_large CHECK-NEXT: warn_odr_tag_type_inconsistent CHECK-NEXT: warn_on_superclass_use CHECK-NEXT: warn_param_default_argument_redefinition -CHECK-NEXT: warn_parens_disambiguated_as_function_decl CHECK-NEXT: warn_partial_specs_not_deducible CHECK-NEXT: warn_pointer_attribute_wrong_type CHECK-NEXT: warn_pp_convert_lhs_to_positive diff --git a/clang/test/SemaCXX/conditional-expr.cpp b/clang/test/SemaCXX/conditional-expr.cpp index 5648d022b522..3cfddb3a416b 100644 --- a/clang/test/SemaCXX/conditional-expr.cpp +++ b/clang/test/SemaCXX/conditional-expr.cpp @@ -96,8 +96,8 @@ void test() (void)(i1 ? BadDerived() : BadBase()); // b2.1 (hierarchy stuff) - const Base constret(); - const Derived constder(); + const Base constret(); // expected-warning {{interpreted as a function declaration}} + const Derived constder(); // expected-warning {{interpreted as a function declaration}} // should use const overload A a1((i1 ? constret() : Base()).trick()); A a2((i1 ? Base() : constret()).trick()); diff --git a/clang/test/SemaCXX/decl-expr-ambiguity.cpp b/clang/test/SemaCXX/decl-expr-ambiguity.cpp index 1ddff8058a3b..555e89ca7de2 100644 --- a/clang/test/SemaCXX/decl-expr-ambiguity.cpp +++ b/clang/test/SemaCXX/decl-expr-ambiguity.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -pedantic-errors %s +// RUN: %clang_cc1 -fsyntax-only -verify -pedantic-errors %s void f() { int a; @@ -24,10 +24,15 @@ void f() { // Declarations. int fd(T(a)); // expected-warning {{parentheses were disambiguated as a function declarator}} T(*d)(int(p)); // expected-warning {{parentheses were disambiguated as a function declarator}} expected-note {{previous definition is here}} + typedef T(*td)(int(p)); + extern T(*tp)(int(p)); + T d3(); // expected-warning {{empty parentheses interpreted as a function declaration}} + typedef T d3t(); + extern T f3(); T(d)[5]; // expected-error {{redefinition of 'd'}} typeof(int[])(f) = { 1, 2 }; // expected-error {{extension used}} void(b)(int); - int(d2) __attribute__(()); + int(d2) __attribute__(()); if (int(a)=1) {} int(d3(int())); } diff --git a/clang/test/SemaTemplate/class-template-ctor-initializer.cpp b/clang/test/SemaTemplate/class-template-ctor-initializer.cpp index 81a5e2b9c662..44bb4bda791e 100644 --- a/clang/test/SemaTemplate/class-template-ctor-initializer.cpp +++ b/clang/test/SemaTemplate/class-template-ctor-initializer.cpp @@ -49,7 +49,7 @@ namespace PR7259 { int main (void) { - Final final(); + Final final; return 0; } }