From 2b5605751786a85f05da3b0a344aa832e582080f Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Sat, 7 Feb 2015 03:11:11 +0000 Subject: [PATCH] [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 | 4 ++++ clang/lib/Serialization/ASTCommon.cpp | 18 +++++++++++++++ clang/lib/Serialization/ASTCommon.h | 19 ++++++++++++++++ clang/lib/Serialization/ASTReaderDecl.cpp | 22 ++++++------------- clang/lib/Serialization/ASTWriter.cpp | 11 ++++------ clang/lib/Serialization/ASTWriterDecl.cpp | 13 +++++++++-- .../Inputs/merge-dependent-friends/a.h | 2 ++ .../Inputs/merge-dependent-friends/b.h | 2 ++ .../Inputs/merge-dependent-friends/c.h | 6 +++++ .../Inputs/merge-dependent-friends/d.h | 2 ++ .../merge-dependent-friends/module.modulemap | 4 ++++ .../test/Modules/merge-dependent-friends.cpp | 4 ++++ 12 files changed, 83 insertions(+), 24 deletions(-) create mode 100644 clang/test/Modules/Inputs/merge-dependent-friends/a.h create mode 100644 clang/test/Modules/Inputs/merge-dependent-friends/b.h create mode 100644 clang/test/Modules/Inputs/merge-dependent-friends/c.h create mode 100644 clang/test/Modules/Inputs/merge-dependent-friends/d.h create mode 100644 clang/test/Modules/Inputs/merge-dependent-friends/module.modulemap create mode 100644 clang/test/Modules/merge-dependent-friends.cpp diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 8862f9f5c621..5f4aa113b7d3 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/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(); } diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp index 13393225b60e..04425acb4c5d 100644 --- a/clang/lib/Serialization/ASTCommon.cpp +++ b/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(D)) { + // For function templates and class templates, the template is numbered and + // not its pattern. + if (auto *FD = dyn_cast(D)) + return !FD->getDescribedFunctionTemplate(); + if (auto *RD = dyn_cast(D)) + return !RD->getDescribedClassTemplate(); + return true; + } + + // Otherwise, we only care about anonymous class members. if (D->getDeclName() || !isa(D->getLexicalDeclContext())) return false; return isa(D) || isa(D); diff --git a/clang/lib/Serialization/ASTCommon.h b/clang/lib/Serialization/ASTCommon.h index 38a0ff5fbd4e..88cdbcfe179e 100644 --- a/clang/lib/Serialization/ASTCommon.h +++ b/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 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(LexicalD)) + LexicalD = FD->getFriendDecl(); + + auto *ND = dyn_cast_or_null(LexicalD); + if (!ND || !needsAnonymousDeclarationNumber(ND)) + continue; + + Visit(ND, Index++); + } +} + } // namespace serialization } // namespace clang diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 970ec84e0425..5fc99b53122c 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/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(DC)->isFromASTFile()) { - unsigned Index = 0; - for (Decl *LexicalD : DC->decls()) { - auto *ND = dyn_cast(LexicalD); - if (!ND || !needsAnonymousDeclarationNumber(ND)) - continue; - if (Previous.size() == Index) + numberAnonymousDeclsWithin(DC, [&](NamedDecl *ND, unsigned Number) { + if (Previous.size() == Number) Previous.push_back(cast(ND->getCanonicalDecl())); else - Previous[Index] = cast(ND->getCanonicalDecl()); - ++Index; - } + Previous[Number] = cast(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)) diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 41ef7ef2fc07..aad4305d8c58 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/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(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() && diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index e4a0426ade63..7f2e80583e5c 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/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 diff --git a/clang/test/Modules/Inputs/merge-dependent-friends/a.h b/clang/test/Modules/Inputs/merge-dependent-friends/a.h new file mode 100644 index 000000000000..44b1c1db5688 --- /dev/null +++ b/clang/test/Modules/Inputs/merge-dependent-friends/a.h @@ -0,0 +1,2 @@ +namespace N { template struct A { friend int f(A); }; } +int a = f(N::A()); diff --git a/clang/test/Modules/Inputs/merge-dependent-friends/b.h b/clang/test/Modules/Inputs/merge-dependent-friends/b.h new file mode 100644 index 000000000000..bce33876d54d --- /dev/null +++ b/clang/test/Modules/Inputs/merge-dependent-friends/b.h @@ -0,0 +1,2 @@ +namespace N { template struct A { friend int f(A); }; } +int b = f(N::A()); diff --git a/clang/test/Modules/Inputs/merge-dependent-friends/c.h b/clang/test/Modules/Inputs/merge-dependent-friends/c.h new file mode 100644 index 000000000000..3620ce3d539f --- /dev/null +++ b/clang/test/Modules/Inputs/merge-dependent-friends/c.h @@ -0,0 +1,6 @@ +namespace N { template 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()); +#include "a.h" +#include "b.h" diff --git a/clang/test/Modules/Inputs/merge-dependent-friends/d.h b/clang/test/Modules/Inputs/merge-dependent-friends/d.h new file mode 100644 index 000000000000..ce3f69fa387f --- /dev/null +++ b/clang/test/Modules/Inputs/merge-dependent-friends/d.h @@ -0,0 +1,2 @@ +namespace N { template struct A { friend int f(A); }; } +#include "c.h" diff --git a/clang/test/Modules/Inputs/merge-dependent-friends/module.modulemap b/clang/test/Modules/Inputs/merge-dependent-friends/module.modulemap new file mode 100644 index 000000000000..73a7c41ec589 --- /dev/null +++ b/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 * } diff --git a/clang/test/Modules/merge-dependent-friends.cpp b/clang/test/Modules/merge-dependent-friends.cpp new file mode 100644 index 000000000000..0b0c903c318f --- /dev/null +++ b/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"