Only use MSVC-compatible dependent base class hack for class template members.

[kimgr]

Summary

Currently the code in SemaCXXScopeSpec does an MSVCCompat fallback to delay lookup of possibly dependent base class names. It does so in all template bodies, even if the template is a simple function template.

Attached patch conditionalizes the compat fallback on whether the template function is part of a CXXRecordDecl as well, so that the dependent base class lookup workaround only happens when there's a chance of dependent base classes.

See discussion here:
http://permalink.gmane.org/gmane.comp.compilers.clang.devel/38225

[kimgr]

I've played with another approach, to just ask the DeclContext if its parent is a record, i.e.

if (DC->isDependentContext() && DC->isFunctionOrMethod() && DC->getParent()->isRecord()) {
   // ...
}

and it seems to work equally well for the test cases I've provided.

I'm not sure if checking for records is sufficient, though, I'm worried about constructs like:

struct Foo {
  template<class T>
  void bar() {
     UndefType::staticMethod();
  }
};

Does that even qualify as a dependent base lookup?

Thanks,
Kim

[rnk]

lgtm

This isn't consistent with MSVC, but it doesn't break ATL "hello world" at the very least. If we commit this, I'll find out what happens in Chrome overnight. Do you need me to land it?

[kimgr]

Thanks! Could you try this one instead? I think it's more representative of what we should do, i.e. only fall back if we're in a CXXRecordDecl and said record has dependent bases.

[kimgr]

... and by 'should', I mean what I intended, to avoid this fallback when the chance of dependent bases is zero.

I don't have commit access, so I'd love it if you could land this, and Chromium is a great baseline, so thanks for trying that!

[rnk]

Closed by commit rL215683 (authored by @rnk).

[rnk]

I landed this in r215683 with a warning and some tests for it. The diagnostic leaves a bit to be desired, though.

Thanks for the patch!

lib/Sema/SemaCXXScopeSpec.cpp

   if (getLangOpts().MSVCCompat) {
     DeclContext *DC = LookupCtx ? LookupCtx : CurContext;
     if (DC->isDependentContext() && DC->isFunctionOrMethod()) {
-      SS.Extend(Context, &Identifier, IdentifierLoc, CCLoc);
-      return false;
+      CXXRecordDecl *ContainingClass = dyn_cast<CXXRecordDecl>(DC->getParent());
+      if (ContainingClass && ContainingClass->hasAnyDependentBases()) {
+        SS.Extend(Context, &Identifier, IdentifierLoc, CCLoc);
+        return false;
+      }
     }
   }
 

test/SemaTemplate/ms-lookup-template-base-classes.cpp

   int x = f<NameFromBase>();
 };
 }
+
+namespace function_template_undef_impl {
+template<class T>
+void f() {
+  Undef::staticMethod(); // expected-error {{use of undeclared identifier 'Undef'}}
+  UndefVar.method(); // expected-error {{use of undeclared identifier 'UndefVar'}}
+}
+}