Recover from missing typenames on template args for MSVC compatibility

[rnk]

Summary

While matching a non-type template argument against a known template
type parameter we now modify the AST's TemplateArgumentLoc to assume the
user wrote typename. Under -fms-compatibility, we downgrade our
diagnostic from an error to an extwarn.

[rsmith]

include/clang/Basic/DiagnosticSemaKinds.td

3115 ↗

(On Diff #10193)

This diagnostic text isn't going to work well with -fms-extensions -pedantic-errors. Maybe phrase this more similarly to the existing cases:

"template argument for template type parameter must be a type; omitted 'typename' is a Microsoft extension"

(I'm also not convinced that we should call this a "Microsoft extension", but I can live with it.)

lib/Sema/SemaExpr.cpp

2196 ↗

(On Diff #10193)

Our policy for fixits is to only put them on errors/warnings if we recover as if the fixit were applied (this is important for -fixit behavior). Either remove the fixit for now or move it to a note.

lib/Sema/SemaTemplate.cpp

5921–5923 ↗

(On Diff #10193)

This comment doesn't look quite right: we don't get here for references to a template-id. We might have an explicit instantiation or an explicit or partial specialization, but the one thing we *don't* have is an implicit instantiation.

This would also make more sense down next to Converted.

[rnk]

include/clang/Basic/DiagnosticSemaKinds.td

3115 ↗

(On Diff #10193)

I was trying to make the diagnostic more concise, but I can put it back. I think I understand the issue with -pedantic-errors. Is it that we shouldn't make the diagnostic sound like we recovered if it gets upgraded from an ExtWarn to an error?

I agree that "Microsoft extension" is maybe not the best way to describe these types of warnings, but it's consistent with every other -Wmicrosoft diagnostic.

lib/Sema/SemaExpr.cpp

2196 ↗

(On Diff #10193)

OK, I'll remove the fixit. The recovery was unnecessary because now we recover prior to instantiation.

lib/Sema/SemaTemplate.cpp

5921–5923 ↗

(On Diff #10193)

Woops, this should be reverted. I wrote this when I was half-way through understanding the problem.

[rnk]

• Fix wording, remove fixit, and revert bad comment

[rsmith]

LGTM

[rnk]

Closed by commit rL210607 (authored by @rnk).

cfe/trunk/include/clang/AST/TemplateBase.h

public:
   const TemplateArgumentLoc *getArgumentArray() const {
     return Arguments.data();
   }
 
   const TemplateArgumentLoc &operator[](unsigned I) const {
     return Arguments[I];
   }
 
+  TemplateArgumentLoc &operator[](unsigned I) {
+    return Arguments[I];
+  }
+
   void addArgument(const TemplateArgumentLoc &Loc) {
     Arguments.push_back(Loc);
   }
 };
 
 /// \brief Represents an explicit template argument list in C++, e.g.,
 /// the "<int>" in "sort<int>".
 /// This is safe to be used inside an AST node, in contrast with

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def err_template_arg_list_different_arity : Error<
   "%select{too few|too many}0 template arguments for "
   "%select{class template|function template|template template parameter"
   "|template}1 %2">;
 def note_template_decl_here : Note<"template is declared here">;
 def err_template_arg_must_be_type : Error<
   "template argument for template type parameter must be a type">;
 def err_template_arg_must_be_type_suggest : Error<
   "template argument for template type parameter must be a type; did you forget 'typename'?">;
+def ext_ms_template_type_arg_missing_typename : ExtWarn<
+  "template argument for template type parameter must be a type; "
+  "omitted 'typename' is a Microsoft extension">,
+  InGroup<Microsoft>;
 def err_template_arg_must_be_expr : Error<
   "template argument for non-type template parameter must be an expression">;
 def err_template_arg_nontype_ambig : Error<
   "template argument for non-type template parameter is treated as function type %0">;
 def err_template_arg_must_be_template : Error<
   "template argument for template template parameter must be a class template%select{| or type alias template}0">;
 def ext_template_arg_local_type : ExtWarn<
   "template argument uses local type %0">, InGroup<LocalTypeTemplateArgs>;

cfe/trunk/include/clang/Sema/Sema.h

enum CheckTemplateArgumentKind {
     CTAK_Deduced,
 
     /// \brief The template argument was deduced from an array bound
     /// via template argument deduction.
     CTAK_DeducedFromArrayBound
   };
 
   bool CheckTemplateArgument(NamedDecl *Param,
-                             const TemplateArgumentLoc &Arg,
+                             TemplateArgumentLoc &Arg,
                              NamedDecl *Template,
                              SourceLocation TemplateLoc,
                              SourceLocation RAngleLoc,
                              unsigned ArgumentPackIndex,
                            SmallVectorImpl<TemplateArgument> &Converted,
                              CheckTemplateArgumentKind CTAK = CTAK_Specified);
 
   /// \brief Check that the given template arguments can be be provided to

   /// \returns true if an error occurred, false otherwise.
   bool CheckTemplateArgumentList(TemplateDecl *Template,
                                  SourceLocation TemplateLoc,
                                  TemplateArgumentListInfo &TemplateArgs,
                                  bool PartialTemplateArgs,
                            SmallVectorImpl<TemplateArgument> &Converted);
 
   bool CheckTemplateTypeArgument(TemplateTypeParmDecl *Param,
-                                 const TemplateArgumentLoc &Arg,
+                                 TemplateArgumentLoc &Arg,
                            SmallVectorImpl<TemplateArgument> &Converted);
 
   bool CheckTemplateArgument(TemplateTypeParmDecl *Param,
                              TypeSourceInfo *Arg);
   ExprResult CheckTemplateArgument(NonTypeTemplateParmDecl *Param,
                                    QualType InstantiatedParamType, Expr *Arg,
                                    TemplateArgument &Converted,
                                CheckTemplateArgumentKind CTAK = CTAK_Specified);
   bool CheckTemplateArgument(TemplateTemplateParmDecl *Param,
-                             const TemplateArgumentLoc &Arg,
+                             TemplateArgumentLoc &Arg,
                              unsigned ArgumentPackIndex);
 
   ExprResult
   BuildExpressionFromDeclTemplateArgument(const TemplateArgument &Arg,
                                           QualType ParamType,
                                           SourceLocation Loc);
   ExprResult
   BuildExpressionFromIntegralTemplateArgument(const TemplateArgument &Arg,

cfe/trunk/lib/Sema/SemaExpr.cpp

return BuildDependentDeclRefExpr(SS, /*TemplateKWLoc=*/SourceLocation(),
                                      NameInfo, /*TemplateArgs=*/nullptr);
 
   if (R.empty()) {
     Diag(NameInfo.getLoc(), diag::err_no_member)
       << NameInfo.getName() << DC << SS.getRange();
     return ExprError();
   }
 
+  if (R.isSingleResult() && R.getAsSingle<TypeDecl>()) {
+    // Diagnose a missing typename if this resolved unambiguously to a type in a
+    // dependent context.
+    // FIXME: Issue a fixit and recover as though the user had written
+    // 'typename'.
+    Diag(SS.getBeginLoc(), diag::err_typename_missing)
+        << SS.getScopeRep() << NameInfo.getName().getAsString()
+        << SourceRange(SS.getBeginLoc(), NameInfo.getEndLoc());
+    return ExprError();
+  }
+
   // Defend against this resolving to an implicit member access. We usually
   // won't get here if this might be a legitimate a class member (we end up in
   // BuildMemberReferenceExpr instead), but this can be valid if we're forming
   // a pointer-to-member or in an unevaluated context in C++11.
   if (!R.empty() && (*R.begin())->isCXXClassMember() && !IsAddressOfOperand)
     return BuildPossibleImplicitMemberExpr(SS,
                                            /*TemplateKWLoc=*/SourceLocation(),
                                            R, /*TemplateArgs=*/nullptr);

cfe/trunk/lib/Sema/SemaTemplate.cpp

Diag(Name.getLocStart(),
        diag::err_template_kw_refers_to_non_template)
     << GetNameFromUnqualifiedId(Name).getName()
     << Name.getSourceRange()
     << TemplateKWLoc;
   return TNK_Non_template;
 }
 
 bool Sema::CheckTemplateTypeArgument(TemplateTypeParmDecl *Param,
-                                     const TemplateArgumentLoc &AL,
+                                     TemplateArgumentLoc &AL,
                           SmallVectorImpl<TemplateArgument> &Converted) {
   const TemplateArgument &Arg = AL.getArgument();
+  QualType ArgType;
+  TypeSourceInfo *TSI = nullptr;
 
   // Check template type parameter.
   switch(Arg.getKind()) {
   case TemplateArgument::Type:
     // C++ [temp.arg.type]p1:
     //   A template-argument for a template-parameter which is a
     //   type shall be a type-id.
+    ArgType = Arg.getAsType();
+    TSI = AL.getTypeSourceInfo();
     break;
   case TemplateArgument::Template: {
     // We have a template type parameter but the template argument
     // is a template without any arguments.
     SourceRange SR = AL.getSourceRange();
     TemplateName Name = Arg.getAsTemplate();
     Diag(SR.getBegin(), diag::err_template_missing_args)
       << Name << SR;
case TemplateArgument::Expression: {
     } else if (CXXDependentScopeMemberExpr *ArgExpr =
                dyn_cast<CXXDependentScopeMemberExpr>(Arg.getAsExpr())) {
       if (ArgExpr->isImplicitAccess()) {
         SS.Adopt(ArgExpr->getQualifierLoc());
         NameInfo = ArgExpr->getMemberNameInfo();
       }
     }
 
-    if (NameInfo.getName().isIdentifier()) {
+    if (auto *II = NameInfo.getName().getAsIdentifierInfo()) {
       LookupResult Result(*this, NameInfo, LookupOrdinaryName);
       LookupParsedName(Result, CurScope, &SS);
 
       if (Result.getAsSingle<TypeDecl>() ||
           Result.getResultKind() ==
               LookupResult::NotFoundInCurrentInstantiation) {
-        // FIXME: Add a FixIt and fix up the template argument for recovery.
+        // Suggest that the user add 'typename' before the NNS.
         SourceLocation Loc = AL.getSourceRange().getBegin();
-        Diag(Loc, diag::err_template_arg_must_be_type_suggest);
+        Diag(Loc, getLangOpts().MSVCCompat
+                      ? diag::ext_ms_template_type_arg_missing_typename
+                      : diag::err_template_arg_must_be_type_suggest)
+            << FixItHint::CreateInsertion(Loc, "typename ");
         Diag(Param->getLocation(), diag::note_template_param_here);
-        return true;
+
+        // Recover by synthesizing a type using the location information that we
+        // already have.
+        ArgType =
+            Context.getDependentNameType(ETK_Typename, SS.getScopeRep(), II);
+        TypeLocBuilder TLB;
+        DependentNameTypeLoc TL = TLB.push<DependentNameTypeLoc>(ArgType);
+        TL.setElaboratedKeywordLoc(SourceLocation(/*synthesized*/));
+        TL.setQualifierLoc(SS.getWithLocInContext(Context));
+        TL.setNameLoc(NameInfo.getLoc());
+        TSI = TLB.getTypeSourceInfo(Context, ArgType);
+
+        // Overwrite our input TemplateArgumentLoc so that we can recover
+        // properly.
+        AL = TemplateArgumentLoc(TemplateArgument(ArgType),
+                                 TemplateArgumentLocInfo(TSI));
+
+        break;
       }
     }
     // fallthrough
   }
   default: {
     // We have a template type parameter but the template argument
     // is not a type.
     SourceRange SR = AL.getSourceRange();
     Diag(SR.getBegin(), diag::err_template_arg_must_be_type) << SR;
     Diag(Param->getLocation(), diag::note_template_param_here);
 
     return true;
   }
   }
 
-  if (CheckTemplateArgument(Param, AL.getTypeSourceInfo()))
+  if (CheckTemplateArgument(Param, TSI))
     return true;
 
   // Add the converted template type argument.
-  QualType ArgType = Context.getCanonicalType(Arg.getAsType());
+  ArgType = Context.getCanonicalType(ArgType);
   
   // Objective-C ARC:
   //   If an explicitly-specified template argument type is a lifetime type
   //   with no lifetime qualifier, the __strong lifetime qualifier is inferred.
   if (getLangOpts().ObjCAutoRefCount &&
       ArgType->isObjCLifetimeType() &&
       !ArgType.getObjCLifetime()) {
     Qualifiers Qs;

 /// \param Converted The checked, converted argument will be added to the
 /// end of this small vector.
 ///
 /// \param CTAK Describes how we arrived at this particular template argument:
 /// explicitly written, deduced, etc.
 ///
 /// \returns true on error, false otherwise.
 bool Sema::CheckTemplateArgument(NamedDecl *Param,
-                                 const TemplateArgumentLoc &Arg,
+                                 TemplateArgumentLoc &Arg,
                                  NamedDecl *Template,
                                  SourceLocation TemplateLoc,
                                  SourceLocation RAngleLoc,
                                  unsigned ArgumentPackIndex,
                             SmallVectorImpl<TemplateArgument> &Converted,
                                  CheckTemplateArgumentKind CTAK) {
   // Check template type parameters.
   if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(Param))

 }
 
 /// \brief Check a template argument against its corresponding
 /// template template parameter.
 ///
 /// This routine implements the semantics of C++ [temp.arg.template].
 /// It returns true if an error occurred, and false otherwise.
 bool Sema::CheckTemplateArgument(TemplateTemplateParmDecl *Param,
-                                 const TemplateArgumentLoc &Arg,
+                                 TemplateArgumentLoc &Arg,
                                  unsigned ArgumentPackIndex) {
   TemplateName Name = Arg.getArgument().getAsTemplateOrTemplatePattern();
   TemplateDecl *Template = Name.getAsTemplateDecl();
   if (!Template) {
     // Any dependent template name is fine.
     assert(Name.isDependent() && "Non-dependent template isn't a declaration?");
     return false;
   }

cfe/trunk/test/SemaTemplate/typename-specifier.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-unused
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-unused -fms-compatibility -DMSVC
 namespace N {
   struct A {
     typedef int type;
   };
 
   struct B {
   };
 
class ExampleClass1 {
 
 
   struct ExampleItemSet {
     typedef ExampleItem* iterator;
     ExampleItem* operator[](unsigned);
   };
 
   void foo() {
-    pair<ExampleItemSet::iterator, int> i; // expected-error {{template argument for template type parameter must be a type; did you forget 'typename'?}}
+#ifdef MSVC
+    // expected-warning@+4 {{omitted 'typename' is a Microsoft extension}}
+#else
+    // expected-error@+2 {{template argument for template type parameter must be a type; did you forget 'typename'?}}
+#endif
+    pair<ExampleItemSet::iterator, int> i;
     pair<this->ExampleItemSet::iterator, int> i; // expected-error-re {{template argument for template type parameter must be a type{{$}}}}
     pair<ExampleItemSet::operator[], int> i; // expected-error-re {{template argument for template type parameter must be a type{{$}}}}
   }
-  pair<ExampleItemSet::iterator, int> elt; // expected-error {{template argument for template type parameter must be a type; did you forget 'typename'?}}
+#ifdef MSVC
+    // expected-warning@+4 {{omitted 'typename' is a Microsoft extension}}
+#else
+  // expected-error@+2 {{template argument for template type parameter must be a type; did you forget 'typename'?}}
+#endif
+  pair<ExampleItemSet::iterator, int> elt;
 
 
   typedef map<int, ExampleItem*> ExampleItemMap;
 
   static void bar() {
-    pair<ExampleItemMap::iterator, int> i; // expected-error {{template argument for template type parameter must be a type; did you forget 'typename'?}}
-  }
-  pair<ExampleItemMap::iterator, int> entry; // expected-error {{template argument for template type parameter must be a type; did you forget 'typename'?}}
+#ifdef MSVC
+    // expected-warning@+4 {{omitted 'typename' is a Microsoft extension}}
+#else
+    // expected-error@+2 {{template argument for template type parameter must be a type; did you forget 'typename'?}}
+#endif
+    pair<ExampleItemMap::iterator, int> i;
+  }
+#ifdef MSVC
+    // expected-warning@+4 {{omitted 'typename' is a Microsoft extension}}
+#else
+  // expected-error@+2 {{template argument for template type parameter must be a type; did you forget 'typename'?}}
+#endif
+  pair<ExampleItemMap::iterator, int> entry;
   pair<bar, int> foobar; // expected-error {{template argument for template type parameter must be a type}}
 };
 } // namespace missing_typename
+
+namespace missing_typename_and_base {
+template <class T> struct Bar {}; // expected-note 1+ {{template parameter is declared here}}
+template <typename T>
+struct Foo : T {
+
+  // FIXME: MSVC accepts this code.
+  Bar<TypeInBase> x; // expected-error {{use of undeclared identifier 'TypeInBase'}}
+
+#ifdef MSVC
+  // expected-warning@+4 {{omitted 'typename' is a Microsoft extension}}
+#else
+  // expected-error@+2 {{must be a type; did you forget 'typename'?}}
+#endif
+  Bar<T::TypeInBase> y;
+
+#ifdef MSVC
+  // expected-warning@+4 {{omitted 'typename' is a Microsoft extension}}
+#else
+  // expected-error@+2 {{must be a type; did you forget 'typename'?}}
+#endif
+  Bar<T::NestedRD::TypeInNestedRD> z;
+
+};
+struct Base {
+  typedef int TypeInBase;
+  struct NestedRD {
+    typedef int TypeInNestedRD;
+  };
+};
+Foo<Base> x;
+} // namespace missing_typename_and_base
+
+namespace func_type_vs_construct_tmp {
+template <typename> struct S { typedef int type; };
+template <typename T> void f();
+template <int N> void f();
+
+// expected-error@+1 {{missing 'typename' prior to dependent type name 'S<int>::type'}}
+template <typename T> void g() { f</*typename*/ S<T>::type(int())>(); }
+
+// Adding typename does fix the diagnostic.
+template <typename T> void h() { f<typename S<T>::type(int())>(); }
+
+void j() {
+  g<int>(); // expected-note-re {{in instantiation {{.*}} requested here}}
+  h<int>();
+}
+} // namespace func_type_vs_construct_tmp
+
+namespace pointer_vs_multiply {
+int x;
+// expected-error@+1 {{missing 'typename' prior to dependent type name 'B::type_or_int'}}
+template <typename T> void g() { T::type_or_int * x; }
+// expected-error@+1 {{typename specifier refers to non-type member 'type_or_int' in 'pointer_vs_multiply::A'}}
+template <typename T> void h() { typename T::type_or_int * x; }
+
+struct A { static const int type_or_int = 5; }; // expected-note {{referenced member 'type_or_int' is declared here}}
+struct B { typedef int type_or_int; };
+
+void j() {
+  g<A>();
+  g<B>(); // expected-note-re {{in instantiation {{.*}} requested here}}
+  h<A>(); // expected-note-re {{in instantiation {{.*}} requested here}}
+  h<B>();
+}
+} // namespace pointer_vs_multiply