Apologies, hit reply rather than reply to all: On 2 May 2012 23:49, William Wilson <[email protected]> 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.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
