[cfe-commits] r147599 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticParseKinds.td include/clang/Sema/DeclSpec.h lib/Parse/ParseDecl.cpp test/CXX/basic/basic.link/p9.cpp test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp test/Misc/warning-flags.c test/SemaCXX/conditional-expr.cpp test/SemaCXX/decl-expr-ambiguity.cpp test/SemaTemplate/class-template-ctor-initializer.cpp

Argyrios Kyrtzidis kyrtzidis at apple.com
Wed Jan 4 20:50:45 PST 2012


On Jan 4, 2012, at 8:12 PM, Richard Smith wrote:

> Author: rsmith
> Date: Wed Jan  4 22:12:21 2012
> New Revision: 147599
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=147599&view=rev
> Log:
> 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.

I love the warning but I have 2 objections to the implementation:

1) This is not what the "vexing parse" is, the case for the new warning is unambiguous from the perspective of the parser, so the name of the warning group for the new warning is incorrect.

2) The new warning belongs in the sema, not the parser, because it requires semantic checks; consider the following case:

void f() {
  typedef void VOID;
  VOID gg();  // false positive here.
}

-Argyrios

> 
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>    cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>    cfe/trunk/include/clang/Sema/DeclSpec.h
>    cfe/trunk/lib/Parse/ParseDecl.cpp
>    cfe/trunk/test/CXX/basic/basic.link/p9.cpp
>    cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp
>    cfe/trunk/test/Misc/warning-flags.c
>    cfe/trunk/test/SemaCXX/conditional-expr.cpp
>    cfe/trunk/test/SemaCXX/decl-expr-ambiguity.cpp
>    cfe/trunk/test/SemaTemplate/class-template-ctor-initializer.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Jan  4 22:12:21 2012
> @@ -231,6 +231,7 @@
> 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">;
> 
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Jan  4 22:12:21 2012
> @@ -387,7 +387,11 @@
> 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<VexingParse>;
> +def warn_empty_parens_are_function_decl : Warning<
> +  "empty parentheses interpreted as a function declaration">,
> +  InGroup<VexingParse>;
> def warn_dangling_else : Warning<
>   "add explicit braces to avoid dangling else">,
>   InGroup<DanglingElse>;
> 
> Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
> +++ cfe/trunk/include/clang/Sema/DeclSpec.h Wed Jan  4 22:12:21 2012
> @@ -1626,6 +1626,13 @@
>   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:
> 
> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed Jan  4 22:12:21 2012
> @@ -1111,6 +1111,21 @@
>   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
> 
> Modified: cfe/trunk/test/CXX/basic/basic.link/p9.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/basic/basic.link/p9.cpp?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/test/CXX/basic/basic.link/p9.cpp (original)
> +++ cfe/trunk/test/CXX/basic/basic.link/p9.cpp Wed Jan  4 22:12:21 2012
> @@ -6,6 +6,5 @@
> // 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}}
> }
> -
> 
> Modified: cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp (original)
> +++ cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p11.cpp Wed Jan  4 22:12:21 2012
> @@ -25,13 +25,13 @@
> 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 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 @@
>   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 ns { void foo(); } // expected-note 2 {{target of using declaration}}
>   template <typename> 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 @@
>   template <typename> 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 @@
>   template class Test0<int>;
>   template class Test1<int>; // expected-note {{in instantiation of member function}}
> }
> -
> 
> Modified: cfe/trunk/test/Misc/warning-flags.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/warning-flags.c?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/test/Misc/warning-flags.c (original)
> +++ cfe/trunk/test/Misc/warning-flags.c Wed Jan  4 22:12:21 2012
> @@ -17,7 +17,7 @@
> 
> 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_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
> 
> Modified: cfe/trunk/test/SemaCXX/conditional-expr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/conditional-expr.cpp?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/conditional-expr.cpp (original)
> +++ cfe/trunk/test/SemaCXX/conditional-expr.cpp Wed Jan  4 22:12:21 2012
> @@ -96,8 +96,8 @@
>   (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());
> 
> 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=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/decl-expr-ambiguity.cpp (original)
> +++ cfe/trunk/test/SemaCXX/decl-expr-ambiguity.cpp Wed Jan  4 22:12:21 2012
> @@ -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 @@
>   // 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()));
> }
> 
> Modified: cfe/trunk/test/SemaTemplate/class-template-ctor-initializer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/class-template-ctor-initializer.cpp?rev=147599&r1=147598&r2=147599&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaTemplate/class-template-ctor-initializer.cpp (original)
> +++ cfe/trunk/test/SemaTemplate/class-template-ctor-initializer.cpp Wed Jan  4 22:12:21 2012
> @@ -49,7 +49,7 @@
>   int
>   main (void)
>   {
> -    Final final();
> +    Final final;
>     return 0;
>   }
> }
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list