[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

[Richard Smith]

On Thu, January 5, 2012 04:50, Argyrios Kyrtzidis wrote:
> 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.

I don't agree with you on this point.

* The only reason there is no ambiguity in the grammar here is because () is
not accepted as an initializer, simply because it would always be ambiguous
(that is, the standard chooses to address this ambiguity in the grammar rather
than in 6.8). That distinction seems essentially irrelevant to the programmer.
If empty parentheses were allowed in the grammar for initializer, they would
mean value-initialization; if xs is an empty argument pack,
  T x(xs...)
declares a value-initialized variable, not a function. Though I note we
implement this incorrectly, and default-initialize x (PR11712)!

* Effective STL, Item 6 (where the phrase "most vexing parse" was first used
to describe this issue) includes this very case as "another manifestation of
this rule". As far as I'm aware, there is no definition of the term more
official than this item (which actually doesn't define it).

* The warning is -Wvexing-parse, not -Wmost-vexing-parse. The parse is
certainly vexing.

That said, I'll happy to change it to a clearer name, if you'd care to suggest
one.

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

I implemented this as a parser warning because I was looking for a construct
which would have parsed as an initializer, if an empty expression-list were
permitted (for instance, "T f(void)" is not such a case). However, I find your
example compelling; I'll look into moving this as you suggest.

Thanks!
Richard