[cfe-commits] [PATCH] Allow typedef with unnecessary "typename" when ms-extensions are enabled.
will at indefiant.com
Wed May 2 14:57:13 PDT 2012
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
>>> assumes that SS is set to something. The parser will get back on its feet
>>> continue parsing the rest of the declaration correctly so it is not a
>> 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
>>> 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
>>> 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
>>> 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? :)
More information about the cfe-commits