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

[David Blaikie]

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)

They can be used at the end of (technically 'after' - the NSS is the
thing before the final name) a nested name specifier, as in:

::foo f;

But not in the middle - though perhaps that's what you mean, that it's
really the attempt to scope within the name (the :: after 'foo', not
before) that is the problem & perhaps that isn't clear from the
message/pointer.

> Just my two cents, but I guess since you already figured out an IMO better
> wording ("lookup into a type without members"), this note is just for the
> record.

Yeah, I'm still not sure what the ideal wording is, really.

- David