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

[Johannes Schaub]

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. 

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.