Delay lookup of simple default template arguments under -fms-compatibility

[rnk]

Summary

MSVC delays parsing of default arguments until instantiation. If the
default argument is never used, it is never parsed. We don't model
this.

Instead, if lookup of a type name fails in a template argument context,
we form a DependentNameType, which will be looked up at instantiation
time.

This fixes errors about 'CControlWinTraits' in atlwin.h.

[majnemer]

lib/Parse/ParseDecl.cpp

2764

Perhaps move the DSContext == DSC_template_type_arg check before the calls to getLangOpts() ? It should be a little cheaper.

lib/Sema/SemaDecl.cpp

358

Perhaps auto RD = dyn_cast<CXXRecordDecl(DC)) ?

366

Perhaps a comment as to why we are emitting this diagnostic?

[rsmith]

include/clang/AST/Type.h

4059–4061

Please update this comment to indicate we also use DependentNameType in cases where we know we have a type but wish to defer the lookup for MS compatibility.

lib/Parse/ParseDecl.cpp

2764

I would think the most common reason this if condition is false is because MSVCCompat is false =) But I don't care either way.

lib/Sema/SemaDecl.cpp

353–354

Hmm. I don't think we actually ever need to recurse. Just walk up to the context into which we should look up the name, and build a single-level NNS for that.

358–359

Should we really ever use a class as the nested name specifier here? If I have a nested class (template) inside a class (template), should I look up a missing base specifier type in the outer class, or in the innermost enclosing namespace? (I would expect the latter is the right answer.)

361

Once we hit this, we should bail out entirely. (This can currently include local classes in the name specifier, which doesn't seem right.)

test/SemaTemplate/ms-delayed-default-template-args.cpp

7–10

Maybe also add a test for

template<typename T> struct Something {};
template<typename T = Something<Bar>> struct SomethingElse;

... and

template<Something<Bar> *p> struct AnotherThing;

[rnk]

• Add tests and fix nits

include/clang/AST/Type.h

4059–4061

Done

lib/Parse/ParseDecl.cpp

2764

OK, I'll hoist the DSC check. I don't think we fail name lookup in template argument contexts very often, but I could be wrong.

lib/Sema/SemaDecl.cpp

353–354

OK.

358

done

358–359

We need to be able to do lookup in enclosing RecordDecl scopes. MSVC accepts this:

struct Outer {
  template <typename T = Baz> struct Foo {
    static_assert(sizeof(T) == 4, "should get int, not double");
  };
  typedef int Baz;
};
typedef double Baz;
template struct Outer::Foo<>;

361

Bail out entirely, as in, return nullptr, or as in, fail the lookup?

366

done

test/SemaTemplate/ms-delayed-default-template-args.cpp

7–10

I assume Bar is always undeclared in these tests.

MSVC accepts your first test case, but Clang doesn't with my patch. I'd need to find a way to delay instantiating Something<Bar> until later, which feels beyond the scope of this patch.

The second test is a negative test, right? Both we and MSVC need to know the type of the non-type template parameter.

[rsmith]

include/clang/AST/Type.h

4056–4057

You seem to be using this to represent an unqualified type in some cases? (You've made NNS optional.) A quick look indicates that downstream code will not do the right thing in this case. (TreeTransform will treat it as if an error has occurred, and bail out with no diagnostic, for instance.)

lib/Sema/SemaDecl.cpp

353–357

What about the translation unit? Should you create a :: NestedNameSpecifier for it? (Once you do that, the 'return nullptr;' below and the null NNS case in DependentNameType are unreachable.)

361

I think this has been made irrelevant by other changes.

test/SemaTemplate/ms-delayed-default-template-args.cpp

7–10

I had meant for these to both be negative tests (maybe with a FIXME for the first case).

[rnk]

include/clang/AST/Type.h

4056–4057

This should be fixed now by using GlobalSpecifier. I guess I have tests covering this case, but we would silently have an invalid AST, so they didn't fail.

lib/Sema/SemaDecl.cpp

353–357

OK, that works. :)

test/SemaTemplate/ms-delayed-default-template-args.cpp

7–10

OK, good. :)

[rnk]

• Add tests and fix nits
• Use GlobalSpecifier so the delayed dependent name is always qualified

[rsmith]

LGTM

lib/Sema/SemaDecl.cpp

349

This looks redundant.

[rnk]

Closed by commit rL210382 (authored by @rnk).

[rnk]

Thanks for the help and the review! r210382

include/clang/AST/Type.h

static void Profile(llvm::FoldingSetNodeID &ID, ElaboratedTypeKeyword Keyword,
     NamedType.Profile(ID);
   }
 
   static bool classof(const Type *T) {
     return T->getTypeClass() == Elaborated;
   }
 };
 
 /// \brief Represents a qualified type name for which the type name is
 /// dependent.
 ///
 /// DependentNameType represents a class of dependent types that involve a
-/// dependent nested-name-specifier (e.g., "T::") followed by a (dependent)
+/// possibly dependent nested-name-specifier (e.g., "T::") followed by a
 /// name of a type. The DependentNameType may start with a "typename" (for a
 /// typename-specifier), "class", "struct", "union", or "enum" (for a
 /// dependent elaborated-type-specifier), or nothing (in contexts where we
 /// know that we must be referring to a type, e.g., in a base class specifier).
+/// Typically the nested-name-specifier is dependent, but in MSVC compatibility
+/// mode, this type is used with non-dependent names to delay name lookup until
+/// instantiation.
 class DependentNameType : public TypeWithKeyword, public llvm::FoldingSetNode {
 
   /// \brief The nested name specifier containing the qualifier.
   NestedNameSpecifier *NNS;
 
   /// \brief The type that this typename specifier refers to.
   const IdentifierInfo *Name;
 
   DependentNameType(ElaboratedTypeKeyword Keyword, NestedNameSpecifier *NNS,
                     const IdentifierInfo *Name, QualType CanonType)
     : TypeWithKeyword(Keyword, DependentName, CanonType, /*Dependent=*/true,
                       /*InstantiationDependent=*/true,
                       /*VariablyModified=*/false,
                       NNS->containsUnexpandedParameterPack()),
-      NNS(NNS), Name(Name) {
-    assert(NNS->isDependent() &&
-           "DependentNameType requires a dependent nested-name-specifier");
-  }
+      NNS(NNS), Name(Name) {}
 
   friend class ASTContext;  // ASTContext creates these
 
 public:
   /// \brief Retrieve the qualification on this type.
   NestedNameSpecifier *getQualifier() const { return NNS; }
 
   /// \brief Retrieve the type named by the typename specifier as an

include/clang/Basic/DiagnosticSemaKinds.td

 def err_pointer_to_member_call_drops_quals : Error<
   "call to pointer to member function of type %0 drops '%1' qualifier%s2">;
 def err_pointer_to_member_oper_value_classify: Error<
   "pointer-to-member function type %0 can only be called on an "
   "%select{rvalue|lvalue}1">;
 def ext_ms_deref_template_argument: ExtWarn<
   "non-type template argument containing a dereference operation is a "
   "Microsoft extension">, InGroup<Microsoft>;
+def ext_ms_delayed_template_argument: ExtWarn<
+  "using the undeclared type %0 as a default template argument is a "
+  "Microsoft extension">, InGroup<Microsoft>;
 
 // C++ template specialization
 def err_template_spec_unknown_kind : Error<
   "can only provide an explicit specialization for a class template, function "
   "template, variable template, or a member function, static data member, "
   "%select{or member class|member class, or member enumeration}0 of a "
   "class template">;
 def note_specialized_entity : Note<

include/clang/Parse/Parser.h

private:
   /// out, there are other significant restrictions on specifiers than
   /// would be best implemented in the parser.
   enum DeclSpecContext {
     DSC_normal, // normal context
     DSC_class,  // class context, enables 'friend'
     DSC_type_specifier, // C++ type-specifier-seq or C specifier-qualifier-list
     DSC_trailing, // C++11 trailing-type-specifier in a trailing return type
     DSC_alias_declaration, // C++11 type-specifier-seq in an alias-declaration
-    DSC_top_level // top-level/namespace declaration context
+    DSC_top_level, // top-level/namespace declaration context
+    DSC_template_type_arg // template type argument context
   };
 
   /// Is this a context in which we are parsing just a type-specifier (or
   /// trailing-type-specifier)?
   static bool isTypeSpecifier(DeclSpecContext DSC) {
     switch (DSC) {
     case DSC_normal:
     case DSC_class:
     case DSC_top_level:
       return false;
 
+    case DSC_template_type_arg:
     case DSC_type_specifier:
     case DSC_trailing:
     case DSC_alias_declaration:
       return true;
     }
     llvm_unreachable("Missing DeclSpecContext case");
   }
 

include/clang/Sema/Sema.h

public:
   bool isMicrosoftMissingTypename(const CXXScopeSpec *SS, Scope *S);
   bool DiagnoseUnknownTypeName(IdentifierInfo *&II,
                                SourceLocation IILoc,
                                Scope *S,
                                CXXScopeSpec *SS,
                                ParsedType &SuggestedType,
                                bool AllowClassTemplates = false);
 
+  /// \brief For compatibility with MSVC, we delay parsing of some default
+  /// template type arguments until instantiation time.  Emits a warning and
+  /// returns a synthesized DependentNameType that isn't really dependent on any
+  /// other template arguments.
+  ParsedType ActOnDelayedDefaultTemplateArg(const IdentifierInfo &II,
+                                            SourceLocation NameLoc);
+
   /// \brief Describes the result of the name lookup and resolution performed
   /// by \c ClassifyName().
   enum NameClassificationKind {
     NC_Unknown,
     NC_Error,
     NC_Keyword,
     NC_Type,
     NC_Expression,

lib/AST/ASTContext.cpp

ASTContext::getParenType(QualType InnerType) const {
   ParenTypes.InsertNode(T, InsertPos);
   return QualType(T, 0);
 }
 
 QualType ASTContext::getDependentNameType(ElaboratedTypeKeyword Keyword,
                                           NestedNameSpecifier *NNS,
                                           const IdentifierInfo *Name,
                                           QualType Canon) const {
-  assert(NNS->isDependent() && "nested-name-specifier must be dependent");
-
   if (Canon.isNull()) {
     NestedNameSpecifier *CanonNNS = getCanonicalNestedNameSpecifier(NNS);
     ElaboratedTypeKeyword CanonKeyword = Keyword;
     if (Keyword == ETK_None)
       CanonKeyword = ETK_Typename;
     
     if (CanonNNS != NNS || CanonKeyword != Keyword)
       Canon = getDependentNameType(CanonKeyword, CanonNNS, Name);

lib/Parse/ParseDecl.cpp

 /// \param Context the declarator context, which is one of the
 /// Declarator::TheContext enumerator values.
 Parser::DeclSpecContext
 Parser::getDeclSpecContextFromDeclaratorContext(unsigned Context) {
   if (Context == Declarator::MemberContext)
     return DSC_class;
   if (Context == Declarator::FileContext)
     return DSC_top_level;
+  if (Context == Declarator::TemplateTypeArgContext)
+    return DSC_template_type_arg;
   if (Context == Declarator::TrailingReturnContext)
     return DSC_trailing;
   if (Context == Declarator::AliasDeclContext ||
       Context == Declarator::AliasTemplateContext)
     return DSC_alias_declaration;
   return DSC_normal;
 }
 
case tok::identifier: {
       //                allow the use of a typedef name as a type specifier.
       if (DS.isTypeAltiVecVector())
         goto DoneWithDeclSpec;
 
       ParsedType TypeRep =
         Actions.getTypeName(*Tok.getIdentifierInfo(),
                             Tok.getLocation(), getCurScope());
 
+      // MSVC: If we weren't able to parse a default template argument, and it's
+      // just a simple identifier, create a DependentNameType.  This will allow us
+      // to defer the name lookup to template instantiation time, as long we forge a
+      // NestedNameSpecifier for the current context.
+      if (!TypeRep && DSContext == DSC_template_type_arg &&
+          getLangOpts().MSVCCompat && getCurScope()->isTemplateParamScope()) {
+        TypeRep = Actions.ActOnDelayedDefaultTemplateArg(
+            *Tok.getIdentifierInfo(), Tok.getLocation());
+      }
+
       // If this is not a typedef name, don't parse it as part of the declspec,
       // it must be an implicit int or an error.
       if (!TypeRep) {
         ParsedAttributesWithRange Attrs(AttrFactory);
         if (ParseImplicitInt(DS, nullptr, TemplateInfo, AS, DSContext, Attrs)) {
           if (!Attrs.empty()) {
             AttrsLastTime = true;
             attrs.takeAllFrom(Attrs);

lib/Sema/SemaDecl.cpp

ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
   if (T.isNull()) {
     // If it's not plausibly a type, suppress diagnostics.
     Result.suppressDiagnostics();
     return ParsedType();
   }
   return ParsedType::make(T);
 }
 
+// Builds a fake NNS for the given decl context.
+static NestedNameSpecifier *
+synthesizeCurrentNestedNameSpecifier(ASTContext &Context, DeclContext *DC) {
+  DC = DC->getPrimaryContext();
+  for (; DC != nullptr; DC = DC->getLookupParent()) {
+    DC = DC->getPrimaryContext();
+    auto *ND = dyn_cast<NamespaceDecl>(DC);
+    if (ND && !ND->isInline() && !ND->isAnonymousNamespace())
+      return NestedNameSpecifier::Create(Context, nullptr, ND);
+    else if (auto *RD = dyn_cast<CXXRecordDecl>(DC))
+      return NestedNameSpecifier::Create(Context, nullptr, RD->isTemplateDecl(),
+                                         RD->getTypeForDecl());
+    else if (isa<TranslationUnitDecl>(DC))
+      return NestedNameSpecifier::GlobalSpecifier(Context);
+  }
+  llvm_unreachable("something isn't in TU scope?");
+}
+
+ParsedType Sema::ActOnDelayedDefaultTemplateArg(const IdentifierInfo &II,
+                                                SourceLocation NameLoc) {
+  // Accepting an undeclared identifier as a default argument for a template
+  // type parameter is a Microsoft extension.
+  Diag(NameLoc, diag::ext_ms_delayed_template_argument) << &II;
+
+  // Build a fake DependentNameType that will perform lookup into CurContext at
+  // instantiation time.  The name specifier isn't dependent, so template
+  // instantiation won't transform it.  It will retry the lookup, however.
+  NestedNameSpecifier *NNS =
+      synthesizeCurrentNestedNameSpecifier(Context, CurContext);
+  QualType T = Context.getDependentNameType(ETK_None, NNS, &II);
+
+  // Build type location information.  We synthesized the qualifier, so we have
+  // to build a fake NestedNameSpecifierLoc.
+  NestedNameSpecifierLocBuilder NNSLocBuilder;
+  NNSLocBuilder.MakeTrivial(Context, NNS, SourceRange(NameLoc));
+  NestedNameSpecifierLoc QualifierLoc = NNSLocBuilder.getWithLocInContext(Context);
+
+  TypeLocBuilder Builder;
+  DependentNameTypeLoc DepTL = Builder.push<DependentNameTypeLoc>(T);
+  DepTL.setNameLoc(NameLoc);
+  DepTL.setElaboratedKeywordLoc(SourceLocation());
+  DepTL.setQualifierLoc(QualifierLoc);
+  return CreateParsedType(T, Builder.getTypeSourceInfo(Context, T));
+}
+
 /// isTagName() - This method is called *for error recovery purposes only*
 /// to determine if the specified name is a valid tag name ("struct foo").  If
 /// so, this returns the TST for the tag corresponding to it (TST_enum,
 /// TST_union, TST_struct, TST_interface, TST_class).  This is used to diagnose
 /// cases in C where the user forgot to specify the tag.
 DeclSpec::TST Sema::isTagName(IdentifierInfo &II, Scope *S) {
   // Do a tag name lookup in this scope.
   LookupResult R(*this, &II, SourceLocation(), LookupTagName);

test/SemaTemplate/ms-delayed-default-template-args.cpp

+// RUN: %clang_cc1 -fms-compatibility -std=c++11 %s -verify
+
+// MSVC should compile this file without errors.
+
+namespace test_basic {
+template <typename T = Baz> // expected-warning {{using the undeclared type 'Baz' as a default template argument is a Microsoft extension}}
+struct Foo { T x; };
+typedef int Baz;
+template struct Foo<>;
+}
+
+namespace test_namespace {
+namespace nested {
+template <typename T = Baz> // expected-warning {{using the undeclared type 'Baz' as a default template argument is a Microsoft extension}}
+struct Foo {
+  static_assert(sizeof(T) == 4, "should get int, not double");
+};
+typedef int Baz;
+}
+typedef double Baz;
+template struct nested::Foo<>;
+}
+
+namespace test_inner_class_template {
+struct Outer {
+  template <typename T = Baz> // expected-warning {{using the undeclared type 'Baz' as a default template argument is a Microsoft extension}}
+  struct Foo {
+    static_assert(sizeof(T) == 4, "should get int, not double");
+  };
+  typedef int Baz;
+};
+typedef double Baz;
+template struct Outer::Foo<>;
+}
+
+namespace test_nontype_param {
+template <typename T> struct Bar { T x; };
+typedef int Qux;
+template <Bar<Qux> *P>
+struct Foo {
+};
+Bar<int> g;
+template struct Foo<&g>;
+}
+
+// MSVC accepts this, but Clang doesn't.
+namespace test_template_instantiation_arg {
+template <typename T> struct Bar { T x; };
+template <typename T = Bar<Weber>>  // expected-error {{use of undeclared identifier 'Weber'}}
+struct Foo {
+  static_assert(sizeof(T) == 4, "Bar should have gotten int");
+  // FIXME: These diagnostics are bad.
+}; // expected-error {{expected ',' or '>' in template-parameter-list}}
+// expected-warning@-1 {{does not declare anything}}
+typedef int Weber;
+}
+
+#ifdef __clang__
+// These are negative test cases that MSVC doesn't compile either.  Try to use
+// unique undeclared identifiers so typo correction doesn't find types declared
+// above.
+
+namespace test_undeclared_nontype_parm_type {
+template <Zargon N> // expected-error {{unknown type name 'Zargon'}}
+struct Foo { int x[N]; };
+typedef int Zargon;
+template struct Foo<4>;
+}
+
+namespace test_undeclared_nontype_parm_type_no_name {
+template <typename T, Asdf> // expected-error {{unknown type name 'Asdf'}}
+struct Foo { T x; };
+template struct Foo<int, 0>;
+}
+
+namespace test_undeclared_type_arg {
+template <typename T>
+struct Foo { T x; };
+template struct Foo<Yodel>; // expected-error {{use of undeclared identifier 'Yodel'}}
+}
+
+namespace test_undeclared_nontype_parm_arg {
+// Bury an undeclared type as a template argument to the type of a non-type
+// template parameter.
+template <typename T> struct Bar { T x; };
+
+template <Bar<Xylophone> *P> // expected-error {{use of undeclared identifier 'Xylophone'}}
+// expected-note@-1 {{template parameter is declared here}}
+struct Foo { };
+
+typedef int Xylophone;
+Bar<Xylophone> g;
+template struct Foo<&g>; // expected-error {{cannot be converted}}
+}
+
+#endif