[cfe-commits] [patch] PR10127/N3031 supporting decltype in nested-namespace-specifiers

[Johannes Schaub]

David Blaikie wrote:

> On Mon, Dec 5, 2011 at 2:58 PM, Johannes Schaub
> <schaub.johannes at googlemail.com> wrote:
>> Douglas Gregor wrote:
>>
>>>
>>> On Nov 14, 2011, at 8:10 AM, David Blaikie wrote:
>>>
>>>>> Some comments follow…
>>>>
>>>> Thanks for the feedback.
>>>>
>>>>> Index: include/clang/Basic/TokenKinds.def
>>>>> ===================================================================
>>>>> --- include/clang/Basic/TokenKinds.def  (revision 143902)
>>>>> +++ include/clang/Basic/TokenKinds.def  (working copy)
>>>>> @@ -559,6 +559,8 @@
>>>>> // function template specialization (not a
>>>>> type), // e.g., "std::swap<int>"
>>>>> ANNOTATION(primary_expr) // annotation for a primary expression
>>>>> +ANNOTATION(decltype)     // annotation for a decltype expression,
>>>>> +                         // e.g., "decltype(foo.bar())"
>>>>>
>>>>> // Annotation for #pragma unused(...)
>>>>> // For each argument inside the parentheses the pragma handler will
>>>>> produce
>>>>>
>>>>> Did you consider using annot_typename for this? I ask because
>>>>> annot_typename handles general types (including decltype), and is
>>>>> already well-supported throughout the parser. It looks like you've
>>>>> checked for annot_decltype in several places, but others (such as the
>>>>> tentative parser) would be confused by this annotation token.
>>>>
>>>> No, I hadn't considered that - but at first blush I'd be concerned
>>>> that this might end up being /too/ permissive, but I'll have a go
>>>> mocking it up that way, it'll probably be tidier but I won't be sure
>>>> what other scenarios might be accidentally accepted.
>>>
>>> I'm fine with keeping annot_decltype. It's super-specific, so just make
>>> sure you're auditing the various places where we have to deal with,
>>> e.g., the 'decltype' keyword or the annot_typename token so you catch
>>> all of the places 'decltype' can show up.
>>>
>>>>> Index: include/clang/Basic/DiagnosticSemaKinds.td
>>>>> ===================================================================
>>>>> --- include/clang/Basic/DiagnosticSemaKinds.td  (revision 143902)
>>>>> +++ include/clang/Basic/DiagnosticSemaKinds.td  (working copy)
>>>>> @@ -3957,6 +3957,7 @@
>>>>> "conversion function from %0 to %1 invokes a deleted function">;
>>>>>
>>>>> def err_expected_class_or_namespace : Error<"expected a class or
>>>>> namespace">;
>>>>> +def err_expected_class : Error<"expected a decltype of a class">;
>>>>> def err_missing_qualified_for_redecl : Error<
>>>>> "must qualify the name %0 to declare %q1 in this scope">;
>>>>> def err_invalid_declarator_scope : Error<
>>>>>
>>>>> Could you provide a better error message here? Something that, at the
>>>>> very least, tells the user what the actual type is? Also, enum types
>>>>> would also be fine in C++11, so the diagnostic isn't quite accurate.
>>>>
>>>> Sure - I was going off the (admittedly rather vague) existing message
>>>> above. I've rephrased this to, for example:
>>>>
>>>> 'decltype(int())' (aka 'int') is not a namespace, class, or scoped
>>>> enumeration
>>>>
>>>> I think it'd be helpful to use this diagnostic for the existing case
>>>> above. It comes up most easily if you have a typedef that resolves to
>>>> int for example:
>>>>
>>>> namespace foo { typedef int bar; }
>>>>
>>>> foo::bar::baz i; // "expected class or namespace" (^ points to the
>>>> 'b' in 'bar')
>>>>
>>>> & you don't get told what foo::bar is. Though my version is a bit more
>>>> verbose.
>>>
>>> It's definitely an improvement. We only want that 'or scoped
>>> enumeration' bit if we're in C++11.
>>>
>>
>> Wanna drop a little comment on this - the diagnostic fools users that a
>> scoped enumeration is required to use "::", but unscoped enumerations
>> ("enum foo { bar }") can be used likewise with the scope operator.
> 
> Actually that's (one of) the point - unscoped enumerations cannot be
> used in the same way:
> 
> enum foo { bar };
> foo f = foo::bar; // GCC: "foo is not a class or namespace", Clang:
> "Expected a class or namespace" (to be revised)
> 

Not quite. Are you using GCC and clang in C++11 mode? GCC accepts such code 
for me. That point of scoped enumerations is that they don't introduce the 
enumerator names into the enclosing scope. 

    enum class A { X };
    int a = X; // error

Their point is not that they exclusively allow A::X. The rules at 3.4.3 are 
stated such that any enumeration - scoped or unscoped - can be used prior to 
a "::". See the example at 7.2p10 demonstrating that.

Note that the normative text in 7.2p10 does not say that the enumerators of 
an unscoped enumeration are introduced into the declarative region of the 
enumeration - there is no such declarative region for unscoped enumerations 
(see 3.3.8). But still, name lookup by 3.4.3 allows "E::X" to refer to an 
unscoped enumerator of E, with E being an unscoped enumeration.

The fact that "E::X" may refer to an unscoped enumerator is slightly 
confusing, even to the point that the spec itself confuses it. See 
http://llvm.org/bugs/show_bug.cgi?id=9124 . Unfortunately my DR seems to 
have gone lost in some of the Core-DR maintainer mailboxes.