Fix bug 4784 and allow friend declarations to properly extend

existing declaration chains.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@80636 91177308-0d34-0410-b5e6-96231b3b80d8

7 files changed, 79 insertions, 13 deletions

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 9aea17bd1b..dcd2482eb3 100644
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -905,7 +905,11 @@ public:
   /// visible from this context, as determined by
   /// NamedDecl::declarationReplaces, the previous declaration will be
   /// replaced with D.
-  void makeDeclVisibleInContext(NamedDecl *D);
+  ///
+  /// @param Recoverable true if it's okay to not add this decl to
+  /// the lookup tables because it can be easily recovered by walking
+  /// the declaration chains.
+  void makeDeclVisibleInContext(NamedDecl *D, bool Recoverable = true);
 
   /// udir_iterator - Iterates through the using-directives stored
   /// within this context.
diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 3a010b0cdf..c24dac9104 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -697,7 +697,7 @@ DeclContext *DeclContext::getEnclosingNamespaceContext() {
   return Ctx->getPrimaryContext();
 }
 
-void DeclContext::makeDeclVisibleInContext(NamedDecl *D) {
+void DeclContext::makeDeclVisibleInContext(NamedDecl *D, bool Recoverable) {
   // FIXME: This feels like a hack. Should DeclarationName support
   // template-ids, or is there a better way to keep specializations
   // from being visible?
@@ -706,20 +706,20 @@ void DeclContext::makeDeclVisibleInContext(NamedDecl *D) {
 
   DeclContext *PrimaryContext = getPrimaryContext();
   if (PrimaryContext != this) {
-    PrimaryContext->makeDeclVisibleInContext(D);
+    PrimaryContext->makeDeclVisibleInContext(D, Recoverable);
     return;
   }
 
   // If we already have a lookup data structure, perform the insertion
   // into it. Otherwise, be lazy and don't build that structure until
   // someone asks for it.
-  if (LookupPtr)
+  if (LookupPtr || !Recoverable)
     makeDeclVisibleInContextImpl(D);
 
   // If we are a transparent context, insert into our parent context,
   // too. This operation is recursive.
   if (isTransparentContext())
-    getParent()->makeDeclVisibleInContext(D);
+    getParent()->makeDeclVisibleInContext(D, Recoverable);
 }
 
 void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D) {
diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index ce56eb4637..a8cdb67bb3 100644
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -703,7 +703,7 @@ public:
   NamedDecl *getCurFunctionOrMethodDecl();
 
   /// Add this decl to the scope shadowed decl chains.
-  void PushOnScopeChains(NamedDecl *D, Scope *S);
+  void PushOnScopeChains(NamedDecl *D, Scope *S, bool AddToContext = true);
 
   /// isDeclInScope - If 'Ctx' is a function/method, isDeclInScope returns true
   /// if 'D' is in Scope 'S', otherwise 'S' is ignored and isDeclInScope returns
@@ -712,6 +712,18 @@ public:
     return IdResolver.isDeclInScope(D, Ctx, Context, S);
   }
 
+  /// Finds the scope corresponding to the given decl context, if it
+  /// happens to be an enclosing scope.  Otherwise return NULL.
+  Scope *getScopeForDeclContext(Scope *S, DeclContext *DC) {
+    DC = DC->getPrimaryContext();
+    do {
+      if (((DeclContext*) S->getEntity())->getPrimaryContext() == DC)
+        return S;
+    } while ((S = S->getParent()));
+
+    return NULL;
+  }
+
 
   /// Subroutines of ActOnDeclarator().
   TypedefDecl *ParseTypedefDecl(Scope *S, Declarator &D, QualType T);
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 4d05144918..8eb2c717b4 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -280,7 +280,7 @@ static bool AllowOverloadingOfFunction(Decl *PrevDecl, ASTContext &Context) {
 }
 
 /// Add this decl to the scope shadowed decl chains.
-void Sema::PushOnScopeChains(NamedDecl *D, Scope *S) {
+void Sema::PushOnScopeChains(NamedDecl *D, Scope *S, bool AddToContext) {
   // Move up the scope chain until we find the nearest enclosing
   // non-transparent context. The declaration will be introduced into this
   // scope.
@@ -293,7 +293,8 @@ void Sema::PushOnScopeChains(NamedDecl *D, Scope *S) {
   // Add scoped declarations into their context, so that they can be
   // found later. Declarations without a context won't be inserted
   // into any context.
-  CurContext->addDecl(D);
+  if (AddToContext)
+    CurContext->addDecl(D);
 
   // C++ [basic.scope]p4:
   //   -- exactly one declaration shall declare a class name or
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index c729b6a7c7..4acde29b7b 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -3820,15 +3820,27 @@ Sema::DeclPtrTy Sema::ActOnFriendFunctionDecl(Scope *S,
                                           IsDefinition,
                                           Redeclaration);
   if (!ND) return DeclPtrTy();
+
+  assert(cast<FunctionDecl>(ND)->getPreviousDeclaration() == FD &&
+         "lost reference to previous declaration");
+
   FD = cast<FunctionDecl>(ND);
 
   assert(FD->getDeclContext() == DC);
   assert(FD->getLexicalDeclContext() == CurContext);
 
-  // We only add the function declaration to the lookup tables, not
-  // the decl list, and only if the context isn't dependent.
-  if (!CurContext->isDependentContext())
-    DC->makeDeclVisibleInContext(FD);
+  // Add the function declaration to the appropriate lookup tables,
+  // adjusting the redeclarations list as necessary.  We don't
+  // want to do this yet if the friending class is dependent.
+  // 
+  // Also update the scope-based lookup if the target context's
+  // lookup context is in lexical scope.
+  if (!CurContext->isDependentContext()) {
+    DC = DC->getLookupContext();
+    DC->makeDeclVisibleInContext(FD, /* Recoverable=*/ false);
+    if (Scope *EnclosingScope = getScopeForDeclContext(S, DC))
+      PushOnScopeChains(FD, EnclosingScope, /*AddToContext=*/ false);
+  }
 
   FriendDecl *FrD = FriendDecl::Create(Context, CurContext,
                                        D.getIdentifierLoc(), FD,
diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index aa116b225d..c5b2894c49 100644
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -498,7 +498,7 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D) {
     bool WasDeclared = (FOK == Decl::FOK_Declared);
     Function->setObjectOfFriendDecl(WasDeclared);
     if (!Owner->isDependentContext())
-      DC->makeDeclVisibleInContext(Function);
+      DC->makeDeclVisibleInContext(Function, /* Recoverable = */ false);
 
     Function->setInstantiationOfMemberFunction(D);
   }
diff --git a/test/CXX/class/class.friend/p1-ambiguous.cpp b/test/CXX/class/class.friend/p1-ambiguous.cpp
new file mode 100644
index 0000000000..a02bc53375
--- /dev/null
+++ b/test/CXX/class/class.friend/p1-ambiguous.cpp
@@ -0,0 +1,37 @@
+// RUN: clang-cc -fsyntax-only -verify %s
+
+// Make sure that friend declarations don't introduce ambiguous
+// declarations.
+
+// Test case courtesy of Shantonu Sen.
+// Bug 4784.
+
+class foo;
+
+extern "C" {
+  int c_func(foo *a);
+};
+int cpp_func(foo *a);
+
+class foo {
+public:
+  friend int c_func(foo *a);
+  friend int cpp_func(foo *a);
+  int caller();
+private:
+  int x;
+};
+
+int c_func(foo *a) {
+  return a->x;
+}
+
+int cpp_func(foo *a) {
+  return a->x;
+}
+
+int foo::caller() {
+    c_func(this);
+    cpp_func(this);
+    return 0;
+}