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

Will Wilson will at indefiant.com
Fri May 11 11:21:57 PDT 2012


Hi Richard et al.

Were there any objections/comments/improvements for this updated
patch? On a purely selfish level it'd be great to get a few of these
MSExt patches in to avoid juggling too many fixes locally... Although
maybe I'd better move to git soon :)

Cheers,
Will.

On 8 May 2012 12:58, Will Wilson <will at indefiant.com> wrote:
>>>> > You should consume the 'typename' token, and try to recover as if the
>>>> > 'typename' keyword were not present, following the logic present later
>>>> > on in
>>>> > that function. As a special case, if that later logic were to find that
>>>> > the
>>>> > tokens after the 'typename' keyword can't be annotated as a type name,
>>>> > you
>>>> > should produce a hard error (even with MS extensions enabled).
>>>> >
>>>> > (In particular, for 'typename identifier', it will be the identifier you
>>>> > annotate, not the 'typename' keyword.)
>>>>
>>>> Sorry for the delay! I've attached an updated patch (with test cases)
>>>> that seems to do the job. I've also had to update two other tests to
>>>> allow for the different code paths (and thus different errors)
>>>> followed due to the attempt at recovering from the error.
>>>>
>>>> All tests pass locally. It'd be great if some other people could
>>>> verify it as well.
>>>
>>>
>>> A few things: This is still only producing a warning if 'typename' is
>>> followed by something which isn't a type name in MS mode. That needs to be
>>> fixed to produce an error in that case. Also, it would be great to apply
>>> typo correction in this case, and to avoid duplicating the code from the
>>> second half of the function.
>>
>> Thanks for giving it the once over.
>>
>> Fair point on the error in MS mode, I'll rework it.
>>
>> I agree the code duplication in TryAnnotateTypeOrScopeToken is less
>> than optimal (I'll try the recursive approach you've suggested below).
>> The typo-correction sounds promising - if I get a change though I'll
>> investigate further (with the sound of deadlines rushing by ;).
>>
>>> How about just recursively calling TryAnnotateTypeOrScopeToken after
>>> consuming the invald 'typename' token, then emitting the error diagnostic if
>>> it fails or we're not in MS mode, and the warning diagnostic otherwise?
>>
>> That actually looks like a promising approach... I'll give it a shot
>> tomorrow if time allows.
>
> Time did allow. The patch is attached using the recursion approach and
> (on the whole) looks a great deal better. Only
> Parser::ParseCastExpression() needed updating to ensure it didn't
> simple assume it got an annotated token back from
> TryAnnotateTypeOrScopeToken(). All tests pass locally.
>
> Cheers,
> Will.




More information about the cfe-commits mailing list