Fix a point of semantics with using declaration hiding: method templates

introduced by using decls are hidden even if their template parameter lists
or return types differ from the "overriding" declaration.

Propagate using shadow declarations around more effectively when looking up
template-ids.  Reperform lookup for template-ids in member expressions so that
access control is properly set up.

Fix some number of latent bugs involving template-ids with totally invalid
base types.  You can only actually get these with a scope specifier, since
otherwise the template-id won't parse as a template-id.

Fixes PR7384.

llvm-svn: 106093

clang/lib/Sema/Lookup.h

@@ -424,6 +424,11 @@ public:
     Diagnose = false;
   }
 
+  /// Determines whether this lookup is suppressing diagnostics.
+  bool isSuppressingDiagnostics() const {
+    return Diagnose;
+  }
+
   /// Sets a 'context' source range.
   void setContextRange(SourceRange SR) {
     NameContextRange = SR;

clang/lib/Sema/Sema.h

@@ -1103,10 +1103,12 @@ public:
     /// non-function.
     Ovl_NonFunction
   };
-  OverloadKind CheckOverload(FunctionDecl *New,
+  OverloadKind CheckOverload(Scope *S,
+                             FunctionDecl *New,
                              const LookupResult &OldDecls,
-                             NamedDecl *&OldDecl);
+                             NamedDecl *&OldDecl,
-  bool IsOverload(FunctionDecl *New, FunctionDecl *Old);
+                             bool IsForUsingDecl);
+  bool IsOverload(FunctionDecl *New, FunctionDecl *Old, bool IsForUsingDecl);
 
   bool TryImplicitConversion(InitializationSequence &Sequence,
                              const InitializedEntity &Entity,
@@ -1952,7 +1954,8 @@ public:
   OwningExprResult LookupMemberExpr(LookupResult &R, Expr *&Base,
                                     bool &IsArrow, SourceLocation OpLoc,
                                     CXXScopeSpec &SS,
-                                    DeclPtrTy ObjCImpDecl);
+                                    DeclPtrTy ObjCImpDecl,
+                                    bool HasTemplateArgs);
 
   bool CheckQualifiedMemberReference(Expr *BaseExpr, QualType BaseType,
                                      const CXXScopeSpec &SS,

clang/lib/Sema/SemaDecl.cpp

@@ -2863,7 +2863,7 @@ static bool FindOverriddenMethod(const CXXBaseSpecifier *Specifier,
   
   // FIXME: Do we care about other names here too?
   if (Name.getNameKind() == DeclarationName::CXXDestructorName) {
-    // We really want to find the base class constructor here.
+    // We really want to find the base class destructor here.
     QualType T = Data->S->Context.getTypeDeclType(BaseRecord);
     CanQualType CT = Data->S->Context.getCanonicalType(T);
     
@@ -2873,8 +2873,9 @@ static bool FindOverriddenMethod(const CXXBaseSpecifier *Specifier,
   for (Path.Decls = BaseRecord->lookup(Name);
        Path.Decls.first != Path.Decls.second;
        ++Path.Decls.first) {
-    if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(*Path.Decls.first)) {
+    NamedDecl *D = (*Path.Decls.first)->getUnderlyingDecl();
-      if (MD->isVirtual() && !Data->S->IsOverload(Data->Method, MD))
+    if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D)) {
+      if (MD->isVirtual() && !Data->S->IsOverload(Data->Method, MD, false))
         return true;
     }
   }
@@ -3588,13 +3589,10 @@ void Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
         }
       }
 
-      switch (CheckOverload(NewFD, Previous, OldDecl)) {
+      switch (CheckOverload(S, NewFD, Previous, OldDecl,
+                            /*NewIsUsingDecl*/ false)) {
       case Ovl_Match:
         Redeclaration = true;
-        if (isa<UsingShadowDecl>(OldDecl) && CurContext->isRecord()) {
-          HideUsingShadowDecl(S, cast<UsingShadowDecl>(OldDecl));
-          Redeclaration = false;
-        }
         break;
 
       case Ovl_NonFunction:

clang/lib/Sema/SemaDeclCXX.cpp

@@ -3649,7 +3649,7 @@ bool Sema::CheckUsingShadowDecl(UsingDecl *Using, NamedDecl *Orig,
       FD = cast<FunctionDecl>(Target);
 
     NamedDecl *OldDecl = 0;
-    switch (CheckOverload(FD, Previous, OldDecl)) {
+    switch (CheckOverload(0, FD, Previous, OldDecl, /*IsForUsingDecl*/ true)) {
     case Ovl_Overload:
       return false;
 
@@ -3659,11 +3659,6 @@ bool Sema::CheckUsingShadowDecl(UsingDecl *Using, NamedDecl *Orig,
       
     // We found a decl with the exact signature.
     case Ovl_Match:
-      if (isa<UsingShadowDecl>(OldDecl)) {
-        // Silently ignore the possible conflict.
-        return false;
-      }
-
       // If we're in a record, we want to hide the target, so we
       // return true (without a diagnostic) to tell the caller not to
       // build a shadow decl.

clang/lib/Sema/SemaExpr.cpp

@@ -677,26 +677,6 @@ static void DecomposeUnqualifiedId(Sema &SemaRef,
   }
 }
 
-/// Decompose the given template name into a list of lookup results.
-///
-/// The unqualified ID must name a non-dependent template, which can
-/// be more easily tested by checking whether DecomposeUnqualifiedId
-/// found template arguments.
-static void DecomposeTemplateName(LookupResult &R, const UnqualifiedId &Id) {
-  assert(Id.getKind() == UnqualifiedId::IK_TemplateId);
-  TemplateName TName =
-    Sema::TemplateTy::make(Id.TemplateId->Template).getAsVal<TemplateName>();
-
-  if (TemplateDecl *TD = TName.getAsTemplateDecl())
-    R.addDecl(TD);
-  else if (OverloadedTemplateStorage *OT = TName.getAsOverloadedTemplate())
-    for (OverloadedTemplateStorage::iterator I = OT->begin(), E = OT->end();
-           I != E; ++I)
-      R.addDecl(*I);
-
-  R.resolveKind();
-}
-
 /// Determines whether the given record is "fully-formed" at the given
 /// location, i.e. whether a qualified lookup into it is assured of
 /// getting consistent results already.
@@ -2580,13 +2560,23 @@ bool Sema::CheckQualifiedMemberReference(Expr *BaseExpr,
 static bool
 LookupMemberExprInRecord(Sema &SemaRef, LookupResult &R,
                          SourceRange BaseRange, const RecordType *RTy,
-                         SourceLocation OpLoc, CXXScopeSpec &SS) {
+                         SourceLocation OpLoc, CXXScopeSpec &SS,
+                         bool HasTemplateArgs) {
   RecordDecl *RDecl = RTy->getDecl();
   if (SemaRef.RequireCompleteType(OpLoc, QualType(RTy, 0),
                               SemaRef.PDiag(diag::err_typecheck_incomplete_tag)
                                     << BaseRange))
     return true;
 
+  if (HasTemplateArgs) {
+    // LookupTemplateName doesn't expect these both to exist simultaneously.
+    QualType ObjectType = SS.isSet() ? QualType() : QualType(RTy, 0);
+
+    bool MOUS;
+    SemaRef.LookupTemplateName(R, 0, SS, ObjectType, false, MOUS);
+    return false;
+  }
+
   DeclContext *DC = RDecl;
   if (SS.isSet()) {
     // If the member name was a qualified-id, look into the
@@ -2660,14 +2650,14 @@ Sema::BuildMemberReferenceExpr(ExprArg BaseArg, QualType BaseType,
     if (IsArrow) RecordTy = RecordTy->getAs<PointerType>()->getPointeeType();
     if (LookupMemberExprInRecord(*this, R, SourceRange(),
                                  RecordTy->getAs<RecordType>(),
-                                 OpLoc, SS))
+                                 OpLoc, SS, TemplateArgs != 0))
       return ExprError();
 
   // Explicit member accesses.
   } else {
     OwningExprResult Result =
       LookupMemberExpr(R, Base, IsArrow, OpLoc,
-                       SS, /*ObjCImpDecl*/ DeclPtrTy());
+                       SS, /*ObjCImpDecl*/ DeclPtrTy(), TemplateArgs != 0);
 
     if (Result.isInvalid()) {
       Owned(Base);
@@ -2880,7 +2870,7 @@ Sema::OwningExprResult
 Sema::LookupMemberExpr(LookupResult &R, Expr *&BaseExpr,
                        bool &IsArrow, SourceLocation OpLoc,
                        CXXScopeSpec &SS,
-                       DeclPtrTy ObjCImpDecl) {
+                       DeclPtrTy ObjCImpDecl, bool HasTemplateArgs) {
   assert(BaseExpr && "no base expression");
 
   // Perform default conversions.
@@ -3057,7 +3047,7 @@ Sema::LookupMemberExpr(LookupResult &R, Expr *&BaseExpr,
   // Handle field access to simple records.
   if (const RecordType *RTy = BaseType->getAs<RecordType>()) {
     if (LookupMemberExprInRecord(*this, R, BaseExpr->getSourceRange(),
-                                 RTy, OpLoc, SS))
+                                 RTy, OpLoc, SS, HasTemplateArgs))
       return ExprError();
     return Owned((Expr*) 0);
   }
@@ -3259,44 +3249,24 @@ Sema::OwningExprResult Sema::ActOnMemberAccessExpr(Scope *S, ExprArg BaseArg,
                                       TemplateArgs);
   } else {
     LookupResult R(*this, Name, NameLoc, LookupMemberName);
-    if (TemplateArgs) {
+    Result = LookupMemberExpr(R, Base, IsArrow, OpLoc,
-      // Re-use the lookup done for the template name.
+                              SS, ObjCImpDecl, TemplateArgs != 0);
-      DecomposeTemplateName(R, Id);
 
-      // Re-derive the naming class.
+    if (Result.isInvalid()) {
-      if (SS.isSet()) {
+      Owned(Base);
-        NestedNameSpecifier *Qualifier
+      return ExprError();
-        = static_cast<NestedNameSpecifier *>(SS.getScopeRep());
+    }
-        if (const Type *Ty = Qualifier->getAsType())
-          if (CXXRecordDecl *NamingClass = Ty->getAsCXXRecordDecl())
-            R.setNamingClass(NamingClass);
-      } else {
-        QualType BaseType = Base->getType();
-        if (const PointerType *Ptr = BaseType->getAs<PointerType>())
-          BaseType = Ptr->getPointeeType();
-        if (CXXRecordDecl *NamingClass = BaseType->getAsCXXRecordDecl())
-          R.setNamingClass(NamingClass);
-      }
-    } else {
-      Result = LookupMemberExpr(R, Base, IsArrow, OpLoc,
-                                SS, ObjCImpDecl);
 
-      if (Result.isInvalid()) {
+    if (Result.get()) {
-        Owned(Base);
+      // The only way a reference to a destructor can be used is to
-        return ExprError();
+      // immediately call it, which falls into this case.  If the
-      }
+      // next token is not a '(', produce a diagnostic and build the
+      // call now.
+      if (!HasTrailingLParen &&
+          Id.getKind() == UnqualifiedId::IK_DestructorName)
+        return DiagnoseDtorReference(NameLoc, move(Result));
 
-      if (Result.get()) {
+      return move(Result);
-        // The only way a reference to a destructor can be used is to
-        // immediately call it, which falls into this case.  If the
-        // next token is not a '(', produce a diagnostic and build the
-        // call now.
-        if (!HasTrailingLParen &&
-            Id.getKind() == UnqualifiedId::IK_DestructorName)
-          return DiagnoseDtorReference(NameLoc, move(Result));
-
-        return move(Result);
-      }
     }
 
     Result = BuildMemberReferenceExpr(ExprArg(*this, Base), Base->getType(),

clang/lib/Sema/SemaOverload.cpp

@@ -500,19 +500,54 @@ void OverloadCandidateSet::clear() {
 // identical (return types of functions are not part of the
 // signature), IsOverload returns false and MatchedDecl will be set to
 // point to the FunctionDecl for #2.
+//
+// 'NewIsUsingShadowDecl' indicates that 'New' is being introduced
+// into a class by a using declaration.  The rules for whether to hide
+// shadow declarations ignore some properties which otherwise figure
+// into a function template's signature.
 Sema::OverloadKind
-Sema::CheckOverload(FunctionDecl *New, const LookupResult &Old,
+Sema::CheckOverload(Scope *S, FunctionDecl *New, const LookupResult &Old,
-                    NamedDecl *&Match) {
+                    NamedDecl *&Match, bool NewIsUsingDecl) {
   for (LookupResult::iterator I = Old.begin(), E = Old.end();
          I != E; ++I) {
-    NamedDecl *OldD = (*I)->getUnderlyingDecl();
+    NamedDecl *OldD = *I;
+
+    bool OldIsUsingDecl = false;
+    if (isa<UsingShadowDecl>(OldD)) {
+      OldIsUsingDecl = true;
+
+      // We can always introduce two using declarations into the same
+      // context, even if they have identical signatures.
+      if (NewIsUsingDecl) continue;
+
+      OldD = cast<UsingShadowDecl>(OldD)->getTargetDecl();
+    }
+
+    // If either declaration was introduced by a using declaration,
+    // we'll need to use slightly different rules for matching.
+    // Essentially, these rules are the normal rules, except that
+    // function templates hide function templates with different
+    // return types or template parameter lists.
+    bool UseMemberUsingDeclRules =
+      (OldIsUsingDecl || NewIsUsingDecl) && CurContext->isRecord();
+
     if (FunctionTemplateDecl *OldT = dyn_cast<FunctionTemplateDecl>(OldD)) {
-      if (!IsOverload(New, OldT->getTemplatedDecl())) {
+      if (!IsOverload(New, OldT->getTemplatedDecl(), UseMemberUsingDeclRules)) {
+        if (UseMemberUsingDeclRules && OldIsUsingDecl) {
+          HideUsingShadowDecl(S, cast<UsingShadowDecl>(*I));
+          continue;
+        }
+
         Match = *I;
         return Ovl_Match;
       }
     } else if (FunctionDecl *OldF = dyn_cast<FunctionDecl>(OldD)) {
-      if (!IsOverload(New, OldF)) {
+      if (!IsOverload(New, OldF, UseMemberUsingDeclRules)) {
+        if (UseMemberUsingDeclRules && OldIsUsingDecl) {
+          HideUsingShadowDecl(S, cast<UsingShadowDecl>(*I));
+          continue;
+        }
+
         Match = *I;
         return Ovl_Match;
       }
@@ -536,7 +571,8 @@ Sema::CheckOverload(FunctionDecl *New, const LookupResult &Old,
   return Ovl_Overload;
 }
 
-bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old) {
+bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old,
+                      bool UseUsingDeclRules) {
   FunctionTemplateDecl *OldTemplate = Old->getDescribedFunctionTemplate();
   FunctionTemplateDecl *NewTemplate = New->getDescribedFunctionTemplate();
 
@@ -581,7 +617,10 @@ bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old) {
   //
   // We check the return type and template parameter lists for function
   // templates first; the remaining checks follow.
-  if (NewTemplate &&
+  //
+  // However, we don't consider either of these when deciding whether
+  // a member introduced by a shadow declaration is hidden.
+  if (!UseUsingDeclRules && NewTemplate &&
       (!TemplateParameterListsAreEqual(NewTemplate->getTemplateParameters(),
                                        OldTemplate->getTemplateParameters(),
                                        false, TPL_TemplateMatch) ||

clang/lib/Sema/SemaTemplate.cpp

@@ -27,12 +27,12 @@ using namespace clang;
 /// \brief Determine whether the declaration found is acceptable as the name
 /// of a template and, if so, return that template declaration. Otherwise,
 /// returns NULL.
-static NamedDecl *isAcceptableTemplateName(ASTContext &Context, NamedDecl *D) {
+static NamedDecl *isAcceptableTemplateName(ASTContext &Context,
-  if (!D)
+                                           NamedDecl *Orig) {
-    return 0;
+  NamedDecl *D = Orig->getUnderlyingDecl();
 
   if (isa<TemplateDecl>(D))
-    return D;
+    return Orig;
 
   if (CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(D)) {
     // C++ [temp.local]p1:
@@ -68,7 +68,7 @@ static void FilterAcceptableTemplateNames(ASTContext &C, LookupResult &R) {
   LookupResult::Filter filter = R.makeFilter();
   while (filter.hasNext()) {
     NamedDecl *Orig = filter.next();
-    NamedDecl *Repl = isAcceptableTemplateName(C, Orig->getUnderlyingDecl());
+    NamedDecl *Repl = isAcceptableTemplateName(C, Orig);
     if (!Repl)
       filter.erase();
     else if (Repl != Orig) {
@@ -260,7 +260,7 @@ void Sema::LookupTemplateName(LookupResult &Found,
     if (DeclarationName Corrected = CorrectTypo(Found, S, &SS, LookupCtx, 
                                                  false, CTC_CXXCasts)) {
       FilterAcceptableTemplateNames(Context, Found);
-      if (!Found.empty() && isa<TemplateDecl>(*Found.begin())) {
+      if (!Found.empty()) {
         if (LookupCtx)
           Diag(Found.getNameLoc(), diag::err_no_member_template_suggest)
             << Name << LookupCtx << Found.getLookupName() << SS.getRange()
@@ -274,8 +274,7 @@ void Sema::LookupTemplateName(LookupResult &Found,
         if (TemplateDecl *Template = Found.getAsSingle<TemplateDecl>())
           Diag(Template->getLocation(), diag::note_previous_decl)
             << Template->getDeclName();
-      } else
+      }
-        Found.clear();
     } else {
       Found.clear();
     }
@@ -303,7 +302,7 @@ void Sema::LookupTemplateName(LookupResult &Found,
       //   - if the name is found in the context of the entire
       //     postfix-expression and does not name a class template, the name
       //     found in the class of the object expression is used, otherwise
-    } else {
+    } else if (!Found.isSuppressingDiagnostics()) {
       //   - if the name found is a class template, it must refer to the same
       //     entity as the one found in the class of the object expression,
       //     otherwise the program is ill-formed.

clang/lib/Sema/TreeTransform.h

@@ -1826,7 +1826,8 @@ public:
                    Sema::LookupMemberName);
     OwningExprResult Result = getSema().LookupMemberExpr(R, Base, IsArrow,
                                                          /*FIME:*/IvarLoc,
-                                                         SS, DeclPtrTy());
+                                                         SS, DeclPtrTy(),
+                                                         false);
     if (Result.isInvalid())
       return getSema().ExprError();
     
@@ -1855,7 +1856,8 @@ public:
     bool IsArrow = false;
     OwningExprResult Result = getSema().LookupMemberExpr(R, Base, IsArrow,
                                                          /*FIME:*/PropertyLoc,
-                                                         SS, DeclPtrTy());
+                                                         SS, DeclPtrTy(),
+                                                         false);
     if (Result.isInvalid())
       return getSema().ExprError();
     
@@ -1903,7 +1905,8 @@ public:
                    Sema::LookupMemberName);
     OwningExprResult Result = getSema().LookupMemberExpr(R, Base, IsArrow,
                                                          /*FIME:*/IsaLoc,
-                                                         SS, DeclPtrTy());
+                                                         SS, DeclPtrTy(),
+                                                         false);
     if (Result.isInvalid())
       return getSema().ExprError();
     

clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p12.cpp

@@ -111,34 +111,53 @@ namespace test3 {
 
   struct Derived1 : Base {
     using Base::foo;
-    template <int n> Opaque<2> foo() { return Opaque<2>(); }
+    template <int n> Opaque<2> foo() { return Opaque<2>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'n'}}
   };
 
   struct Derived2 : Base {
-    template <int n> Opaque<2> foo() { return Opaque<2>(); }
+    template <int n> Opaque<2> foo() { return Opaque<2>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'n'}}
     using Base::foo;
   };
 
   struct Derived3 : Base {
     using Base::foo;
-    template <class T> Opaque<3> foo() { return Opaque<3>(); }
+    template <class T> Opaque<3> foo() { return Opaque<3>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'T'}}
   };
 
   struct Derived4 : Base {
-    template <class T> Opaque<3> foo() { return Opaque<3>(); }
+    template <class T> Opaque<3> foo() { return Opaque<3>(); } // expected-note {{invalid explicitly-specified argument for template parameter 'T'}}
     using Base::foo;
   };
 
   void test() {
     expect<0>(Base().foo<int>());
     expect<1>(Base().foo<0>());
-    expect<0>(Derived1().foo<int>());
+    expect<0>(Derived1().foo<int>()); // expected-error {{no matching member function for call to 'foo'}}
     expect<2>(Derived1().foo<0>());
-    expect<0>(Derived2().foo<int>());
+    expect<0>(Derived2().foo<int>()); // expected-error {{no matching member function for call to 'foo'}}
     expect<2>(Derived2().foo<0>());
     expect<3>(Derived3().foo<int>());
-    expect<1>(Derived3().foo<0>());
+    expect<1>(Derived3().foo<0>()); // expected-error {{no matching member function for call to 'foo'}}
     expect<3>(Derived4().foo<int>());
-    expect<1>(Derived4().foo<0>());
+    expect<1>(Derived4().foo<0>()); // expected-error {{no matching member function for call to 'foo'}}
+  }
+}
+
+// PR7384: access control for member templates.
+namespace test4 {
+  class Base {
+  protected:
+    template<typename T> void foo(T);
+    template<typename T> void bar(T); // expected-note {{declared protected here}}
+  };
+
+  struct Derived : Base {
+    using Base::foo;
+  };
+
+  void test() {
+    Derived d;
+    d.foo<int>(3);
+    d.bar<int>(3); // expected-error {{'bar' is a protected member}}
   }
 }

clang/test/SemaCXX/member-expr.cpp

@@ -72,3 +72,21 @@ namespace test4 {
     y.f(17);
   }
 }
+
+namespace test5 {
+  struct A {
+    template <class T> void foo();
+  };
+
+  void test0(int x) {
+    x.A::foo<int>(); // expected-error {{'int' is not a structure or union}}
+  }
+
+  void test1(A *x) {
+    x.A::foo<int>(); // expected-error {{'test5::A *' is a pointer}}
+  }
+
+  void test2(A &x) {
+    x->A::foo<int>(); // expected-error {{'test5::A' is not a pointer}}
+  }
+}