[PATCH] D34741: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306583: [ASTReader] Treat multiple defns of ObjC protocols
the same as interfaces. (authored by graydon).
Repository:
rL LLVM
https://reviews.llvm.org/D34741
Files:
cfe/trunk/include/clang/AST/Redeclarable.h
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
Index: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
@@ -126,6 +126,9 @@
void ReadObjCDefinitionData(struct ObjCInterfaceDecl::DefinitionData );
void MergeDefinitionData(ObjCInterfaceDecl *D,
struct ObjCInterfaceDecl::DefinitionData &);
+void ReadObjCDefinitionData(struct ObjCProtocolDecl::DefinitionData );
+void MergeDefinitionData(ObjCProtocolDecl *D,
+ struct ObjCProtocolDecl::DefinitionData &);
static NamedDecl *getAnonymousDeclForMerging(ASTReader ,
DeclContext *DC,
@@ -1045,18 +1048,8 @@
IVD->setSynthesize(synth);
}
-void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {
- RedeclarableResult Redecl = VisitRedeclarable(PD);
- VisitObjCContainerDecl(PD);
- mergeRedeclarable(PD, Redecl);
-
- if (Record.readInt()) {
-// Read the definition.
-PD->allocateDefinitionData();
-
-// Set the definition data of the canonical declaration, so other
-// redeclarations will see it.
-PD->getCanonicalDecl()->Data = PD->Data;
+void ASTDeclReader::ReadObjCDefinitionData(
+ struct ObjCProtocolDecl::DefinitionData ) {
unsigned NumProtoRefs = Record.readInt();
SmallVector ProtoRefs;
@@ -1067,9 +1060,37 @@
ProtoLocs.reserve(NumProtoRefs);
for (unsigned I = 0; I != NumProtoRefs; ++I)
ProtoLocs.push_back(ReadSourceLocation());
-PD->setProtocolList(ProtoRefs.data(), NumProtoRefs, ProtoLocs.data(),
-Reader.getContext());
+Data.ReferencedProtocols.set(ProtoRefs.data(), NumProtoRefs,
+ ProtoLocs.data(), Reader.getContext());
+}
+
+void ASTDeclReader::MergeDefinitionData(ObjCProtocolDecl *D,
+ struct ObjCProtocolDecl::DefinitionData &) {
+ // FIXME: odr checking?
+}
+void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {
+ RedeclarableResult Redecl = VisitRedeclarable(PD);
+ VisitObjCContainerDecl(PD);
+ mergeRedeclarable(PD, Redecl);
+
+ if (Record.readInt()) {
+// Read the definition.
+PD->allocateDefinitionData();
+
+ReadObjCDefinitionData(PD->data());
+
+ObjCProtocolDecl *Canon = PD->getCanonicalDecl();
+if (Canon->Data.getPointer()) {
+ // If we already have a definition, keep the definition invariant and
+ // merge the data.
+ MergeDefinitionData(Canon, std::move(PD->data()));
+ PD->Data = Canon->Data;
+} else {
+ // Set the definition data of the canonical declaration, so other
+ // redeclarations will see it.
+ PD->getCanonicalDecl()->Data = PD->Data;
+}
// Note that we have deserialized a definition.
Reader.PendingDefinitions.insert(PD);
} else {
Index: cfe/trunk/include/clang/AST/Redeclarable.h
===
--- cfe/trunk/include/clang/AST/Redeclarable.h
+++ cfe/trunk/include/clang/AST/Redeclarable.h
@@ -21,6 +21,60 @@
namespace clang {
class ASTContext;
+// Some notes on redeclarables:
+//
+// - Every redeclarable is on a circular linked list.
+//
+// - Every decl has a pointer to the first element of the chain _and_ a
+//DeclLink that may point to one of 3 possible states:
+// - the "previous" (temporal) element in the chain
+// - the "latest" (temporal) element in the chain
+// - the an "uninitialized-latest" value (when newly-constructed)
+//
+// - The first element is also often called the canonical element. Every
+//element has a pointer to it so that "getCanonical" can be fast.
+//
+// - Most links in the chain point to previous, except the link out of
+//the first; it points to latest.
+//
+// - Elements are called "first", "previous", "latest" or
+//"most-recent" when referring to temporal order: order of addition
+//to the chain.
+//
+// - To make matters confusing, the DeclLink type uses the term "next"
+//for its pointer-storage internally (thus functions like
+//NextIsPrevious). It's easiest to just ignore the implementation of
+//DeclLink when making sense of the redeclaration chain.
+//
+// - There's also a "definition" link for several types of
+//redeclarable, where only one definition should exist at any given
+//time (and the defn pointer is stored in the decl's "data" which
+//is copied to every element on the chain when it's changed).
+//
+//Here is some ASCII art:
+//
+// "first"
[PATCH] D34741: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.
doug.gregor accepted this revision. doug.gregor added a comment. This revision is now accepted and ready to land. Looks great! https://reviews.llvm.org/D34741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34741: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.
graydon created this revision.
In change 2ba19793512, the ASTReader logic for ObjC interfaces was modified to
preserve the first definition-data read, "merging" later definitions into it
rather than overwriting it (though this "merging" is, in practice, a no-op that
discards the later definition-data).
Unfortunately this change was only made to ObjC interfaces, not protocols; this
means that when (for example) loading a protocol that references an interface,
if both the protocol and interface are multiply defined (as can easily happen
if the same header is read from multiple contexts), an _inconsistent_ pair of
definitions is loaded: first-read for the interface and last-read for the
protocol.
This in turn causes very subtle downstream bugs in the Swift ClangImporter,
which filters the results of name lookups based on the owning module of a
definition; inconsistency between a pair of related definitions causes name
lookup failures at various stages of compilation.
To fix these downstream issues, this change replicates the logic applied to
interfaces in change 2ba19793512, but for ObjC protocols.
rdar://30851899
https://reviews.llvm.org/D34741
Files:
include/clang/AST/Redeclarable.h
lib/Serialization/ASTReaderDecl.cpp
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -126,6 +126,9 @@
void ReadObjCDefinitionData(struct ObjCInterfaceDecl::DefinitionData );
void MergeDefinitionData(ObjCInterfaceDecl *D,
struct ObjCInterfaceDecl::DefinitionData &);
+void ReadObjCDefinitionData(struct ObjCProtocolDecl::DefinitionData );
+void MergeDefinitionData(ObjCProtocolDecl *D,
+ struct ObjCProtocolDecl::DefinitionData &);
static NamedDecl *getAnonymousDeclForMerging(ASTReader ,
DeclContext *DC,
@@ -1045,18 +1048,8 @@
IVD->setSynthesize(synth);
}
-void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {
- RedeclarableResult Redecl = VisitRedeclarable(PD);
- VisitObjCContainerDecl(PD);
- mergeRedeclarable(PD, Redecl);
-
- if (Record.readInt()) {
-// Read the definition.
-PD->allocateDefinitionData();
-
-// Set the definition data of the canonical declaration, so other
-// redeclarations will see it.
-PD->getCanonicalDecl()->Data = PD->Data;
+void ASTDeclReader::ReadObjCDefinitionData(
+ struct ObjCProtocolDecl::DefinitionData ) {
unsigned NumProtoRefs = Record.readInt();
SmallVector ProtoRefs;
@@ -1067,9 +1060,37 @@
ProtoLocs.reserve(NumProtoRefs);
for (unsigned I = 0; I != NumProtoRefs; ++I)
ProtoLocs.push_back(ReadSourceLocation());
-PD->setProtocolList(ProtoRefs.data(), NumProtoRefs, ProtoLocs.data(),
-Reader.getContext());
+Data.ReferencedProtocols.set(ProtoRefs.data(), NumProtoRefs,
+ ProtoLocs.data(), Reader.getContext());
+}
+
+void ASTDeclReader::MergeDefinitionData(ObjCProtocolDecl *D,
+ struct ObjCProtocolDecl::DefinitionData &) {
+ // FIXME: odr checking?
+}
+void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {
+ RedeclarableResult Redecl = VisitRedeclarable(PD);
+ VisitObjCContainerDecl(PD);
+ mergeRedeclarable(PD, Redecl);
+
+ if (Record.readInt()) {
+// Read the definition.
+PD->allocateDefinitionData();
+
+ReadObjCDefinitionData(PD->data());
+
+ObjCProtocolDecl *Canon = PD->getCanonicalDecl();
+if (Canon->Data.getPointer()) {
+ // If we already have a definition, keep the definition invariant and
+ // merge the data.
+ MergeDefinitionData(Canon, std::move(PD->data()));
+ PD->Data = Canon->Data;
+} else {
+ // Set the definition data of the canonical declaration, so other
+ // redeclarations will see it.
+ PD->getCanonicalDecl()->Data = PD->Data;
+}
// Note that we have deserialized a definition.
Reader.PendingDefinitions.insert(PD);
} else {
Index: include/clang/AST/Redeclarable.h
===
--- include/clang/AST/Redeclarable.h
+++ include/clang/AST/Redeclarable.h
@@ -21,6 +21,60 @@
namespace clang {
class ASTContext;
+// Some notes on redeclarables:
+//
+// - Every redeclarable is on a circular linked list.
+//
+// - Every decl has a pointer to the first element of the chain _and_ a
+//DeclLink that may point to one of 3 possible states:
+// - the "previous" (temporal) element in the chain
+// - the "latest" (temporal) element in the chain
+// - the an "uninitialized-latest" value (when newly-constructed)
+//
+// - The first element is also often called the canonical element. Every
+//element has a pointer to it so that "getCanonical" can be fast.
+//
+// - Most links in the chain point to
