r228371 - [modules] If a module declares an entity and then imports another declaration
Richard Smith
richard-llvm at metafoo.co.uk
Thu Feb 5 18:42:59 PST 2015
Author: rsmith
Date: Thu Feb 5 20:42:59 2015
New Revision: 228371
URL: http://llvm.org/viewvc/llvm-project?rev=228371&view=rev
Log:
[modules] If a module declares an entity and then imports another declaration
of that entity, ensure that the redeclaration chain is reordered properly on
reload. Otherwise, the result of name lookup for that entity may point to an
entity that is too old; if that's an injected friend name or the like, that
can result in the name not being found at all.
Added:
cfe/trunk/test/Modules/Inputs/merge-friends/
cfe/trunk/test/Modules/Inputs/merge-friends/decl.h
cfe/trunk/test/Modules/Inputs/merge-friends/friend.h
cfe/trunk/test/Modules/Inputs/merge-friends/module.modulemap
cfe/trunk/test/Modules/merge-friends.cpp
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/Modules/cxx-templates.cpp
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=228371&r1=228370&r2=228371&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Thu Feb 5 20:42:59 2015
@@ -124,6 +124,7 @@ namespace clang {
class RedeclarableResult {
ASTReader &Reader;
GlobalDeclID FirstID;
+ Decl *MergeWith;
mutable bool Owning;
Decl::Kind DeclKind;
@@ -131,13 +132,14 @@ namespace clang {
public:
RedeclarableResult(ASTReader &Reader, GlobalDeclID FirstID,
- Decl::Kind DeclKind)
- : Reader(Reader), FirstID(FirstID), Owning(true), DeclKind(DeclKind) { }
+ Decl *MergeWith, Decl::Kind DeclKind)
+ : Reader(Reader), FirstID(FirstID), MergeWith(MergeWith),
+ Owning(true), DeclKind(DeclKind) {}
RedeclarableResult(const RedeclarableResult &Other)
- : Reader(Other.Reader), FirstID(Other.FirstID), Owning(Other.Owning) ,
- DeclKind(Other.DeclKind)
- {
+ : Reader(Other.Reader), FirstID(Other.FirstID),
+ MergeWith(Other.MergeWith), Owning(Other.Owning),
+ DeclKind(Other.DeclKind) {
Other.Owning = false;
}
@@ -149,7 +151,11 @@ namespace clang {
/// \brief Retrieve the first ID.
GlobalDeclID getFirstID() const { return FirstID; }
-
+
+ /// \brief Get a known declaration that this should be merged with, if
+ /// any.
+ Decl *getKnownMergeTarget() const { return MergeWith; }
+
/// \brief Do not introduce this declaration ID into the set of pending
/// declaration chains.
void suppress() {
@@ -2077,15 +2083,23 @@ ASTDeclReader::VisitDeclContext(DeclCont
}
template <typename T>
-ASTDeclReader::RedeclarableResult
+ASTDeclReader::RedeclarableResult
ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
DeclID FirstDeclID = ReadDeclID(Record, Idx);
-
+ Decl *MergeWith = nullptr;
+
// 0 indicates that this declaration was the only declaration of its entity,
// and is used for space optimization.
if (FirstDeclID == 0)
FirstDeclID = ThisDeclID;
-
+ else if (Record[Idx++]) {
+ // We need to merge with FirstDeclID. Read it now to ensure that it is
+ // before us in the redecl chain, then forget we saw it so that we will
+ // merge with it.
+ MergeWith = Reader.GetDecl(FirstDeclID);
+ FirstDeclID = ThisDeclID;
+ }
+
T *FirstDecl = cast_or_null<T>(Reader.GetDecl(FirstDeclID));
if (FirstDecl != D) {
// We delay loading of the redeclaration chain to avoid deeply nested calls.
@@ -2100,7 +2114,7 @@ ASTDeclReader::VisitRedeclarable(Redecla
// The result structure takes care to note that we need to load the
// other declaration chains for this ID.
- return RedeclarableResult(Reader, FirstDeclID,
+ return RedeclarableResult(Reader, FirstDeclID, MergeWith,
static_cast<T *>(D)->getKind());
}
@@ -2127,7 +2141,10 @@ void ASTDeclReader::mergeRedeclarable(Re
if (!Reader.getContext().getLangOpts().Modules)
return;
- if (FindExistingResult ExistingRes = findExisting(D))
+ if (auto *Existing = Redecl.getKnownMergeTarget())
+ // We already know of an existing declaration we should merge with.
+ mergeRedeclarable(D, cast<T>(Existing), Redecl, TemplatePatternID);
+ else if (FindExistingResult ExistingRes = findExisting(D))
if (T *Existing = ExistingRes)
mergeRedeclarable(D, Existing, Redecl, TemplatePatternID);
}
@@ -2148,7 +2165,7 @@ void ASTDeclReader::mergeTemplatePattern
auto *DPattern = D->getTemplatedDecl();
auto *ExistingPattern = Existing->getTemplatedDecl();
RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(),
- DPattern->getKind());
+ /*MergeWith*/ExistingPattern, DPattern->getKind());
if (auto *DClass = dyn_cast<CXXRecordDecl>(DPattern)) {
// Merge with any existing definition.
Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=228371&r1=228370&r2=228371&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Thu Feb 5 20:42:59 2015
@@ -1448,25 +1448,42 @@ void ASTDeclWriter::VisitDeclContext(Dec
template <typename T>
void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
T *First = D->getFirstDecl();
- if (First->getMostRecentDecl() != First) {
+ T *MostRecent = First->getMostRecentDecl();
+ if (MostRecent != First) {
assert(isRedeclarableDeclKind(static_cast<T *>(D)->getKind()) &&
"Not considered redeclarable?");
-
+
+ auto *Previous = D->getPreviousDecl();
+ auto *FirstToEmit = First;
+ if (Context.getLangOpts().Modules && Writer.Chain && !Previous) {
+ // In a modules build, we can have imported declarations after a local
+ // canonical declaration. If we do, we want to treat the first imported
+ // declaration as our canonical declaration on reload, in order to
+ // rebuild the redecl chain in the right order.
+ for (auto *Redecl = MostRecent; Redecl;
+ Redecl = Redecl->getPreviousDecl())
+ if (Redecl->isFromASTFile())
+ FirstToEmit = Redecl;
+ }
+
// There is more than one declaration of this entity, so we will need to
// write a redeclaration chain.
- Writer.AddDeclRef(First, Record);
+ Writer.AddDeclRef(FirstToEmit, Record);
+ Record.push_back(FirstToEmit != First);
Writer.Redeclarations.insert(First);
// Make sure that we serialize both the previous and the most-recent
// declarations, which (transitively) ensures that all declarations in the
// chain get serialized.
- (void)Writer.GetDeclRef(D->getPreviousDecl());
- (void)Writer.GetDeclRef(First->getMostRecentDecl());
+ //
+ // FIXME: This is not correct; when we reach an imported declaration we
+ // won't emit its previous declaration.
+ (void)Writer.GetDeclRef(Previous);
+ (void)Writer.GetDeclRef(MostRecent);
} else {
// We use the sentinel value 0 to indicate an only declaration.
Record.push_back(0);
}
-
}
void ASTDeclWriter::VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D) {
@@ -1948,9 +1965,10 @@ void ASTWriter::WriteDecl(ASTContext &Co
// Determine the ID for this declaration.
serialization::DeclID ID;
- if (D->isFromASTFile())
+ if (D->isFromASTFile()) {
+ assert(isRewritten(D) && "should not be emitting imported decl");
ID = getDeclID(D);
- else {
+ } else {
serialization::DeclID &IDR = DeclIDs[D];
if (IDR == 0)
IDR = NextDeclID++;
Added: cfe/trunk/test/Modules/Inputs/merge-friends/decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-friends/decl.h?rev=228371&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-friends/decl.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-friends/decl.h Thu Feb 5 20:42:59 2015
@@ -0,0 +1 @@
+namespace N { struct foo; }
Added: cfe/trunk/test/Modules/Inputs/merge-friends/friend.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-friends/friend.h?rev=228371&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-friends/friend.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-friends/friend.h Thu Feb 5 20:42:59 2015
@@ -0,0 +1,2 @@
+namespace N { struct n8 { friend struct foo; }; }
+#include "decl.h"
Added: cfe/trunk/test/Modules/Inputs/merge-friends/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-friends/module.modulemap?rev=228371&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-friends/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/merge-friends/module.modulemap Thu Feb 5 20:42:59 2015
@@ -0,0 +1,2 @@
+module decl { header "decl.h" export * }
+module friend { header "friend.h" export * }
Modified: cfe/trunk/test/Modules/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=228371&r1=228370&r2=228371&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)
+++ cfe/trunk/test/Modules/cxx-templates.cpp Thu Feb 5 20:42:59 2015
@@ -28,14 +28,8 @@ void g() {
f<double>(1.0);
f<int>();
f(); // expected-error {{no matching function}}
-#ifdef EARLY_IMPORT
- // FIXME: The textual inclusion above shouldn't affect this!
- // expected-note at Inputs/cxx-templates-a.h:3 {{couldn't infer template argument}}
- // expected-note at Inputs/cxx-templates-a.h:4 {{requires 1 argument}}
-#else
// expected-note at Inputs/cxx-templates-b.h:3 {{couldn't infer template argument}}
// expected-note at Inputs/cxx-templates-b.h:4 {{requires single argument}}
-#endif
N::f(0);
N::f<double>(1.0);
Added: cfe/trunk/test/Modules/merge-friends.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-friends.cpp?rev=228371&view=auto
==============================================================================
--- cfe/trunk/test/Modules/merge-friends.cpp (added)
+++ cfe/trunk/test/Modules/merge-friends.cpp Thu Feb 5 20:42:59 2015
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/merge-friends -verify %s
+// expected-no-diagnostics
+#include "friend.h"
+N::foo *use;
More information about the cfe-commits
mailing list