<div class="gmail_quote">Hi,</div><div class="gmail_quote"><br></div><div class="gmail_quote">Thanks for working on this!</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Wed, May 2, 2012 at 6:54 AM, William Wilson <span dir="ltr"><<a href="mailto:will@indefiant.com" target="_blank">will@indefiant.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi All,<br>
<br>
This patch follows on from <a href="http://llvm.org/bugs/show_bug.cgi?id=12715" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=12715</a><br>
and includes a test case for validation. Local testing shows no<br>
regression.<br>
<br>
Original outline of issue from bug report:<br>
<br>
----<br>
<br>
I've run into a issue parsing headers where the following constructs are<br>
(frequently) used and successfully compiled by the MSVC compilers:<br>
<br>
template <typename T><br>
class Foo<br>
{<br>
  typedef typename T* pT;<br>
};<br>
<br>
The additional typename qualification is unnecessary and causes the following<br>
warning to be emitted from Parser::TryAnnotateTypeOrScopeToken():<br>
<br>
warning: expected a qualified name after 'typename'<br>
        typedef typename T* pT;<br>
<br>
Which is fine and expected, however the parsing quickly fails with:<br>
<br>
error: expected ';' at end of declaration list<br>
        typedef typename T* pT;<br>
<br>
This appears to be due the return true (error occurred) from<br>
TryAnnotateTypeOrScopeToken() even though the occurrence is treated as a<br>
warning.<br>
<br>
Interestingly the original commit that introduced the warning (r130088 by<br>
fpichet) states the following:<br>
<br>
"Must "return true;" even if it is a warning because the rest of the code path<br>
assumes that SS is set to something. The parser will get back on its feet and<br>
continue parsing the rest of the declaration correctly so it is not a problem."<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It's true that the rest of the code expects SS to be valid, but unfortunately<br>
the assumption that the "parser will get back on its feet" is not true with the<br>
current implementation.<br>
<br>
I tried a naive return false (no error) in the event of the<br>
diag::warn_expected_qualified_after_typename instance and the parser appeared<br>
to recover successfully, but being unfamiliar with the logic in question I<br>
wonder if that is the correct approach for recovery in this case?<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
----<br>
<br>
Other concerns:<br>
<br>
Do I need to annotate the typename token before the return false?<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Additionally, this recovery state seems more applicable to<br>
ms-compatibility rather than ms-extensions, but as r130088 uses<br>
MicrosoftExt I chose to leave it as is. Any thoughts?<br></blockquote><div><br></div><div>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.</div>
<div><br></div><div>-- Richard</div></div>