Implement a framework for the delay of arbitrary diagnostics within

templates.  So delay access-control diagnostics when (for example) the target
of a friend declaration is a specific specialization of a template.

I was surprised to find that this was required for an access-controlled selfhost.

llvm-svn: 99383

clang/include/clang/AST/ASTContext.h

@@ -57,6 +57,7 @@ namespace clang {
   class ObjCIvarRefExpr;
   class ObjCPropertyDecl;
   class RecordDecl;
+  class StoredDeclsMap;
   class TagDecl;
   class TemplateTypeParmDecl;
   class TranslationUnitDecl;
@@ -1272,9 +1273,8 @@ private:
   // FIXME: This currently contains the set of StoredDeclMaps used
   // by DeclContext objects.  This probably should not be in ASTContext,
   // but we include it here so that ASTContext can quickly deallocate them.
-  std::vector<void*> SDMs; 
+  llvm::PointerIntPair<StoredDeclsMap*,1> LastSDM;
   friend class DeclContext;
-  void *CreateStoredDeclsMap();
   void ReleaseDeclContextMaps();
 };
   

clang/include/clang/AST/DeclBase.h

@@ -41,6 +41,8 @@ class LinkageSpecDecl;
 class BlockDecl;
 class DeclarationName;
 class CompoundStmt;
+class StoredDeclsMap;
+class DependentDiagnostic;
 }
 
 namespace llvm {
@@ -545,9 +547,9 @@ class DeclContext {
   mutable bool ExternalVisibleStorage : 1;
 
   /// \brief Pointer to the data structure used to lookup declarations
-  /// within this context, which is a DenseMap<DeclarationName,
+  /// within this context (or a DependentStoredDeclsMap if this is a
-  /// StoredDeclsList>.
+  /// dependent context).
-  mutable void* LookupPtr;
+  mutable StoredDeclsMap *LookupPtr;
 
   /// FirstDecl - The first declaration stored within this declaration
   /// context.
@@ -674,6 +676,9 @@ public:
   /// "primary" DeclContext structure, which will contain the
   /// information needed to perform name lookup into this context.
   DeclContext *getPrimaryContext();
+  const DeclContext *getPrimaryContext() const {
+    return const_cast<DeclContext*>(this)->getPrimaryContext();
+  }
 
   /// getLookupContext - Retrieve the innermost non-transparent
   /// context of this context, which corresponds to the innermost
@@ -976,10 +981,15 @@ public:
     return getUsingDirectives().second;
   }
 
+  // These are all defined in DependentDiagnostic.h.
+  class ddiag_iterator;
+  inline ddiag_iterator ddiag_begin() const;
+  inline ddiag_iterator ddiag_end() const;
+
   // Low-level accessors
 
   /// \brief Retrieve the internal representation of the lookup structure.
-  void* getLookupPtr() const { return LookupPtr; }
+  StoredDeclsMap* getLookupPtr() const { return LookupPtr; }
 
   /// \brief Whether this DeclContext has external storage containing
   /// additional declarations that are lexically in this context.
@@ -1013,6 +1023,9 @@ private:
   void LoadLexicalDeclsFromExternalStorage() const;
   void LoadVisibleDeclsFromExternalStorage() const;
 
+  friend class DependentDiagnostic;
+  StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const;
+
   void buildLookup(DeclContext *DCtx);
   void makeDeclVisibleInContextImpl(NamedDecl *D);
 };

clang/include/clang/AST/DeclContextInternals.h

@@ -24,6 +24,8 @@
 
 namespace clang {
 
+class DependentDiagnostic;
+
 /// StoredDeclsList - This is an array of decls optimized a common case of only
 /// containing one entry.
 struct StoredDeclsList {
@@ -258,8 +260,29 @@ public:
   }
 };
 
-typedef llvm::DenseMap<DeclarationName, StoredDeclsList> StoredDeclsMap;
+class StoredDeclsMap
+  : public llvm::DenseMap<DeclarationName, StoredDeclsList> {
 
+public:
+  static void DestroyAll(StoredDeclsMap *Map, bool Dependent);
+
+private:
+  friend class ASTContext; // walks the chain deleting these
+  friend class DeclContext;
+  llvm::PointerIntPair<StoredDeclsMap*, 1> Previous;
+};
+
+class DependentStoredDeclsMap : public StoredDeclsMap {
+public:
+  DependentStoredDeclsMap() : FirstDiagnostic(0) {}
+  ~DependentStoredDeclsMap();
+
+private:
+  friend class DependentDiagnostic;
+  friend class DeclContext; // iterates over diagnostics
+
+  DependentDiagnostic *FirstDiagnostic;
+};
 
 } // end namespace clang
 

clang/include/clang/AST/DependentDiagnostic.h

@@ -0,0 +1,180 @@
+//===-- DependentDiagnostic.h - Dependently-generated diagnostics -*- C++ -*-=//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines interfaces for diagnostics which may or may
+//  fire based on how a template is instantiated.
+//
+//  At the moment, the only consumer of this interface is access
+//  control.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_DEPENDENT_DIAGNOSTIC_H
+#define LLVM_CLANG_AST_DEPENDENT_DIAGNOSTIC_H
+
+#include "clang/Basic/PartialDiagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclContextInternals.h"
+
+namespace clang {
+
+class ASTContext;
+class CXXRecordDecl;
+class NamedDecl;
+
+/// A dependently-generated diagnostic.
+class DependentDiagnostic {
+public:
+  enum AccessNonce { Access = 0 };
+
+  static DependentDiagnostic *Create(ASTContext &Context,
+                                     DeclContext *Parent,
+                                     AccessNonce _,
+                                     SourceLocation Loc,
+                                     bool IsMemberAccess,
+                                     AccessSpecifier AS,
+                                     NamedDecl *TargetDecl,
+                                     CXXRecordDecl *NamingClass,
+                                     const PartialDiagnostic &PDiag) {
+    DependentDiagnostic *DD = Create(Context, Parent, PDiag);
+    DD->AccessData.Loc = Loc.getRawEncoding();
+    DD->AccessData.IsMember = IsMemberAccess;
+    DD->AccessData.Access = AS;
+    DD->AccessData.TargetDecl = TargetDecl;
+    DD->AccessData.NamingClass = NamingClass;
+    return DD;
+  }
+
+  unsigned getKind() const {
+    return Access;
+  }
+
+  bool isAccessToMember() const {
+    assert(getKind() == Access);
+    return AccessData.IsMember;
+  }
+
+  AccessSpecifier getAccess() const {
+    assert(getKind() == Access);
+    return AccessSpecifier(AccessData.Access);
+  }
+
+  SourceLocation getAccessLoc() const {
+    assert(getKind() == Access);
+    return SourceLocation::getFromRawEncoding(AccessData.Loc);
+  }
+
+  NamedDecl *getAccessTarget() const {
+    assert(getKind() == Access);
+    return AccessData.TargetDecl;
+  }
+
+  NamedDecl *getAccessNamingClass() const {
+    assert(getKind() == Access);
+    return AccessData.NamingClass;
+  }
+
+  const PartialDiagnostic &getDiagnostic() const {
+    return Diag;
+  }
+
+private:
+  DependentDiagnostic(const PartialDiagnostic &PDiag) : Diag(PDiag) {}
+  static DependentDiagnostic *Create(ASTContext &Context,
+                                     DeclContext *Parent,
+                                     const PartialDiagnostic &PDiag);
+
+  friend class DependentStoredDeclsMap;
+  friend class DeclContext::ddiag_iterator;
+  DependentDiagnostic *NextDiagnostic;
+
+  PartialDiagnostic Diag;
+
+  union {
+    struct {
+      unsigned Loc;
+      unsigned Access : 2;
+      unsigned IsMember : 1;
+      NamedDecl *TargetDecl;
+      CXXRecordDecl *NamingClass;
+    } AccessData;
+  };
+};
+
+/// 
+
+/// An iterator over the dependent diagnostics in a dependent context.
+class DeclContext::ddiag_iterator {
+public:
+  ddiag_iterator() : Ptr(0) {}
+  explicit ddiag_iterator(DependentDiagnostic *Ptr) : Ptr(Ptr) {}
+
+  typedef DependentDiagnostic *value_type;
+  typedef DependentDiagnostic *reference;
+  typedef DependentDiagnostic *pointer;
+  typedef int difference_type;
+  typedef std::forward_iterator_tag iterator_category;
+
+  reference operator*() const { return Ptr; }
+
+  ddiag_iterator &operator++() {
+    assert(Ptr && "attempt to increment past end of diag list");
+    Ptr = Ptr->NextDiagnostic;
+    return *this;
+  }
+
+  ddiag_iterator operator++(int) {
+    ddiag_iterator tmp = *this;
+    ++*this;
+    return tmp;
+  }
+
+  bool operator==(ddiag_iterator Other) const {
+    return Ptr == Other.Ptr;
+  }
+
+  bool operator!=(ddiag_iterator Other) const {
+    return Ptr != Other.Ptr;
+  }
+
+  ddiag_iterator &operator+=(difference_type N) {
+    assert(N >= 0 && "cannot rewind a DeclContext::ddiag_iterator");
+    while (N--)
+      ++*this;
+    return *this;
+  }
+
+  ddiag_iterator operator+(difference_type N) const {
+    ddiag_iterator tmp = *this;
+    tmp += N;
+    return tmp;
+  }
+
+private:
+  DependentDiagnostic *Ptr;
+};
+
+inline DeclContext::ddiag_iterator DeclContext::ddiag_begin() const {
+  assert(isDependentContext()
+         && "cannot iterate dependent diagnostics of non-dependent context");
+  const DependentStoredDeclsMap *Map
+    = static_cast<DependentStoredDeclsMap*>(getPrimaryContext()->LookupPtr);
+
+  if (!Map) return ddiag_iterator();
+  return ddiag_iterator(Map->FirstDiagnostic);
+}
+
+inline DeclContext::ddiag_iterator DeclContext::ddiag_end() const {
+  return ddiag_iterator();
+}
+
+}
+
+#endif

clang/lib/AST/ASTContext.cpp

@@ -45,7 +45,8 @@ ASTContext::ASTContext(const LangOptions& LOpts, SourceManager &SM,
   sigjmp_bufDecl(0), BlockDescriptorType(0), BlockDescriptorExtendedType(0),
   SourceMgr(SM), LangOpts(LOpts), FreeMemory(FreeMem), Target(t),
   Idents(idents), Selectors(sels),
-  BuiltinInfo(builtins), ExternalSource(0), PrintingPolicy(LOpts) {
+  BuiltinInfo(builtins), ExternalSource(0), PrintingPolicy(LOpts),
+  LastSDM(0, 0) {
   ObjCIdRedefinitionType = QualType();
   ObjCClassRedefinitionType = QualType();
   ObjCSelRedefinitionType = QualType();

clang/lib/AST/DeclBase.cpp

@@ -18,6 +18,7 @@
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DependentDiagnostic.h"
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Type.h"
@@ -481,7 +482,7 @@ DeclContext::~DeclContext() {
   // FIXME: Currently ~ASTContext will delete the StoredDeclsMaps because
   // ~DeclContext() is not guaranteed to be called when ASTContext uses
   // a BumpPtrAllocator.
-  // delete static_cast<StoredDeclsMap*>(LookupPtr);
+  // delete LookupPtr;
 }
 
 void DeclContext::DestroyDecls(ASTContext &C) {
@@ -516,10 +517,16 @@ bool DeclContext::isDependentContext() const {
     if (Record->getDescribedClassTemplate())
       return true;
 
-  if (const FunctionDecl *Function = dyn_cast<FunctionDecl>(this))
+  if (const FunctionDecl *Function = dyn_cast<FunctionDecl>(this)) {
     if (Function->getDescribedFunctionTemplate())
       return true;
 
+    // Friend function declarations are dependent if their *lexical*
+    // context is dependent.
+    if (cast<Decl>(this)->getFriendObjectKind())
+      return getLexicalParent()->isDependentContext();
+  }
+
   return getParent() && getParent()->isDependentContext();
 }
 
@@ -666,9 +673,7 @@ DeclContext::LoadVisibleDeclsFromExternalStorage() const {
   // Load the declaration IDs for all of the names visible in this
   // context.
   assert(!LookupPtr && "Have a lookup map before de-serialization?");
-  StoredDeclsMap *Map =
+  StoredDeclsMap *Map = CreateStoredDeclsMap(getParentASTContext());
-    (StoredDeclsMap*) getParentASTContext().CreateStoredDeclsMap();
-  LookupPtr = Map;
   for (unsigned I = 0, N = Decls.size(); I != N; ++I) {
     (*Map)[Decls[I].Name].setFromDeclIDs(Decls[I].Declarations);
   }
@@ -727,10 +732,9 @@ void DeclContext::removeDecl(Decl *D) {
   if (isa<NamedDecl>(D)) {
     NamedDecl *ND = cast<NamedDecl>(D);
 
-    void *OpaqueMap = getPrimaryContext()->LookupPtr;
+    StoredDeclsMap *Map = getPrimaryContext()->LookupPtr;
-    if (!OpaqueMap) return;
+    if (!Map) return;
 
-    StoredDeclsMap *Map = static_cast<StoredDeclsMap*>(OpaqueMap);
     StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName());
     assert(Pos != Map->end() && "no lookup entry for decl");
     Pos->second.remove(ND);
@@ -808,9 +812,8 @@ DeclContext::lookup(DeclarationName Name) {
       return lookup_result(0, 0);
   }
 
-  StoredDeclsMap *Map = static_cast<StoredDeclsMap*>(LookupPtr);
+  StoredDeclsMap::iterator Pos = LookupPtr->find(Name);
-  StoredDeclsMap::iterator Pos = Map->find(Name);
+  if (Pos == LookupPtr->end())
-  if (Pos == Map->end())
     return lookup_result(0, 0);
   return Pos->second.getLookupResult(getParentASTContext());
 }
@@ -878,12 +881,11 @@ void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D) {
   ASTContext *C = 0;
   if (!LookupPtr) {
     C = &getParentASTContext();
-    LookupPtr = (StoredDeclsMap*) C->CreateStoredDeclsMap();
+    CreateStoredDeclsMap(*C);
   }
 
   // Insert this declaration into the map.
-  StoredDeclsMap &Map = *static_cast<StoredDeclsMap*>(LookupPtr);
+  StoredDeclsList &DeclNameEntries = (*LookupPtr)[D->getDeclName()];
-  StoredDeclsList &DeclNameEntries = Map[D->getDeclName()];
   if (DeclNameEntries.isNull()) {
     DeclNameEntries.setOnlyValue(D);
     return;
@@ -952,13 +954,74 @@ void StoredDeclsList::materializeDecls(ASTContext &Context) {
 // Creation and Destruction of StoredDeclsMaps.                               //
 //===----------------------------------------------------------------------===//
 
-void *ASTContext::CreateStoredDeclsMap() {
+StoredDeclsMap *DeclContext::CreateStoredDeclsMap(ASTContext &C) const {
-  StoredDeclsMap *M = new StoredDeclsMap();
+  assert(!LookupPtr && "context already has a decls map");
-  SDMs.push_back(M);
+  assert(getPrimaryContext() == this &&
+         "creating decls map on non-primary context");
+
+  StoredDeclsMap *M;
+  bool Dependent = isDependentContext();
+  if (Dependent)
+    M = new DependentStoredDeclsMap();
+  else
+    M = new StoredDeclsMap();
+  M->Previous = C.LastSDM;
+  C.LastSDM = llvm::PointerIntPair<StoredDeclsMap*,1>(M, Dependent);
+  LookupPtr = M;
   return M;
 }
 
 void ASTContext::ReleaseDeclContextMaps() {
-  for (std::vector<void*>::iterator I = SDMs.begin(), E = SDMs.end(); I!=E; ++I)
+  // It's okay to delete DependentStoredDeclsMaps via a StoredDeclsMap
-    delete (StoredDeclsMap*) *I;
+  // pointer because the subclass doesn't add anything that needs to
+  // be deleted.
+  
+  StoredDeclsMap::DestroyAll(LastSDM.getPointer(), LastSDM.getInt());
+}
+
+void StoredDeclsMap::DestroyAll(StoredDeclsMap *Map, bool Dependent) {
+  while (Map) {
+    // Advance the iteration before we invalidate memory.
+    llvm::PointerIntPair<StoredDeclsMap*,1> Next = Map->Previous;
+
+    if (Dependent)
+      delete static_cast<DependentStoredDeclsMap*>(Map);
+    else
+      delete Map;
+
+    Map = Next.getPointer();
+    Dependent = Next.getInt();
+  }
+}
+
+DependentStoredDeclsMap::~DependentStoredDeclsMap() {
+  // Kill off the dependent diagnostics.  They don't need to be
+  // deleted, but they do need to be destructed.
+  DependentDiagnostic *CurD = FirstDiagnostic;
+  while (CurD) {
+    DependentDiagnostic *NextD = CurD->NextDiagnostic;
+    CurD->~DependentDiagnostic();
+    CurD = NextD;
+  }
+}
+
+DependentDiagnostic *DependentDiagnostic::Create(ASTContext &C,
+                                                 DeclContext *Parent,
+                                           const PartialDiagnostic &PDiag) {
+  assert(Parent->isDependentContext()
+         && "cannot iterate dependent diagnostics of non-dependent context");
+  Parent = Parent->getPrimaryContext();
+  if (!Parent->LookupPtr)
+    Parent->CreateStoredDeclsMap(C);
+
+  DependentStoredDeclsMap *Map
+    = static_cast<DependentStoredDeclsMap*>(Parent->LookupPtr);
+
+  DependentDiagnostic *DD = new (C) DependentDiagnostic(PDiag);
+
+  // TODO: Maybe we shouldn't reverse the order during insertion.
+  DD->NextDiagnostic = Map->FirstDiagnostic;
+  Map->FirstDiagnostic = DD;
+
+  return DD;
 }

clang/lib/Sema/Sema.h

@@ -2648,9 +2648,13 @@ public:
                                     unsigned DiagID,
                                     bool ForceCheck = false,
                                     bool ForceUnprivileged = false);
-                            
   void CheckLookupAccess(const LookupResult &R);
 
+  void HandleDependentAccessCheck(const DependentDiagnostic &DD,
+                         const MultiLevelTemplateArgumentList &TemplateArgs);
+  void PerformDependentDiagnostics(const DeclContext *Pattern,
+                        const MultiLevelTemplateArgumentList &TemplateArgs);
+
   void HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *Ctx);
 
   enum AbstractDiagSelID {

clang/lib/Sema/SemaAccess.cpp

@@ -17,6 +17,7 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclFriend.h"
+#include "clang/AST/DependentDiagnostic.h"
 #include "clang/AST/ExprCXX.h"
 
 using namespace clang;
@@ -52,9 +53,11 @@ bool Sema::SetMemberAccessSpecifier(NamedDecl *MemberDecl,
 
 namespace {
 struct EffectiveContext {
-  EffectiveContext() : Function(0) {}
+  EffectiveContext() : Function(0), Dependent(false) {}
 
   explicit EffectiveContext(DeclContext *DC) {
+    Dependent = DC->isDependentContext();
+
     if (isa<FunctionDecl>(DC)) {
       Function = cast<FunctionDecl>(DC)->getCanonicalDecl();
       DC = Function->getDeclContext();
@@ -75,14 +78,25 @@ struct EffectiveContext {
     }
   }
 
+  bool isDependent() const { return Dependent; }
+
   bool includesClass(const CXXRecordDecl *R) const {
     R = R->getCanonicalDecl();
     return std::find(Records.begin(), Records.end(), R)
              != Records.end();
   }
 
+  DeclContext *getPrimaryContext() const {
+    assert((Function || !Records.empty()) && "context has no primary context");
+    if (Function) return Function;
+    return Records[0];
+  }
+
+  typedef llvm::SmallVectorImpl<CXXRecordDecl*>::const_iterator record_iterator;
+
   llvm::SmallVector<CXXRecordDecl*, 4> Records;
   FunctionDecl *Function;
+  bool Dependent;
 };
 }
 
@@ -93,42 +107,134 @@ static CXXRecordDecl *FindDeclaringClass(NamedDecl *D) {
   return DeclaringClass;
 }
 
+static bool MightInstantiateTo(Sema &S, DeclContext *Context,
+                               DeclContext *Friend) {
+  if (Friend == Context)
+    return true;
+
+  assert(!Friend->isDependentContext() &&
+         "can't handle friends with dependent contexts here");
+
+  if (!Context->isDependentContext())
+    return false;
+
+  if (Friend->isFileContext())
+    return false;
+
+  // TODO: this is very conservative
+  return true;
+}
+
+// Asks whether the type in 'context' can ever instantiate to the type
+// in 'friend'.
+static bool MightInstantiateTo(Sema &S, CanQualType Context, CanQualType Friend) {
+  if (Friend == Context)
+    return true;
+
+  if (!Friend->isDependentType() && !Context->isDependentType())
+    return false;
+
+  // TODO: this is very conservative.
+  return true;
+}
+
+static bool MightInstantiateTo(Sema &S,
+                               FunctionDecl *Context,
+                               FunctionDecl *Friend) {
+  if (Context->getDeclName() != Friend->getDeclName())
+    return false;
+
+  if (!MightInstantiateTo(S,
+                          Context->getDeclContext(),
+                          Friend->getDeclContext()))
+    return false;
+
+  CanQual<FunctionProtoType> FriendTy
+    = S.Context.getCanonicalType(Friend->getType())
+         ->getAs<FunctionProtoType>();
+  CanQual<FunctionProtoType> ContextTy
+    = S.Context.getCanonicalType(Context->getType())
+         ->getAs<FunctionProtoType>();
+
+  // There isn't any way that I know of to add qualifiers
+  // during instantiation.
+  if (FriendTy.getQualifiers() != ContextTy.getQualifiers())
+    return false;
+
+  if (FriendTy->getNumArgs() != ContextTy->getNumArgs())
+    return false;
+
+  if (!MightInstantiateTo(S,
+                          ContextTy->getResultType(),
+                          FriendTy->getResultType()))
+    return false;
+
+  for (unsigned I = 0, E = FriendTy->getNumArgs(); I != E; ++I)
+    if (!MightInstantiateTo(S,
+                            ContextTy->getArgType(I),
+                            FriendTy->getArgType(I)))
+      return false;
+
+  return true;
+}
+
+static bool MightInstantiateTo(Sema &S,
+                               FunctionTemplateDecl *Context,
+                               FunctionTemplateDecl *Friend) {
+  return MightInstantiateTo(S,
+                            Context->getTemplatedDecl(),
+                            Friend->getTemplatedDecl());
+}
+
 static Sema::AccessResult MatchesFriend(Sema &S,
                                         const EffectiveContext &EC,
                                         const CXXRecordDecl *Friend) {
-  // FIXME: close matches becuse of dependency
   if (EC.includesClass(Friend))
     return Sema::AR_accessible;
 
+  if (EC.isDependent()) {
+    CanQualType FriendTy
+      = S.Context.getCanonicalType(S.Context.getTypeDeclType(Friend));
+
+    for (EffectiveContext::record_iterator
+           I = EC.Records.begin(), E = EC.Records.end(); I != E; ++I) {
+      CanQualType ContextTy
+        = S.Context.getCanonicalType(S.Context.getTypeDeclType(*I));
+      if (MightInstantiateTo(S, ContextTy, FriendTy))
+        return Sema::AR_dependent;
+    }
+  }
+
   return Sema::AR_inaccessible;
 }
 
 static Sema::AccessResult MatchesFriend(Sema &S,
                                         const EffectiveContext &EC,
-                                        FriendDecl *Friend) {
+                                        CanQualType Friend) {
-  if (Type *T = Friend->getFriendType()) {
+  if (const RecordType *RT = Friend->getAs<RecordType>())
-    CanQualType CT = T->getCanonicalTypeUnqualified();
-    if (const RecordType *RT = CT->getAs<RecordType>())
     return MatchesFriend(S, EC, cast<CXXRecordDecl>(RT->getDecl()));
 
-    // TODO: we can fail early for a lot of type classes.
+  // TODO: we can do better than this
-    if (T->isDependentType())
+  if (Friend->isDependentType())
     return Sema::AR_dependent;
 
   return Sema::AR_inaccessible;
-  }
+}
 
-  NamedDecl *D
+/// Determines whether the given friend class template matches
-    = cast<NamedDecl>(Friend->getFriendDecl()->getCanonicalDecl());
+/// anything in the effective context.
+static Sema::AccessResult MatchesFriend(Sema &S,
+                                        const EffectiveContext &EC,
+                                        ClassTemplateDecl *Friend) {
+  Sema::AccessResult OnFailure = Sema::AR_inaccessible;
 
-  // FIXME: declarations with dependent or templated scope.
-
-  // For class templates, we want to check whether any of the records
-  // are possible specializations of the template.
-  if (isa<ClassTemplateDecl>(D)) {
   for (llvm::SmallVectorImpl<CXXRecordDecl*>::const_iterator
          I = EC.Records.begin(), E = EC.Records.end(); I != E; ++I) {
     CXXRecordDecl *Record = *I;
+
+    // Check whether the friend is the template of a class in the
+    // context chain.  To do that, we need to figure out whether the
+    // current class has a template:
     ClassTemplateDecl *CTD;
 
     // A specialization of the template...
@@ -139,42 +245,96 @@ static Sema::AccessResult MatchesFriend(Sema &S,
     // ... or the template pattern itself.
     } else {
       CTD = Record->getDescribedClassTemplate();
+      if (!CTD) continue;
     }
 
-      if (CTD && D == CTD->getCanonicalDecl())
+    // It's a match.
+    if (Friend == CTD->getCanonicalDecl())
       return Sema::AR_accessible;
+
+    // If the template names don't match, it can't be a dependent
+    // match.  This isn't true in C++0x because of template aliases.
+    if (!S.LangOpts.CPlusPlus0x && CTD->getDeclName() != Friend->getDeclName())
+      continue;
+
+    // If the class's context can't instantiate to the friend's
+    // context, it can't be a dependent match.
+    if (!MightInstantiateTo(S, CTD->getDeclContext(),
+                            Friend->getDeclContext()))
+      continue;
+
+    // Otherwise, it's a dependent match.
+    OnFailure = Sema::AR_dependent;
   }
 
+  return OnFailure;
+}
+
+/// Determines whether the given friend function matches anything in
+/// the effective context.
+static Sema::AccessResult MatchesFriend(Sema &S,
+                                        const EffectiveContext &EC,
+                                        FunctionDecl *Friend) {
+  if (!EC.Function)
+    return Sema::AR_inaccessible;
+
+  if (Friend == EC.Function)
+    return Sema::AR_accessible;
+
+  if (EC.isDependent() && MightInstantiateTo(S, EC.Function, Friend))
+    return Sema::AR_dependent;
+
   return Sema::AR_inaccessible;
-  }
+}
 
-  // Same thing for function templates.
+/// Determines whether the given friend function template matches
-  if (isa<FunctionTemplateDecl>(D)) {
+/// anything in the effective context.
+static Sema::AccessResult MatchesFriend(Sema &S,
+                                        const EffectiveContext &EC,
+                                        FunctionTemplateDecl *Friend) {
   if (!EC.Function) return Sema::AR_inaccessible;
 
   FunctionTemplateDecl *FTD = EC.Function->getPrimaryTemplate();
   if (!FTD)
     FTD = EC.Function->getDescribedFunctionTemplate();
+  if (!FTD)
+    return Sema::AR_inaccessible;
 
-    if (FTD && D == FTD->getCanonicalDecl())
+  if (Friend == FTD->getCanonicalDecl())
     return Sema::AR_accessible;
 
-    return Sema::AR_inaccessible;
+  if (MightInstantiateTo(S, FTD, Friend))
-  }
+    return Sema::AR_dependent;
-
-  // Friend functions.  FIXME: close matches due to dependency.
-  // 
-  // The decl pointers in EC have been canonicalized, so pointer
-  // equality is sufficient.
-  if (D == EC.Function)
-    return Sema::AR_accessible;
-
-  if (isa<CXXRecordDecl>(D))
-    return MatchesFriend(S, EC, cast<CXXRecordDecl>(D));
 
   return Sema::AR_inaccessible;
 }
 
+/// Determines whether the given friend declaration matches anything
+/// in the effective context.
+static Sema::AccessResult MatchesFriend(Sema &S,
+                                        const EffectiveContext &EC,
+                                        FriendDecl *FriendD) {
+  if (Type *T = FriendD->getFriendType())
+    return MatchesFriend(S, EC, T->getCanonicalTypeUnqualified());
+
+  NamedDecl *Friend
+    = cast<NamedDecl>(FriendD->getFriendDecl()->getCanonicalDecl());
+
+  // FIXME: declarations with dependent or templated scope.
+
+  if (isa<ClassTemplateDecl>(Friend))
+    return MatchesFriend(S, EC, cast<ClassTemplateDecl>(Friend));
+
+  if (isa<FunctionTemplateDecl>(Friend))
+    return MatchesFriend(S, EC, cast<FunctionTemplateDecl>(Friend));
+
+  if (isa<CXXRecordDecl>(Friend))
+    return MatchesFriend(S, EC, cast<CXXRecordDecl>(Friend));
+
+  assert(isa<FunctionDecl>(Friend) && "unknown friend decl kind");
+  return MatchesFriend(S, EC, cast<FunctionDecl>(Friend));
+}
+
 static Sema::AccessResult GetFriendKind(Sema &S,
                                         const EffectiveContext &EC,
                                         const CXXRecordDecl *Class) {
@@ -230,6 +390,8 @@ static CXXBasePath *FindBestPath(Sema &S,
 
   assert(FinalAccess != AS_none && "forbidden access after declaring class");
 
+  bool AnyDependent = false;
+
   // Derive the friend-modified access along each path.
   for (CXXBasePaths::paths_iterator PI = Paths.begin(), PE = Paths.end();
          PI != PE; ++PI) {
@@ -260,7 +422,8 @@ static CXXBasePath *FindBestPath(Sema &S,
           PathAccess = AS_public;
           break;
         case Sema::AR_dependent:
-          return 0;
+          AnyDependent = true;
+          goto Next;
         case Sema::AR_delayed:
           llvm_unreachable("friend resolution is never delayed"); break;
         }
@@ -272,9 +435,23 @@ static CXXBasePath *FindBestPath(Sema &S,
     if (BestPath == 0 || PathAccess < BestPath->Access) {
       BestPath = &*PI;
       BestPath->Access = PathAccess;
+
+      // Short-circuit if we found a public path.
+      if (BestPath->Access == AS_public)
+        return BestPath;
     }
+
+  Next: ;
   }
 
+  assert((!BestPath || BestPath->Access != AS_public) &&
+         "fell out of loop with public path");
+
+  // We didn't find a public path, but at least one path was subject
+  // to dependent friendship, so delay the check.
+  if (AnyDependent)
+    return 0;
+
   return BestPath;
 }
 
@@ -402,7 +579,9 @@ static void DiagnoseBadAccess(Sema &S, SourceLocation Loc,
 
 /// Try to elevate access using friend declarations.  This is
 /// potentially quite expensive.
-static void TryElevateAccess(Sema &S,
+///
+/// \return true if elevation was dependent
+static bool TryElevateAccess(Sema &S,
                              const EffectiveContext &EC,
                              const Sema::AccessedEntity &Entity,
                              AccessSpecifier &Access) {
@@ -424,14 +603,14 @@ static void TryElevateAccess(Sema &S,
       switch (GetFriendKind(S, EC, DeclaringClass)) {
       case Sema::AR_accessible: DeclAccess = AS_public; break;
       case Sema::AR_inaccessible: break;
-      case Sema::AR_dependent: /* FIXME: delay dependent friendship */ return;
+      case Sema::AR_dependent: return true;
       case Sema::AR_delayed: llvm_unreachable("friend status is never delayed");
       }
     }
 
     if (DeclaringClass == NamingClass) {
       Access = DeclAccess;
-      return;
+      return false;
     }
   }
 
@@ -441,16 +620,31 @@ static void TryElevateAccess(Sema &S,
   CXXBasePaths Paths;
   CXXBasePath *Path = FindBestPath(S, EC, Entity.getNamingClass(),
                                    DeclaringClass, DeclAccess, Paths);
-  if (!Path) {
+  if (!Path)
-    // FIXME: delay dependent friendship
+    return true;
-    return;
-  }
 
   // Grab the access along the best path (note that this includes the
   // final-step access).
   AccessSpecifier NewAccess = Path->Access;
   assert(NewAccess <= Access && "access along best path worse than direct?");
   Access = NewAccess;
+  return false;
+}
+
+static void DelayAccess(Sema &S,
+                        const EffectiveContext &EC,
+                        SourceLocation Loc,
+                        const Sema::AccessedEntity &Entity) {
+  assert(EC.isDependent() && "delaying non-dependent access");
+  DeclContext *DC = EC.getPrimaryContext();
+  assert(DC->isDependentContext() && "delaying non-dependent access");
+  DependentDiagnostic::Create(S.Context, DC, DependentDiagnostic::Access,
+                              Loc,
+                              Entity.isMemberAccess(),
+                              Entity.getAccess(),
+                              Entity.getTargetDecl(),
+                              Entity.getNamingClass(),
+                              Entity.getDiag());
 }
 
 /// Checks access to an entity from the given effective context.
@@ -474,10 +668,13 @@ static Sema::AccessResult CheckEffectiveAccess(Sema &S,
     return Sema::AR_accessible;
 
   // Try to elevate access.
-  // FIXME: delay if elevation was dependent?
   // TODO: on some code, it might be better to do the protected check
   // without trying to elevate first.
-  TryElevateAccess(S, EC, Entity, Access);
+  if (TryElevateAccess(S, EC, Entity, Access)) {
+    DelayAccess(S, EC, Loc, Entity);
+    return Sema::AR_dependent;
+  }
+
   if (Access == AS_public) return Sema::AR_accessible;
 
   // Protected access.
@@ -526,6 +723,35 @@ void Sema::HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *Ctx) {
     DD.Triggered = true;
 }
 
+void Sema::HandleDependentAccessCheck(const DependentDiagnostic &DD,
+                        const MultiLevelTemplateArgumentList &TemplateArgs) {
+  SourceLocation Loc = DD.getAccessLoc();
+  AccessSpecifier Access = DD.getAccess();
+
+  Decl *NamingD = FindInstantiatedDecl(Loc, DD.getAccessNamingClass(),
+                                       TemplateArgs);
+  if (!NamingD) return;
+  Decl *TargetD = FindInstantiatedDecl(Loc, DD.getAccessTarget(),
+                                       TemplateArgs);
+  if (!TargetD) return;
+
+  if (DD.isAccessToMember()) {
+    AccessedEntity Entity(AccessedEntity::Member,
+                          cast<CXXRecordDecl>(NamingD),
+                          Access,
+                          cast<NamedDecl>(TargetD));
+    Entity.setDiag(DD.getDiagnostic());
+    CheckAccess(*this, Loc, Entity);
+  } else {
+    AccessedEntity Entity(AccessedEntity::Base,
+                          cast<CXXRecordDecl>(TargetD),
+                          cast<CXXRecordDecl>(NamingD),
+                          Access);
+    Entity.setDiag(DD.getDiagnostic());
+    CheckAccess(*this, Loc, Entity);
+  }
+}
+
 Sema::AccessResult Sema::CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E,
                                                      DeclAccessPair Found) {
   if (!getLangOptions().AccessControl ||

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -15,6 +15,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclVisitor.h"
+#include "clang/AST/DependentDiagnostic.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/TypeLoc.h"
@@ -1839,6 +1840,8 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   ActOnFinishFunctionBody(DeclPtrTy::make(Function), move(Body),
                           /*IsInstantiation=*/true);
 
+  PerformDependentDiagnostics(PatternDecl, TemplateArgs);
+
   CurContext = PreviousContext;
 
   DeclGroupRef DG(Function);
@@ -2475,3 +2478,17 @@ void Sema::PerformPendingImplicitInstantiations(bool LocalOnly) {
     InstantiateStaticDataMemberDefinition(/*FIXME:*/Inst.second, Var, true);
   }
 }
+
+void Sema::PerformDependentDiagnostics(const DeclContext *Pattern,
+                       const MultiLevelTemplateArgumentList &TemplateArgs) {
+  for (DeclContext::ddiag_iterator I = Pattern->ddiag_begin(),
+         E = Pattern->ddiag_end(); I != E; ++I) {
+    DependentDiagnostic *DD = *I;
+
+    switch (DD->getKind()) {
+    case DependentDiagnostic::Access:
+      HandleDependentAccessCheck(*DD, TemplateArgs);
+      break;
+    }
+  }
+}

clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp

@@ -85,3 +85,35 @@ namespace test2 {
     }
   };
 }
+
+namespace test3 {
+  class Bool;
+  template <class T> class User;
+  template <class T> T transform(class Bool, T);
+
+  class Bool {
+    friend class User<bool>;
+    friend bool transform<>(Bool, bool);
+
+    bool value; // expected-note {{declared private here}}
+  };
+
+  template <class T> class User {
+    static T compute(Bool b) {
+      return b.value; // expected-error {{'value' is a private member of 'test3::Bool'}}
+    }
+  };
+
+  template <class T> T transform(Bool b, T value) {
+    if (b.value)
+      return value;
+    return value + 1;
+  }
+
+  template bool transform(Bool, bool);
+  template int transform(Bool, int);
+
+  template class User<bool>;
+  template class User<int>; // expected-note {{requested here}}
+
+}