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

Richard Smith richard at metafoo.co.uk
Wed May 2 13:35:37 PDT 2012


Thanks for working on this!

On Wed, May 2, 2012 at 6:54 AM, William Wilson <will at indefiant.com> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120502/878f63fb/attachment.html>

More information about the cfe-commits mailing list