================
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
----------------
Richard Smith wrote:
> 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.
Done

================
Comment at: lib/Parse/ParseDecl.cpp:2764
@@ +2763,3 @@
+          getCurScope()->isTemplateParamScope() &&
+          DSContext == DSC_template_type_arg) {
+        TypeRep = Actions.ActOnDelayedDefaultTemplateArg(
----------------
Richard Smith wrote:
> 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.
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.

================
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);
----------------
Richard Smith wrote:
> 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.
OK.

================
Comment at: lib/Sema/SemaDecl.cpp:358
@@ +357,3 @@
+    return NestedNameSpecifier::Create(Context, SubNNS, ND);
+  else if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC))
+    return NestedNameSpecifier::Create(Context, SubNNS, RD->isTemplateDecl(),
----------------
Reid Kleckner wrote:
> Richard Smith wrote:
> > 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.)
> 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<>;
done

================
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());
----------------
Richard Smith wrote:
> 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.)
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<>;

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

================
Comment at: lib/Sema/SemaDecl.cpp:366
@@ +365,3 @@
+                                                SourceLocation NameLoc) {
+  Diag(NameLoc, diag::ext_ms_delayed_template_argument) << &II;
+
----------------
David Majnemer wrote:
> Perhaps a comment as to why we are emitting this diagnostic?
done

================
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<>;
+}
----------------
Richard Smith wrote:
> 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;
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.

http://reviews.llvm.org/D3995



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to