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

[William Wilson]

Apologies, hit reply rather than reply to all:

On 2 May 2012 23:49, William Wilson <will at indefiant.com> wrote:
>> Thanks for working on this!
>
> The least I could do, thanks for taking a look at it!
>
>>> Interestingly the original commit that introduced the warning (r130088 by
>>> fpichet) states the following:
>>>
>>> "Must "return true;" even if it is a warning because the rest of the code
>>> path
>>> assumes that SS is set to something. The parser will get back on its feet
>>> and
>>> continue parsing the rest of the declaration correctly so it is not a
>>> problem."
>>
>>
>> That's not right. Propagating an error return when we've not encountered an
>> error can lead to us silently not generating code for a declaration, and
>> other surprises.
>
> Good to know I'm on the right track.
>
>>> It's true that the rest of the code expects SS to be valid, but
>>> unfortunately
>>> the assumption that the "parser will get back on its feet" is not true
>>> with the
>>> current implementation.
>>>
>>> I tried a naive return false (no error) in the event of the
>>> diag::warn_expected_qualified_after_typename instance and the parser
>>> appeared
>>> to recover successfully, but being unfamiliar with the logic in question I
>>> wonder if that is the correct approach for recovery in this case?
>>
>>
>> Yes, this function should return false (no error) in this case. This
>> recovery should be applied irrespective of whether we're in Microsoft mode
>> -- this is the right way to recover from this, even if we're treating it as
>> an error.
>
> Okay, I'll prepare an updated patch tomorrow reflecting this behaviour.
>
>>>
>>> Other concerns:
>>>
>>> Do I need to annotate the typename token before the return false?
>>
>>
>> Yes. Some of the parser code assumes (reasonably!) that if
>> TryAnnotateTypeOrScopeToken returns false and the current token is an
>> identifier, then the current token does not name a type.
>
> Newbie question: I'll also have a go at this, but as the token is
> effectively ignored do I use tok::unknown for it's kind or
> tok::annot_typename? Or alternatively, am I barking up the wrong tree
> entirely?
>
>
>>> Additionally, this recovery state seems more applicable to
>>> ms-compatibility rather than ms-extensions, but as r130088 uses
>>> MicrosoftExt I chose to leave it as is. Any thoughts?
>>
>>
>> This is a pure extension (it doesn't change the meaning of any conforming
>> code), so there's no problem with it being in MicrosoftExt.
>
> I guess so, it's just an ugly extension. But aren't they all? :)
>
> Cheers,
> Will.