Hi, Thanks for working on this!
On Wed, May 2, 2012 at 6:54 AM, William Wilson <[email protected]> wrote: > Hi All, > > This patch follows on from http://llvm.org/bugs/show_bug.cgi?id=12715 > and includes a test case for validation. Local testing shows no > regression. > > Original outline of issue from bug report: > > ---- > > I've run into a issue parsing headers where the following constructs are > (frequently) used and successfully compiled by the MSVC compilers: > > template <typename T> > class Foo > { > typedef typename T* pT; > }; > > The additional typename qualification is unnecessary and causes the > following > warning to be emitted from Parser::TryAnnotateTypeOrScopeToken(): > > warning: expected a qualified name after 'typename' > typedef typename T* pT; > > Which is fine and expected, however the parsing quickly fails with: > > error: expected ';' at end of declaration list > typedef typename T* pT; > > This appears to be due the return true (error occurred) from > TryAnnotateTypeOrScopeToken() even though the occurrence is treated as a > warning. > > 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. > 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. > ---- > > 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. > 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. -- Richard
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
