================
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