[modules] Treat friend declarations that are lexically within a dependent

context as anonymous for merging purposes. They can't be found by their names,
so we merge them based on their position within the surrounding context.

llvm-svn: 228485

clang/lib/AST/DeclBase.cpp

@@ -846,6 +846,10 @@ bool DeclContext::isDependentContext() const {
       return getLexicalParent()->isDependentContext();
   }
 
+  // FIXME: A variable template is a dependent context, but is not a
+  // DeclContext. A context within it (such as a lambda-expression)
+  // should be considered dependent.
+
   return getParent() && getParent()->isDependentContext();
 }
 

clang/lib/Serialization/ASTCommon.cpp

@@ -223,6 +223,24 @@ bool serialization::isRedeclarableDeclKind(unsigned Kind) {
 }
 
 bool serialization::needsAnonymousDeclarationNumber(const NamedDecl *D) {
+  // Friend declarations in dependent contexts aren't anonymous in the usual
+  // sense, but they cannot be found by name lookup in their semantic context
+  // (or indeed in any context), so we treat them as anonymous.
+  //
+  // This doesn't apply to friend tag decls; Sema makes those available to name
+  // lookup in the surrounding context.
+  if (D->getFriendObjectKind() &&
+      D->getLexicalDeclContext()->isDependentContext() && !isa<TagDecl>(D)) {
+    // For function templates and class templates, the template is numbered and
+    // not its pattern.
+    if (auto *FD = dyn_cast<FunctionDecl>(D))
+      return !FD->getDescribedFunctionTemplate();
+    if (auto *RD = dyn_cast<CXXRecordDecl>(D))
+      return !RD->getDescribedClassTemplate();
+    return true;
+  }
+
+  // Otherwise, we only care about anonymous class members.
   if (D->getDeclName() || !isa<CXXRecordDecl>(D->getLexicalDeclContext()))
     return false;
   return isa<TagDecl>(D) || isa<FieldDecl>(D);

clang/lib/Serialization/ASTCommon.h

@@ -15,6 +15,7 @@
 #define LLVM_CLANG_LIB_SERIALIZATION_ASTCOMMON_H
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclFriend.h"
 #include "clang/Serialization/ASTBitCodes.h"
 
 namespace clang {
@@ -85,6 +86,24 @@ bool isRedeclarableDeclKind(unsigned Kind);
 /// declaration number.
 bool needsAnonymousDeclarationNumber(const NamedDecl *D);
 
+/// \brief Visit each declaration within \c DC that needs an anonymous
+/// declaration number and call \p Visit with the declaration and its number.
+template<typename Fn> void numberAnonymousDeclsWithin(const DeclContext *DC,
+                                                      Fn Visit) {
+  unsigned Index = 0;
+  for (Decl *LexicalD : DC->decls()) {
+    // For a friend decl, we care about the declaration within it, if any.
+    if (auto *FD = dyn_cast<FriendDecl>(LexicalD))
+      LexicalD = FD->getFriendDecl();
+
+    auto *ND = dyn_cast_or_null<NamedDecl>(LexicalD);
+    if (!ND || !needsAnonymousDeclarationNumber(ND))
+      continue;
+
+    Visit(ND, Index++);
+  }
+}
+
 } // namespace serialization
 
 } // namespace clang

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -478,8 +478,7 @@ void ASTDeclReader::VisitTranslationUnitDecl(TranslationUnitDecl *TU) {
 void ASTDeclReader::VisitNamedDecl(NamedDecl *ND) {
   VisitDecl(ND);
   ND->setDeclName(Reader.ReadDeclarationName(F, Record, Idx));
-  if (needsAnonymousDeclarationNumber(ND))
-    AnonymousDeclNumber = Record[Idx++];
+  AnonymousDeclNumber = Record[Idx++];
 }
 
 void ASTDeclReader::VisitTypeDecl(TypeDecl *TD) {
@@ -2631,8 +2630,7 @@ ASTDeclReader::FindExistingResult::~FindExistingResult() {
 
   DeclarationName Name = New->getDeclName();
   DeclContext *DC = New->getDeclContext()->getRedeclContext();
-  if (!Name) {
-    assert(needsAnonymousDeclarationNumber(New));
+  if (needsAnonymousDeclarationNumber(New)) {
     setAnonymousDeclForMerging(Reader, New->getLexicalDeclContext(),
                                AnonymousDeclNumber, New);
   } else if (DC->isTranslationUnit() && Reader.SemaObj) {
@@ -2681,17 +2679,12 @@ NamedDecl *ASTDeclReader::getAnonymousDeclForMerging(ASTReader &Reader,
   // If this is the first time, but we have parsed a declaration of the context,
   // build the anonymous declaration list from the parsed declaration.
   if (!cast<Decl>(DC)->isFromASTFile()) {
-    unsigned Index = 0;
-    for (Decl *LexicalD : DC->decls()) {
-      auto *ND = dyn_cast<NamedDecl>(LexicalD);
-      if (!ND || !needsAnonymousDeclarationNumber(ND))
-        continue;
-      if (Previous.size() == Index)
+    numberAnonymousDeclsWithin(DC, [&](NamedDecl *ND, unsigned Number) {
+      if (Previous.size() == Number)
         Previous.push_back(cast<NamedDecl>(ND->getCanonicalDecl()));
       else
-        Previous[Index] = cast<NamedDecl>(ND->getCanonicalDecl());
-      ++Index;
-    }
+        Previous[Number] = cast<NamedDecl>(ND->getCanonicalDecl());
+    });
   }
 
   return Index < Previous.size() ? Previous[Index] : nullptr;
@@ -2740,10 +2733,9 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) {
     // was not imported.
   }
 
-  if (!Name) {
+  if (needsAnonymousDeclarationNumber(D)) {
     // This is an anonymous declaration that we may need to merge. Look it up
     // in its context by number.
-    assert(needsAnonymousDeclarationNumber(D));
     if (auto *Existing = getAnonymousDeclForMerging(
             Reader, D->getLexicalDeclContext(), AnonymousDeclNumber))
       if (isSameEntity(Existing, D))

clang/lib/Serialization/ASTWriter.cpp

@@ -5226,13 +5226,10 @@ unsigned ASTWriter::getAnonymousDeclarationNumber(const NamedDecl *D) {
   // already done so.
   auto It = AnonymousDeclarationNumbers.find(D);
   if (It == AnonymousDeclarationNumbers.end()) {
-    unsigned Index = 0;
-    for (Decl *LexicalD : D->getLexicalDeclContext()->decls()) {
-      auto *ND = dyn_cast<NamedDecl>(LexicalD);
-      if (!ND || !needsAnonymousDeclarationNumber(ND))
-        continue;
-      AnonymousDeclarationNumbers[ND] = Index++;
-    }
+    auto *DC = D->getLexicalDeclContext();
+    numberAnonymousDeclsWithin(DC, [&](const NamedDecl *ND, unsigned Number) {
+      AnonymousDeclarationNumbers[ND] = Number;
+    });
 
     It = AnonymousDeclarationNumbers.find(D);
     assert(It != AnonymousDeclarationNumbers.end() &&

clang/lib/Serialization/ASTWriterDecl.cpp

@@ -203,8 +203,9 @@ void ASTDeclWriter::VisitTranslationUnitDecl(TranslationUnitDecl *D) {
 void ASTDeclWriter::VisitNamedDecl(NamedDecl *D) {
   VisitDecl(D);
   Writer.AddDeclarationName(D->getDeclName(), Record);
-  if (needsAnonymousDeclarationNumber(D))
-    Record.push_back(Writer.getAnonymousDeclarationNumber(D));
+  Record.push_back(needsAnonymousDeclarationNumber(D)
+                       ? Writer.getAnonymousDeclarationNumber(D)
+                       : 0);
 }
 
 void ASTDeclWriter::VisitTypeDecl(TypeDecl *D) {
@@ -1521,6 +1522,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Name
+  Abv->Add(BitCodeAbbrevOp(0));                       // AnonDeclNumber
   // ValueDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
   // DeclaratorDecl
@@ -1553,6 +1555,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Name
+  Abv->Add(BitCodeAbbrevOp(0));                       // AnonDeclNumber
   // ValueDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
   // DeclaratorDecl
@@ -1590,6 +1593,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Name
+  Abv->Add(BitCodeAbbrevOp(0));                       // AnonDeclNumber
   // TypeDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type Ref
@@ -1637,6 +1641,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Name
+  Abv->Add(BitCodeAbbrevOp(0));                       // AnonDeclNumber
   // TypeDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type Ref
@@ -1679,6 +1684,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Name
+  Abv->Add(BitCodeAbbrevOp(0));                       // AnonDeclNumber
   // ValueDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
   // DeclaratorDecl
@@ -1732,6 +1738,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Name
+  Abv->Add(BitCodeAbbrevOp(0));                       // AnonDeclNumber
   // TypeDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type Ref
@@ -1760,6 +1767,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // NameKind = Identifier
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Name
+  Abv->Add(BitCodeAbbrevOp(0));                       // AnonDeclNumber
   // ValueDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
   // DeclaratorDecl
@@ -1805,6 +1813,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   // NamedDecl
   Abv->Add(BitCodeAbbrevOp(DeclarationName::Identifier)); // NameKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // Identifier
+  Abv->Add(BitCodeAbbrevOp(0));                         // AnonDeclNumber
   // ValueDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // Type
   // DeclaratorDecl

clang/test/Modules/Inputs/merge-dependent-friends/a.h

@@ -0,0 +1,2 @@
+namespace N { template<typename T> struct A { friend int f(A); }; }
+int a = f(N::A<int>());

clang/test/Modules/Inputs/merge-dependent-friends/b.h

@@ -0,0 +1,2 @@
+namespace N { template<typename T> struct A { friend int f(A); }; }
+int b = f(N::A<int>());

clang/test/Modules/Inputs/merge-dependent-friends/c.h

@@ -0,0 +1,6 @@
+namespace N { template<typename T> struct A { friend int f(A); }; }
+// It would seem like this variable should be called 'c'.
+// But that makes the original problem disappear...
+int e = f(N::A<int>());
+#include "a.h"
+#include "b.h"

clang/test/Modules/Inputs/merge-dependent-friends/d.h

@@ -0,0 +1,2 @@
+namespace N { template<typename T> struct A { friend int f(A); }; }
+#include "c.h"

clang/test/Modules/Inputs/merge-dependent-friends/module.modulemap

@@ -0,0 +1,4 @@
+module a { header "a.h" export * }
+module b { header "b.h" export * }
+module c { header "c.h" export * }
+module d { header "d.h" export * }

clang/test/Modules/merge-dependent-friends.cpp

@@ -0,0 +1,4 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/merge-dependent-friends -verify %s
+// expected-no-diagnostics
+#include "d.h"