[cfe-commits] [PATCH] Allow typedef with unnecessary "typename" when ms-extensions are enabled.

[Will Wilson]

>> This works for simple cases, but obviously looses the meaning of the
>> typename keyword which would impact any occurrences of annotated or
>> scoped types thereafter. That said, given this is MicrosoftExt
>> specific and it allows a little more existing code to compile without
>> too much code churn perhaps it's a reasonable approach? It doesn't
>> solve all of the MS compat issues in this area but it's a small
>> improvement after all.
>
> So, a couple of questions. Firstly, do you have an idea how common
> this pattern is? Do we need to be compatible with platform headers, or
> only for user code which uses those headers? Secondly, are there real
> cases where this patch doesn't go far enough (that is, where we
> actually need the 'typename' keyword to disambiguate something)? I
> *suspect* this patch will be fine in practice, because the 'typename'
> should be enough for any disambiguation, and once we know we're
> parsing a type, we shouldn't need the 'typename' keyword.

1. I haven't come across a huge number of these cases (I think 10
instances so far) but have found it now in two different codebases
(from two different companies). And no this isn't relating to platform
headers, this is very much user code, unfortunately I'm not the "user"
so changing comes with a degree of political pain.

2. There will be scoped cases ie. "typedef typename const T::foo_type
bar_type;" where it will fail, but in exactly the same way as it did
before. This simply deal with the common case I've found, typically
for declaring the const iterators for a container ie. typedef typename
const T* const_iterator;

>> If it's not too disgusting, let me know and I'll knock up a patch with
>> test cases...
>
> I have some concerns over the details of the implementation -- in
> particular, because you're not producing an annotation token,
> TryAnnotateTypeOrScopeToken could get called multiple times for the
> same 'typename' token (in a tentative parse then later in a full
> parse), resulting in duplicated diagnostics.

I agree, it's pretty bad form not producing an annotated token in this
case. However, that would either require detecting the CV case in the
caller and dealing with it before TryAnnotateTypeOrScopeToken were
called, or somehow parsing the CV-qualifier in
TryAnnotateTypeOrScopeToken before moving on to the identifier - but
that seems a lot of work for a compatibility workaround. I suppose it
might be possible to return true and then let the caller decide if it
can recover from the error? Any favorites, or all too ugly?

Thanks for the input!
Will.