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

[David Blaikie]

On Mon, Dec 5, 2011 at 3:27 PM, Johannes Schaub
<schaub.johannes at googlemail.com> wrote:
> 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?

Indeed I wasn't. In -std=c++0x both GCC & Clang accept the above.
Thanks for the correction & standard pointers.

> 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.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits