Allow lookup into dependent bases in static methods under -fms-compatibility

[rnk]

Summary

We currently allow unqualified lookup for instance methods but not
static methods because we can't recover with a semantic 'this->'
insertion.

ATL headers have static methods that do unqualified lookup into
dependent base classes. The pattern looks like:

template <typename T> struct Foo : T {
  static int *getBarFromT() { return Bar; }
};

Now we recover as if the user had written:

template <typename T> struct Foo : T {
  static int *getBarFromT() { return Foo::Bar; }
};

... which will eventually look up Bar in T at instantiation time.

Now we also emit a diagnostic in both cases.

[majnemer]

lib/Sema/SemaExpr.cpp

1940 ↗

(On Diff #10255)

auto?

[rsmith]

lib/Sema/SemaExpr.cpp

1949 ↗

(On Diff #10255)

This diagnostic seems inappropriate: it says the member was "found by lookup into dependent bases", but we don't know that. All we know is it wasn't found by normal lookup. Maybe rephrase it to only suggest that the name was not found by normal lookup?

1952 ↗

(On Diff #10255)

This isn't exactly correct. You should also check S.CXXThisTypeOverride; if that's set, then member accesses with this-> are OK even though we're not in a member function. (This happens when parsing a trailing return type or default initializer.)

[rnk]

lib/Sema/SemaExpr.cpp

1940 ↗

(On Diff #10255)

done

1949 ↗

(On Diff #10255)

What do you think of this diagnostic text?

warning: use of undeclared identifier 'StartWindowProc'; lookup into dependent bases of class template 'Derived' is a Microsoft extension

I'm trying to make it phrase well for -pedantic-errors.

1952 ↗

(On Diff #10255)

I wasn't trying to be this general, but I went ahead and implemented it with some extra tests.

[rnk]

review

[rsmith]

Looks fine with some minor tweaks.

include/clang/Basic/DiagnosticSemaKinds.td

3661 ↗

(On Diff #10308)

Should be ext_...

3665 ↗

(On Diff #10308)

Hmm, this one too =)

lib/Sema/SemaExpr.cpp

1952 ↗

(On Diff #10308)

This comment is out of date.

1954 ↗

(On Diff #10308)

Maybe fold this into the S.Diag call below?

1960–1961 ↗

(On Diff #10308)

I don't think this comment makes much sense in its new context; maybe just remove it?

[rnk]

Thanks!

include/clang/Basic/DiagnosticSemaKinds.td

3661 ↗

(On Diff #10308)

done

lib/Sema/SemaExpr.cpp

1954 ↗

(On Diff #10308)

done, previously the formatting was horrible

1960–1961 ↗

(On Diff #10308)

done.

[rnk]

Closed by commit rL210611 (authored by @rnk).

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

 def err_sizeof_pack_no_pack_name : Error<
   "%0 does not refer to the name of a parameter pack">;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;
 def err_unexpected_namespace : Error<
   "unexpected namespace name %0: expected expression">;
 def err_undeclared_var_use : Error<"use of undeclared identifier %0">;
-def warn_found_via_dependent_bases_lookup : ExtWarn<"use of identifier %0 "
-   "found via unqualified lookup into dependent bases of class templates is a "
-   "Microsoft extension">, InGroup<Microsoft>;
+def ext_undeclared_unqual_id_with_dependent_base : ExtWarn<
+  "use of undeclared identifier %0; "
+  "unqualified lookup into dependent bases of class template %1 is a Microsoft extension">,
+  InGroup<Microsoft>;
+def ext_found_via_dependent_bases_lookup : ExtWarn<"use of identifier %0 "
 def note_dependent_var_use : Note<"must qualify identifier to find this "
     "declaration in dependent base class">;
 def err_not_found_by_two_phase_lookup : Error<"call to function %0 that is neither "
     "visible in the template definition nor found by argument-dependent lookup">;
 def note_not_found_by_two_phase_lookup : Note<"%0 should be declared prior to the "
     "call site%select{| or in %2| or in an associated namespace of one of its arguments}1">;
 def err_undeclared_use : Error<"use of undeclared %0">;
 def warn_deprecated : Warning<"%0 is deprecated">,

cfe/trunk/lib/Sema/SemaExpr.cpp

if (isa<CXXRecordDecl>(DC)) {
                           CurMethod->isInstance() &&
                           DC == CurMethod->getParent() && !isDefaultArgument;
                           
 
         // Give a code modification hint to insert 'this->'.
         // TODO: fixit for inserting 'Base<T>::' in the other cases.
         // Actually quite difficult!
         if (getLangOpts().MSVCCompat)
-          diagnostic = diag::warn_found_via_dependent_bases_lookup;
+          diagnostic = diag::ext_found_via_dependent_bases_lookup;
         if (isInstance) {
           Diag(R.getNameLoc(), diagnostic) << Name
             << FixItHint::CreateInsertion(R.getNameLoc(), "this->");
           UnresolvedLookupExpr *ULE = cast<UnresolvedLookupExpr>(
               CallsUndergoingInstantiation.back()->getCallee());
 
           CXXMethodDecl *DepMethod;
           if (CurMethod->isDependentContext())
if (!SS.isEmpty()) {
     return true;
   }
 
   // Give up, we can't recover.
   Diag(R.getNameLoc(), diagnostic) << Name;
   return true;
 }
 
+/// In Microsoft mode, if we are inside a template class whose parent class has
+/// dependent base classes, and we can't resolve an unqualified identifier, then
+/// assume the identifier is a member of a dependent base class.  We can only
+/// recover successfully in static methods, instance methods, and other contexts
+/// where 'this' is available.  This doesn't precisely match MSVC's
+/// instantiation model, but it's close enough.
+static Expr *
+recoverFromMSUnqualifiedLookup(Sema &S, ASTContext &Context,
+                               DeclarationNameInfo &NameInfo,
+                               SourceLocation TemplateKWLoc,
+                               const TemplateArgumentListInfo *TemplateArgs) {
+  // Only try to recover from lookup into dependent bases in static methods or
+  // contexts where 'this' is available.
+  QualType ThisType = S.getCurrentThisType();
+  const CXXRecordDecl *RD = nullptr;
+  if (!ThisType.isNull())
+    RD = ThisType->getPointeeType()->getAsCXXRecordDecl();
+  else if (auto *MD = dyn_cast<CXXMethodDecl>(S.CurContext))
+    RD = MD->getParent();
+  if (!RD || !RD->hasAnyDependentBases())
+    return nullptr;
+
+  // Diagnose this as unqualified lookup into a dependent base class.  If 'this'
+  // is available, suggest inserting 'this->' as a fixit.
+  SourceLocation Loc = NameInfo.getLoc();
+  DiagnosticBuilder DB =
+      S.Diag(Loc, diag::ext_undeclared_unqual_id_with_dependent_base)
+      << NameInfo.getName() << RD;
+
+  if (!ThisType.isNull()) {
+    DB << FixItHint::CreateInsertion(Loc, "this->");
+    return CXXDependentScopeMemberExpr::Create(
+        Context, /*This=*/nullptr, ThisType, /*IsArrow=*/true,
+        /*Op=*/SourceLocation(), NestedNameSpecifierLoc(), TemplateKWLoc,
+        /*FirstQualifierInScope=*/nullptr, NameInfo, TemplateArgs);
+  }
+
+  // Synthesize a fake NNS that points to the derived class.  This will
+  // perform name lookup during template instantiation.
+  CXXScopeSpec SS;
+  auto *NNS =
+      NestedNameSpecifier::Create(Context, nullptr, true, RD->getTypeForDecl());
+  SS.MakeTrivial(Context, NNS, SourceRange(Loc, Loc));
+  return DependentScopeDeclRefExpr::Create(
+      Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
+      TemplateArgs);
+}
+
 ExprResult Sema::ActOnIdExpression(Scope *S,
                                    CXXScopeSpec &SS,
                                    SourceLocation TemplateKWLoc,
                                    UnqualifiedId &Id,
                                    bool HasTrailingLParen,
                                    bool IsAddressOfOperand,
                                    CorrectionCandidateCallback *CCC,
                                    bool IsInlineAsmIdentifier) {
if (R.empty() && HasTrailingLParen && II && !getLangOpts().CPlusPlus) {
     if (D) R.addDecl(D);
   }
 
   // Determine whether this name might be a candidate for
   // argument-dependent lookup.
   bool ADL = UseArgumentDependentLookup(SS, R, HasTrailingLParen);
 
   if (R.empty() && !ADL) {
-    // In Microsoft mode, if we are inside a template class member function
-    // whose parent class has dependent base classes, and we can't resolve
-    // an unqualified identifier, then assume the identifier is a member of a
-    // dependent base class.  The goal is to postpone name lookup to
-    // instantiation time to be able to search into the type dependent base
-    // classes.
-    // FIXME: If we want 100% compatibility with MSVC, we will have delay all
-    // unqualified name lookup.  Any name lookup during template parsing means
-    // clang might find something that MSVC doesn't.  For now, we only handle
-    // the common case of members of a dependent base class.
     if (SS.isEmpty() && getLangOpts().MSVCCompat) {
-      CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(CurContext);
-      if (MD && MD->isInstance() && MD->getParent()->hasAnyDependentBases()) {
-        QualType ThisType = MD->getThisType(Context);
-        // Since the 'this' expression is synthesized, we don't need to
-        // perform the double-lookup check.
-        NamedDecl *FirstQualifierInScope = nullptr;
-        return CXXDependentScopeMemberExpr::Create(
-            Context, /*This=*/nullptr, ThisType, /*IsArrow=*/true,
-            /*Op=*/SourceLocation(), SS.getWithLocInContext(Context),
-            TemplateKWLoc, FirstQualifierInScope, NameInfo, TemplateArgs);
-      }
+      if (Expr *E = recoverFromMSUnqualifiedLookup(*this, Context, NameInfo,
+                                                   TemplateKWLoc, TemplateArgs))
+        return E;
     }
 
     // Don't diagnose an empty lookup for inline assmebly.
     if (IsInlineAsmIdentifier)
       return ExprError();
 
     // If this name wasn't predeclared and if this is not a function
     // call, diagnose the problem.

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

-// RUN: %clang_cc1 -fms-compatibility -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -verify %s
 
 
 template <class T>
 class A {
 public:
    void f(T a) { }// expected-note {{must qualify identifier to find this declaration in dependent base class}}
    void g();// expected-note {{must qualify identifier to find this declaration in dependent base class}}
 };
public:
   int var;
 };
 
 
 template<class T>
 class B : public A<T> {
 public:
   void f() {
-    var = 3;
+    var = 3; // expected-warning {{use of undeclared identifier 'var'; unqualified lookup into dependent bases of class template 'B' is a Microsoft extension}}
   }
 };
 
 template class B<int>;
 
 }
 
 

 public:
   int m_hWnd;
 };
 
 template <class T>
 class A : public T {
 public:
   void f(int hWnd) {
-    m_hWnd = 1;
+    m_hWnd = 1; // expected-warning {{use of undeclared identifier 'm_hWnd'; unqualified lookup into dependent bases of class template 'A' is a Microsoft extension}}
   }
 };
 
 template class A<C>;
 
 }
 
 namespace PR12701 {

 
 namespace PR16014 {
 
 struct A {
   int a;
   static int sa;
 };
 template <typename T> struct B : T {
-  int     foo() { return a; }
-  int    *bar() { return &a; }
+  int     foo() { return a; }           // expected-warning {{lookup into dependent bases}}
+  int    *bar() { return &a; }          // expected-warning {{lookup into dependent bases}}
   int     baz() { return T::a; }
   int T::*qux() { return &T::a; }
   static int T::*stuff() { return &T::a; }
   static int stuff1() { return T::sa; }
   static int *stuff2() { return &T::sa; }
+  static int stuff3() { return sa; }    // expected-warning {{lookup into dependent bases}}
+  static int *stuff4() { return &sa; }  // expected-warning {{lookup into dependent bases}}
 };
 
 template <typename T> struct C : T {
-  int     foo() { return b; }      // expected-error {{no member named 'b' in 'PR16014::C<PR16014::A>'}}
-  int    *bar() { return &b; }     // expected-error {{no member named 'b' in 'PR16014::C<PR16014::A>'}}
+  int     foo() { return b; }      // expected-error {{no member named 'b' in 'PR16014::C<PR16014::A>'}} expected-warning {{lookup into dependent bases}}
+  int    *bar() { return &b; }     // expected-error {{no member named 'b' in 'PR16014::C<PR16014::A>'}} expected-warning {{lookup into dependent bases}}
   int     baz() { return T::b; }   // expected-error {{no member named 'b' in 'PR16014::A'}}
   int T::*qux() { return &T::b; }  // expected-error {{no member named 'b' in 'PR16014::A'}}
   int T::*fuz() { return &U::a; }  // expected-error {{use of undeclared identifier 'U'}}
 };
 
 template struct B<A>;
 template struct C<A>;  // expected-note-re 1+ {{in instantiation of member function 'PR16014::C<PR16014::A>::{{.*}}' requested here}}
 

 struct B { void qux(); };
 struct C : B { };
 template struct A<C>; // No error!  B is a base of A<C>, and qux is available.
 
 struct D { };
 template struct A<D>; // expected-note {{in instantiation of member function 'PR19233::A<PR19233::D>::baz' requested here}}
 
 }
+
+namespace nonmethod_missing_this {
+template <typename T> struct Base { int y = 42; };
+template <typename T> struct Derived : Base<T> {
+  int x = y; // expected-warning {{lookup into dependent bases}}
+  auto foo(int j) -> decltype(y * j) { // expected-warning {{lookup into dependent bases}}
+    return y * j; // expected-warning {{lookup into dependent bases}}
+  }
+  int bar() {
+    return [&] { return y; }(); // expected-warning {{lookup into dependent bases}}
+  }
+};
+template struct Derived<int>;
+}