[cfe-commits] [PATCH][MS][Review request] - Optional template keyword in Microsoft mode.

[Douglas Gregor]

On Mar 24, 2011, at 5:47 AM, Francois Pichet wrote:

> Hi,
> 
> Small parser hack to be able to parse type dependent template name
> without the "template" keyword in Microsoft mode.
> This patch fixes about a dozen compile errors when parsing MSVC STL
> headers with clang.
> 
> For example:
> =========
> template <class A>
> class C1 {
> public:
>  template <int B>
>  class Iterator {
>  };
> };
> 
> template<class T>
> class C2  {
>  // template is optional in microsoft mode.
>  typename C1<T>:: /*template*/  Iterator<0> Mypos;
> };
> 
> ok to commit?

This particular parser recovery would be great, not just for Microsoft mode but in general. So, I'd like to see this handled by making the parser recover well from a missing 'template' keyword (diagnostic + Fix-It) and then either downgrading the diagnostic (to a warning) or omitting it entirely for Microsoft mode.

+    // Microsoft mode parser hack to be able to parse type dependent 
+    // template name without the "template" keyword.  
+    bool IsImplicitTemplate = getLang().Microsoft && IsTypename &&
+                              Tok.is(tok::identifier) &&
+                              NextToken().is(tok::less) &&
+                              (getCurScope()->getFlags() & Scope::DeclScope) &&
+                              (HasScopeSpecifier || ObjectType);
+    
     // nested-name-specifier:
     //   nested-name-specifier 'template'[opt] simple-template-id '::'
 
     // Parse the optional 'template' keyword, then make sure we have
     // 'identifier <' after it.
-    if (Tok.is(tok::kw_template)) {
+    if (Tok.is(tok::kw_template) || IsImplicitTemplate) {
       // If we don't have a scope specifier or an object type, this isn't a
       // nested-name-specifier, since they aren't allowed to start with
       // 'template'.

I think this is the wrong place to perform the recovery, because we're forcing Clang to go down the dependent template-name path even in cases where we might be looking at a non-dependent template name.

Instead, I suggest improving recovery down in the identifier case, by improving the heuristics for the current diagnostic that inserts a dependent 'template' keyword:

      if (MemberOfUnknownSpecialization && (ObjectType || SS.isSet()) && 
          IsTemplateArgumentList(1)) {
        // We have something like t::getAs<T>, where getAs is a 
        // member of an unknown specialization. However, this will only
        // parse correctly as a template, so suggest the keyword 'template'
        // before 'getAs' and treat this as a dependent template name.
        Diag(Tok.getLocation(), diag::err_missing_dependent_template_keyword)
          << II.getName()
          << FixItHint::CreateInsertion(Tok.getLocation(), "template ");

It might be as easy as turning that last condition into "(IsTypename || IsTemplateArgumentList(1))".

	- Doug