From: Richard Smith Date: Wed, 16 Jan 2019 22:01:39 +0000 (+0000) Subject: PR40329: [adl] Fix determination of associated classes when searching a X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a7d7e428cf6497d79dc3a557586cf9c7fe7b4839;p=clang PR40329: [adl] Fix determination of associated classes when searching a member enum and then its enclosing class. There are situations where ADL will collect a class but not the complete set of associated classes / namespaces of that class. When that happened, and we later tried to collect those associated classes / namespaces, we would previously short-circuit the lookup and not find them. Eg, for: struct A : B { enum E; }; if we first looked for associated classes/namespaces of A::E, we'd find only A. But if we then tried to also collect associated classes/namespaces of A (which should include the base class B), we would not add B because we had already visited A. This also fixes a minor issue where we would fail to collect associated classes from an overloaded class member access expression naming a static member function. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@351382 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index effccc2f3d..506e07670a 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -2417,10 +2417,18 @@ namespace { InstantiationLoc(InstantiationLoc) { } + bool addClassTransitive(CXXRecordDecl *RD) { + Classes.insert(RD); + return ClassesTransitive.insert(RD); + } + Sema &S; Sema::AssociatedNamespaceSet &Namespaces; Sema::AssociatedClassSet &Classes; SourceLocation InstantiationLoc; + + private: + Sema::AssociatedClassSet ClassesTransitive; }; } // end anonymous namespace @@ -2521,15 +2529,6 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result, // Add the associated namespace for this class. CollectEnclosingNamespace(Result.Namespaces, Ctx); - // Add the class itself. If we've already seen this class, we don't - // need to visit base classes. - // - // FIXME: That's not correct, we may have added this class only because it - // was the enclosing class of another class, and in that case we won't have - // added its base classes yet. - if (!Result.Classes.insert(Class)) - return; - // -- If T is a template-id, its associated namespaces and classes are // the namespace in which the template is defined; for member // templates, the member template's class; the namespaces and classes @@ -2552,6 +2551,11 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result, addAssociatedClassesAndNamespaces(Result, TemplateArgs[I]); } + // Add the class itself. If we've already transitively visited this class, + // we don't need to visit base classes. + if (!Result.addClassTransitive(Class)) + return; + // Only recurse into base classes for complete types. if (!Result.S.isCompleteType(Result.InstantiationLoc, Result.S.Context.getRecordType(Class))) @@ -2577,7 +2581,7 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result, if (!BaseType) continue; CXXRecordDecl *BaseDecl = cast(BaseType->getDecl()); - if (Result.Classes.insert(BaseDecl)) { + if (Result.addClassTransitive(BaseDecl)) { // Find the associated namespace for this base class. DeclContext *BaseCtx = BaseDecl->getDeclContext(); CollectEnclosingNamespace(Result.Namespaces, BaseCtx); @@ -2793,15 +2797,9 @@ void Sema::FindAssociatedClassesAndNamespaces( // in which the function or function template is defined and the // classes and namespaces associated with its (non-dependent) // parameter types and return type. - Arg = Arg->IgnoreParens(); - if (UnaryOperator *unaryOp = dyn_cast(Arg)) - if (unaryOp->getOpcode() == UO_AddrOf) - Arg = unaryOp->getSubExpr(); - - UnresolvedLookupExpr *ULE = dyn_cast(Arg); - if (!ULE) continue; + OverloadExpr *OE = OverloadExpr::find(Arg).Expression; - for (const auto *D : ULE->decls()) { + for (const NamedDecl *D : OE->decls()) { // Look through any using declarations to find the underlying function. const FunctionDecl *FDecl = D->getUnderlyingDecl()->getAsFunction(); diff --git a/test/CXX/drs/dr0xx.cpp b/test/CXX/drs/dr0xx.cpp index 048187814d..911aab1747 100644 --- a/test/CXX/drs/dr0xx.cpp +++ b/test/CXX/drs/dr0xx.cpp @@ -413,6 +413,36 @@ namespace dr33 { // dr33: yes void g(X::S); template Z g(Y::T); void h() { f(&g); } // expected-error {{ambiguous}} + + template void t(X::S); + template void u(X::S); // expected-error 0-1{{default template argument}} + void templ() { f(t); f(u); } + + // Even though v cannot select the first overload, ADL considers it + // and adds namespace Z to the set of associated namespaces, and then picks + // Z::f even though that function has nothing to do with any associated type. + namespace Z { struct Q; void f(void(*)()); } + template Z::Q v(); + template void v(); + void unrelated_templ() { f(v); } + + namespace dependent { + struct X {}; + template struct Y { + friend int operator+(X, void(*)(Y)) {} + }; + + template void f(Y); + int use = X() + f; // expected-error {{invalid operands}} + } + + namespace member { + struct Q {}; + struct Y { friend int operator+(Q, Y (*)()); }; + struct X { template static Y f(); }; + int m = Q() + X().f; // ok + int n = Q() + (&(X().f)); // ok + } } // dr34: na diff --git a/test/SemaCXX/adl.cpp b/test/SemaCXX/adl.cpp new file mode 100644 index 0000000000..392ddddcb4 --- /dev/null +++ b/test/SemaCXX/adl.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -std=c++11 -verify %s + +namespace PR40329 { + struct A { + A(int); + friend int operator->*(A, A); + }; + struct B : A { + B(); + enum E { e }; + }; + // Associated classes for B are {B, A} + // Associated classes for B::E are {B} (non-transitive in this case) + // + // If we search B::E first, we must not mark B "visited" and shortcircuit + // visiting it later, or we won't find the associated class A. + int k0 = B::e ->* B::e; // expected-error {{non-pointer-to-member type}} + int k1 = B::e ->* B(); + int k2 = B() ->* B::e; +}