[PATCH] Delay lookup of simple default template arguments under -fms-compatibility

[Richard Smith]

================
Comment at: include/clang/AST/Type.h:4059-4061
@@ -4058,5 +4058,5 @@
 ///
 /// DependentNameType represents a class of dependent types that involve a
 /// dependent nested-name-specifier (e.g., "T::") followed by a (dependent)
 /// name of a type. The DependentNameType may start with a "typename" (for a
 /// typename-specifier), "class", "struct", "union", or "enum" (for a
----------------
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.

================
Comment at: lib/Parse/ParseDecl.cpp:2764
@@ +2763,3 @@
+          getCurScope()->isTemplateParamScope() &&
+          DSContext == DSC_template_type_arg) {
+        TypeRep = Actions.ActOnDelayedDefaultTemplateArg(
----------------
David Majnemer wrote:
> Perhaps move the `DSContext == DSC_template_type_arg` check before the calls to `getLangOpts()` ? It should be a little cheaper.
I would think the most common reason this `if` condition is `false` is because `MSVCCompat` is `false` =) But I don't care either way.

================
Comment at: lib/Sema/SemaDecl.cpp:353-354
@@ +352,4 @@
+  DC = DC->getPrimaryContext();
+  NestedNameSpecifier *SubNNS = synthesizeCurrentNestedNameSpecifier(
+      Context, DC->getLookupParent(), NNS);
+  NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
----------------
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.

================
Comment at: lib/Sema/SemaDecl.cpp:358-359
@@ +357,4 @@
+    return NestedNameSpecifier::Create(Context, SubNNS, ND);
+  else if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC))
+    return NestedNameSpecifier::Create(Context, SubNNS, RD->isTemplateDecl(),
+                                       RD->getTypeForDecl());
----------------
David Majnemer wrote:
> Perhaps `auto RD = dyn_cast<CXXRecordDecl(DC))` ?
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.)

================
Comment at: lib/Sema/SemaDecl.cpp:361
@@ +360,3 @@
+                                       RD->getTypeForDecl());
+  return SubNNS;
+}
----------------
Once we hit this, we should bail out entirely. (This can currently include local classes in the name specifier, which doesn't seem right.)

================
Comment at: test/SemaTemplate/ms-delayed-default-template-args.cpp:6-9
@@ +5,6 @@
+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<>;
+}
----------------
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;

http://reviews.llvm.org/D3995