bruno updated this revision to Diff 236964.
bruno added a comment.
Change the approach: handle type merging for tag types using the ODRHash
mechanism
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71734/new/
https://reviews.llvm.org/D71734
Files:
clang/include/clang/AST/Decl.h
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/ODRHash.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclCXX.cpp
clang/lib/AST/ODRHash.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/Modules/odr_hash-record.c
Index: clang/test/Modules/odr_hash-record.c
===================================================================
--- /dev/null
+++ clang/test/Modules/odr_hash-record.c
@@ -0,0 +1,293 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/Inputs
+
+// Build first header file
+// RUN: echo "#define FIRST" >> %t/Inputs/first.h
+// RUN: cat %s >> %t/Inputs/first.h
+
+// Build second header file
+// RUN: echo "#define SECOND" >> %t/Inputs/second.h
+// RUN: cat %s >> %t/Inputs/second.h
+
+// Test that each header can compile
+// RUN: %clang_cc1 -fsyntax-only -x c %t/Inputs/first.h
+// RUN: %clang_cc1 -fsyntax-only -x c %t/Inputs/second.h
+
+// Build module map file
+// RUN: echo "module FirstModule {" >> %t/Inputs/module.map
+// RUN: echo " header \"first.h\"" >> %t/Inputs/module.map
+// RUN: echo "}" >> %t/Inputs/module.map
+// RUN: echo "module SecondModule {" >> %t/Inputs/module.map
+// RUN: echo " header \"second.h\"" >> %t/Inputs/module.map
+// RUN: echo "}" >> %t/Inputs/module.map
+
+// Run test
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -x c \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN: -I%t/Inputs -verify %s
+
+#if !defined(FIRST) && !defined(SECOND)
+#include "first.h"
+#include "second.h"
+#endif
+
+#if defined(FIRST)
+struct S1 {};
+struct S1 s1a;
+#elif defined(SECOND)
+struct S1 {};
+#else
+struct S1 s1;
+#endif
+
+#if defined(FIRST)
+struct S2 {
+ int x;
+ int y;
+};
+#elif defined(SECOND)
+struct S2 {
+ int y;
+ int x;
+};
+#else
+struct S2 s2;
+// expected-error@first.h:* {{'S2' has different definitions in different modules; first difference is definition in module 'FirstModule' found field 'x'}}
+// expected-note@second.h:* {{but in 'SecondModule' found field 'y'}}
+#endif
+
+#if defined(FIRST)
+struct S3 {
+ double x;
+};
+#elif defined(SECOND)
+struct S3 {
+ int x;
+};
+#else
+struct S3 s3;
+// expected-error@second.h:* {{'S3::x' from module 'SecondModule' is not present in definition of 'struct S3' in module 'FirstModule'}}
+// expected-note@first.h:* {{declaration of 'x' does not match}}
+#endif
+
+#if defined(FIRST)
+typedef int A;
+struct S4 {
+ A x;
+};
+
+struct S5 {
+ A x;
+};
+#elif defined(SECOND)
+typedef int B;
+struct S4 {
+ B x;
+};
+
+struct S5 {
+ int x;
+};
+#else
+struct S4 s4;
+// expected-error@first.h:* {{'S4' has different definitions in different modules; first difference is definition in module 'FirstModule' found field 'x' with type 'A' (aka 'int')}}
+// expected-note@second.h:* {{but in 'SecondModule' found field 'x' with type 'B' (aka 'int')}}
+
+struct S5 s5;
+// expected-error@first.h:* {{'S5' has different definitions in different modules; first difference is definition in module 'FirstModule' found field 'x' with type 'A' (aka 'int')}}
+// expected-note@second.h:* {{but in 'SecondModule' found field 'x' with type 'int'}}
+#endif
+
+#if defined(FIRST)
+struct S6 {
+ unsigned x;
+};
+#elif defined(SECOND)
+struct S6 {
+ unsigned x : 1;
+};
+#else
+struct S6 s6;
+// expected-error@first.h:* {{'S6' has different definitions in different modules; first difference is definition in module 'FirstModule' found non-bitfield 'x'}}
+// expected-note@second.h:* {{but in 'SecondModule' found bitfield 'x'}}
+#endif
+
+#if defined(FIRST)
+struct S7 {
+ unsigned x : 2;
+};
+#elif defined(SECOND)
+struct S7 {
+ unsigned x : 1;
+};
+#else
+struct S7 s7;
+// expected-error@first.h:* {{'S7' has different definitions in different modules; first difference is definition in module 'FirstModule' found bitfield 'x' with one width expression}}
+// expected-note@second.h:* {{but in 'SecondModule' found bitfield 'x' with different width expression}}
+#endif
+
+#if defined(FIRST)
+struct S8 {
+ unsigned x : 2;
+};
+#elif defined(SECOND)
+struct S8 {
+ unsigned x : 1 + 1;
+};
+#else
+struct S8 s8;
+// expected-error@first.h:* {{'S8' has different definitions in different modules; first difference is definition in module 'FirstModule' found bitfield 'x' with one width expression}}
+// expected-note@second.h:* {{but in 'SecondModule' found bitfield 'x' with different width expression}}
+#endif
+
+#if defined(FIRST)
+struct S12 {
+ unsigned x[5];
+};
+#elif defined(SECOND)
+struct S12 {
+ unsigned x[7];
+};
+#else
+struct S12 s12;
+// expected-error@second.h:* {{'S12::x' from module 'SecondModule' is not present in definition of 'struct S12' in module 'FirstModule'}}
+// expected-note@first.h:* {{declaration of 'x' does not match}}
+#endif
+
+#if defined(FIRST)
+struct S13 {
+ unsigned x[7];
+};
+#elif defined(SECOND)
+struct S13 {
+ double x[7];
+};
+#else
+struct S13 s13;
+// expected-error@second.h:* {{'S13::x' from module 'SecondModule' is not present in definition of 'struct S13' in module 'FirstModule'}}
+// expected-note@first.h:* {{declaration of 'x' does not match}}
+#endif
+
+#if defined(FIRST)
+struct B1 {};
+struct SS1 {
+ struct B1 x;
+};
+#elif defined(SECOND)
+struct A1 {};
+struct SS1 {
+ struct A1 x;
+};
+#else
+struct SS1 ss1;
+// expected-error@second.h:* {{'SS1::x' from module 'SecondModule' is not present in definition of 'struct SS1' in module 'FirstModule'}}
+// expected-note@first.h:* {{declaration of 'x' does not match}}
+#endif
+
+#if defined(FIRST)
+enum E1 { x42 };
+struct SE1 {
+ enum E1 x;
+};
+#elif defined(SECOND)
+enum E2 { x42 };
+struct SE1 {
+ enum E2 x;
+};
+#else
+struct SE1 se1;
+// expected-error@second.h:* {{'SE1::x' from module 'SecondModule' is not present in definition of 'struct SE1' in module 'FirstModule'}}
+// expected-note@first.h:* {{declaration of 'x' does not match}}
+#endif
+
+// struct with forward declaration
+#if defined(FIRST)
+struct P {};
+struct S {
+ struct P *ptr;
+};
+#elif defined(SECOND)
+struct S {
+ struct P *ptr;
+};
+#else
+struct S s;
+#endif
+
+// struct with forward declaration and no definition
+#if defined(FIRST)
+struct PA;
+struct SA {
+ struct PA *ptr;
+};
+#elif defined(SECOND)
+struct SA {
+ struct PA *ptr;
+};
+#else
+struct SA sa;
+#endif
+
+// struct with multiple typedefs
+#if defined(FIRST)
+typedef int BB1;
+typedef BB1 AA1;
+struct TS1 {
+ AA1 x;
+};
+#elif defined(SECOND)
+typedef int AA1;
+struct TS1 {
+ AA1 x;
+};
+#else
+struct TS1 ts1;
+#endif
+
+#if defined(FIRST)
+struct T2 {
+ int x;
+};
+typedef struct T2 B2;
+typedef struct B2 A2;
+struct TS2 {
+ struct T2 x;
+};
+#elif defined(SECOND)
+struct T2 {
+ int x;
+};
+typedef struct T2 A2;
+struct TS2 {
+ struct T2 x;
+};
+#else
+struct TS2 ts2;
+#endif
+
+#if defined(FIRST)
+struct T3;
+struct TS3 {
+ struct T3 *t;
+};
+#elif defined(SECOND)
+typedef struct T3 {
+} T3;
+struct TS3 {
+ struct T3 *t;
+};
+#else
+struct TS3 ts3;
+#endif
+
+// Keep macros contained to one file.
+#ifdef FIRST
+#undef FIRST
+#endif
+
+#ifdef SECOND
+#undef SECOND
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -483,6 +483,9 @@
Record.push_back(D->hasNonTrivialToPrimitiveCopyCUnion());
Record.push_back(D->isParamDestroyedInCallee());
Record.push_back(D->getArgPassingRestrictions());
+ // Only compute this for C/Objective-C, in C++ this is computed as part
+ // of CXXRecordDecl.
+ Record.push_back(Writer.getLangOpts().CPlusPlus ? 0UL : D->getODRHash());
if (D->getDeclContext() == D->getLexicalDeclContext() &&
!D->hasAttrs() &&
@@ -2046,6 +2049,8 @@
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1));
// getArgPassingRestrictions
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2));
+ // ODRHash
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
// DC
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // LexicalOffset
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -723,8 +723,11 @@
llvm_unreachable("unexpected tag info kind");
}
- if (!isa<CXXRecordDecl>(TD))
- mergeRedeclarable(TD, Redecl);
+ if (isa<CXXRecordDecl>(TD))
+ return Redecl;
+
+ // Handle merging in C and Objective-C
+ mergeRedeclarable(TD, Redecl);
return Redecl;
}
@@ -794,6 +797,22 @@
RD->setHasNonTrivialToPrimitiveCopyCUnion(Record.readInt());
RD->setParamDestroyedInCallee(Record.readInt());
RD->setArgPassingRestrictions((RecordDecl::ArgPassingKind)Record.readInt());
+ RD->setHasODRHash(true);
+ RD->ODRHash = Record.readInt();
+
+ // C++ applies ODR checking in VisitCXXRecordDecl instead. Note that
+ // structural equivalence is the usual way to check for ODR-like semantics
+ // in ObjC/C, but using ODRHash is prefered if possible because of better
+ // performance.
+ if (!Reader.getContext().getLangOpts().CPlusPlus) {
+ RecordDecl *Canon = static_cast<RecordDecl *>(RD->getCanonicalDecl());
+ if (RD == Canon || Canon->getODRHash() == RD->getODRHash())
+ return Redecl;
+ Reader.PendingRecordOdrMergeFailures[Canon].push_back(RD);
+ // Track that we merged the definitions.
+ Reader.MergedDeclContexts.insert(std::make_pair(RD, Canon));
+ }
+
return Redecl;
}
@@ -2561,7 +2580,7 @@
if (!ND)
return false;
// TODO: implement merge for other necessary decls.
- if (isa<EnumConstantDecl>(ND))
+ if (isa<EnumConstantDecl>(ND) || isa<FieldDecl>(ND))
return true;
return false;
}
@@ -3197,6 +3216,10 @@
return ED->getASTContext().getLangOpts().CPlusPlus? ED->getDefinition()
: nullptr;
+ if (auto *RD = dyn_cast<RecordDecl>(DC))
+ if (!RD->getASTContext().getLangOpts().CPlusPlus)
+ return RD->getCanonicalDecl()->getDefinition();
+
// We can see the TU here only if we have no Sema object. In that case,
// there's no TU scope to look in, so using the DC alone is sufficient.
if (auto *TU = dyn_cast<TranslationUnitDecl>(DC))
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -9267,7 +9267,8 @@
void ASTReader::diagnoseOdrViolations() {
if (PendingOdrMergeFailures.empty() && PendingOdrMergeChecks.empty() &&
PendingFunctionOdrMergeFailures.empty() &&
- PendingEnumOdrMergeFailures.empty())
+ PendingEnumOdrMergeFailures.empty() &&
+ PendingRecordOdrMergeFailures.empty())
return;
// Trigger the import of the full definition of each class that had any
@@ -9289,6 +9290,15 @@
}
}
+ // Trigger the import of the full definition of each record in C/ObjC.
+ auto RecordOdrMergeFailures = std::move(PendingRecordOdrMergeFailures);
+ PendingRecordOdrMergeFailures.clear();
+ for (auto &Merge : RecordOdrMergeFailures) {
+ Merge.first->decls_begin();
+ for (auto &D : Merge.second)
+ D->decls_begin();
+ }
+
// Trigger the import of functions.
auto FunctionOdrMergeFailures = std::move(PendingFunctionOdrMergeFailures);
PendingFunctionOdrMergeFailures.clear();
@@ -9331,6 +9341,11 @@
DeclContext *CanonDef = D->getDeclContext();
+ // Skip ODR checking for structs without a definition for C/ObjC mode.
+ if (RecordDecl *RD = dyn_cast<RecordDecl>(CanonDef))
+ if (!RD->isCompleteDefinition())
+ continue;
+
bool Found = false;
const Decl *DCanon = D->getCanonicalDecl();
@@ -9396,7 +9411,7 @@
}
if (OdrMergeFailures.empty() && FunctionOdrMergeFailures.empty() &&
- EnumOdrMergeFailures.empty())
+ EnumOdrMergeFailures.empty() && RecordOdrMergeFailures.empty())
return;
// Ensure we don't accidentally recursively enter deserialization while
@@ -9439,6 +9454,113 @@
return Hash.CalculateHash();
};
+ // Used with err_module_odr_violation_mismatch_decl and
+ // note_module_odr_violation_mismatch_decl
+ // This list should be the same Decl's as in ODRHash::isWhiteListedDecl
+ enum RecordDiffType {
+ EndOfClass,
+ PublicSpecifer,
+ PrivateSpecifer,
+ ProtectedSpecifer,
+ StaticAssert,
+ Field,
+ CXXMethod,
+ TypeAlias,
+ TypeDef,
+ Var,
+ Friend,
+ FunctionTemplate,
+ Other
+ };
+
+ // Used with err_module_odr_violation_mismatch_decl_diff and
+ // note_module_odr_violation_mismatch_decl_diff
+ enum ODRDeclDifference {
+ StaticAssertCondition,
+ StaticAssertMessage,
+ StaticAssertOnlyMessage,
+ FieldName,
+ FieldTypeName,
+ FieldSingleBitField,
+ FieldDifferentWidthBitField,
+ FieldSingleMutable,
+ FieldSingleInitializer,
+ FieldDifferentInitializers,
+ MethodName,
+ MethodDeleted,
+ MethodDefaulted,
+ MethodVirtual,
+ MethodStatic,
+ MethodVolatile,
+ MethodConst,
+ MethodInline,
+ MethodNumberParameters,
+ MethodParameterType,
+ MethodParameterName,
+ MethodParameterSingleDefaultArgument,
+ MethodParameterDifferentDefaultArgument,
+ MethodNoTemplateArguments,
+ MethodDifferentNumberTemplateArguments,
+ MethodDifferentTemplateArgument,
+ MethodSingleBody,
+ MethodDifferentBody,
+ TypedefName,
+ TypedefType,
+ VarName,
+ VarType,
+ VarSingleInitializer,
+ VarDifferentInitializer,
+ VarConstexpr,
+ FriendTypeFunction,
+ FriendType,
+ FriendFunction,
+ FunctionTemplateDifferentNumberParameters,
+ FunctionTemplateParameterDifferentKind,
+ FunctionTemplateParameterName,
+ FunctionTemplateParameterSingleDefaultArgument,
+ FunctionTemplateParameterDifferentDefaultArgument,
+ FunctionTemplateParameterDifferentType,
+ FunctionTemplatePackParameter,
+ };
+
+ auto DifferenceSelector = [](Decl *D) {
+ assert(D && "valid Decl required");
+ switch (D->getKind()) {
+ default:
+ return Other;
+ case Decl::AccessSpec:
+ switch (D->getAccess()) {
+ case AS_public:
+ return PublicSpecifer;
+ case AS_private:
+ return PrivateSpecifer;
+ case AS_protected:
+ return ProtectedSpecifer;
+ case AS_none:
+ break;
+ }
+ llvm_unreachable("Invalid access specifier");
+ case Decl::StaticAssert:
+ return StaticAssert;
+ case Decl::Field:
+ return Field;
+ case Decl::CXXMethod:
+ case Decl::CXXConstructor:
+ case Decl::CXXDestructor:
+ return CXXMethod;
+ case Decl::TypeAlias:
+ return TypeAlias;
+ case Decl::Typedef:
+ return TypeDef;
+ case Decl::Var:
+ return Var;
+ case Decl::Friend:
+ return Friend;
+ case Decl::FunctionTemplate:
+ return FunctionTemplate;
+ }
+ };
+
// Issue any pending ODR-failure diagnostics.
for (auto &Merge : OdrMergeFailures) {
// If we've already pointed out a specific problem with this class, don't
@@ -9776,64 +9898,7 @@
PopulateHashes(FirstHashes, FirstRecord);
PopulateHashes(SecondHashes, SecondRecord);
- // Used with err_module_odr_violation_mismatch_decl and
- // note_module_odr_violation_mismatch_decl
- // This list should be the same Decl's as in ODRHash::isWhiteListedDecl
- enum {
- EndOfClass,
- PublicSpecifer,
- PrivateSpecifer,
- ProtectedSpecifer,
- StaticAssert,
- Field,
- CXXMethod,
- TypeAlias,
- TypeDef,
- Var,
- Friend,
- FunctionTemplate,
- Other
- } FirstDiffType = Other,
- SecondDiffType = Other;
-
- auto DifferenceSelector = [](Decl *D) {
- assert(D && "valid Decl required");
- switch (D->getKind()) {
- default:
- return Other;
- case Decl::AccessSpec:
- switch (D->getAccess()) {
- case AS_public:
- return PublicSpecifer;
- case AS_private:
- return PrivateSpecifer;
- case AS_protected:
- return ProtectedSpecifer;
- case AS_none:
- break;
- }
- llvm_unreachable("Invalid access specifier");
- case Decl::StaticAssert:
- return StaticAssert;
- case Decl::Field:
- return Field;
- case Decl::CXXMethod:
- case Decl::CXXConstructor:
- case Decl::CXXDestructor:
- return CXXMethod;
- case Decl::TypeAlias:
- return TypeAlias;
- case Decl::Typedef:
- return TypeDef;
- case Decl::Var:
- return Var;
- case Decl::Friend:
- return Friend;
- case Decl::FunctionTemplate:
- return FunctionTemplate;
- }
- };
-
+ RecordDiffType FirstDiffType = Other, SecondDiffType = Other;
Decl *FirstDecl = nullptr;
Decl *SecondDecl = nullptr;
auto FirstIt = FirstHashes.begin();
@@ -9915,56 +9980,6 @@
assert(FirstDiffType == SecondDiffType);
- // Used with err_module_odr_violation_mismatch_decl_diff and
- // note_module_odr_violation_mismatch_decl_diff
- enum ODRDeclDifference {
- StaticAssertCondition,
- StaticAssertMessage,
- StaticAssertOnlyMessage,
- FieldName,
- FieldTypeName,
- FieldSingleBitField,
- FieldDifferentWidthBitField,
- FieldSingleMutable,
- FieldSingleInitializer,
- FieldDifferentInitializers,
- MethodName,
- MethodDeleted,
- MethodDefaulted,
- MethodVirtual,
- MethodStatic,
- MethodVolatile,
- MethodConst,
- MethodInline,
- MethodNumberParameters,
- MethodParameterType,
- MethodParameterName,
- MethodParameterSingleDefaultArgument,
- MethodParameterDifferentDefaultArgument,
- MethodNoTemplateArguments,
- MethodDifferentNumberTemplateArguments,
- MethodDifferentTemplateArgument,
- MethodSingleBody,
- MethodDifferentBody,
- TypedefName,
- TypedefType,
- VarName,
- VarType,
- VarSingleInitializer,
- VarDifferentInitializer,
- VarConstexpr,
- FriendTypeFunction,
- FriendType,
- FriendFunction,
- FunctionTemplateDifferentNumberParameters,
- FunctionTemplateParameterDifferentKind,
- FunctionTemplateParameterName,
- FunctionTemplateParameterSingleDefaultArgument,
- FunctionTemplateParameterDifferentDefaultArgument,
- FunctionTemplateParameterDifferentType,
- FunctionTemplatePackParameter,
- };
-
// These lambdas have the common portions of the ODR diagnostics. This
// has the same return as Diag(), so addition parameters can be passed
// in with operator<<
@@ -10989,6 +11004,296 @@
}
}
+ // Issue any pending ODR-failure diagnostics.
+ for (auto &Merge : RecordOdrMergeFailures) {
+ // If we've already pointed out a specific problem with this class, don't
+ // bother issuing a general "something's different" diagnostic.
+ if (!DiagnosedOdrMergeFailures.insert(Merge.first).second)
+ continue;
+
+ bool Diagnosed = false;
+ RecordDecl *FirstRecord = Merge.first;
+ if (!FirstRecord->isCompleteDefinition())
+ continue;
+
+ std::string FirstModule = getOwningModuleNameForDiagnostic(FirstRecord);
+ for (auto *SecondRecord : Merge.second) {
+ // Multiple different declarations got merged together; tell the user
+ // where they came from.
+ if (FirstRecord == SecondRecord)
+ continue;
+ if (!SecondRecord->isCompleteDefinition())
+ continue;
+
+ std::string SecondModule = getOwningModuleNameForDiagnostic(SecondRecord);
+ using DeclHashes = llvm::SmallVector<std::pair<Decl *, unsigned>, 4>;
+ DeclHashes FirstHashes;
+ DeclHashes SecondHashes;
+
+ auto PopulateHashes = [&ComputeSubDeclODRHash, FirstRecord](
+ DeclHashes &Hashes, RecordDecl *Record) {
+ for (auto *D : Record->decls()) {
+ // FIXME: is this really needed? RecordDecl doesnt have an extra
+ // layer like CXXRecordDecl does.
+ if (!ODRHash::isWhitelistedDecl(D, FirstRecord))
+ continue;
+ Hashes.emplace_back(D, ComputeSubDeclODRHash(D));
+ }
+ };
+ PopulateHashes(FirstHashes, FirstRecord);
+ PopulateHashes(SecondHashes, SecondRecord);
+
+ RecordDiffType FirstDiffType = Other, SecondDiffType = Other;
+ Decl *FirstDecl = nullptr;
+ Decl *SecondDecl = nullptr;
+ auto FirstIt = FirstHashes.begin();
+ auto SecondIt = SecondHashes.begin();
+
+ // If there is a diagnoseable difference, FirstDiffType and
+ // SecondDiffType will not be Other and FirstDecl and SecondDecl will be
+ // filled in if not EndOfClass.
+ while (FirstIt != FirstHashes.end() || SecondIt != SecondHashes.end()) {
+ if (FirstIt != FirstHashes.end() && SecondIt != SecondHashes.end() &&
+ FirstIt->second == SecondIt->second) {
+ ++FirstIt;
+ ++SecondIt;
+ continue;
+ }
+
+ FirstDecl = FirstIt == FirstHashes.end() ? nullptr : FirstIt->first;
+ SecondDecl = SecondIt == SecondHashes.end() ? nullptr : SecondIt->first;
+
+ FirstDiffType = FirstDecl ? DifferenceSelector(FirstDecl) : EndOfClass;
+ SecondDiffType =
+ SecondDecl ? DifferenceSelector(SecondDecl) : EndOfClass;
+
+ break;
+ }
+
+ if (FirstDiffType == Other || SecondDiffType == Other) {
+ // Reaching this point means an unexpected Decl was encountered
+ // or no difference was detected. This causes a generic error
+ // message to be emitted.
+ Diag(FirstRecord->getLocation(),
+ diag::err_module_odr_violation_different_definitions)
+ << FirstRecord << FirstModule.empty() << FirstModule;
+
+ if (FirstDecl) {
+ Diag(FirstDecl->getLocation(), diag::note_first_module_difference)
+ << FirstRecord << FirstDecl->getSourceRange();
+ }
+
+ Diag(SecondRecord->getLocation(),
+ diag::note_module_odr_violation_different_definitions)
+ << SecondModule;
+
+ if (SecondDecl) {
+ Diag(SecondDecl->getLocation(), diag::note_second_module_difference)
+ << SecondDecl->getSourceRange();
+ }
+
+ Diagnosed = true;
+ break;
+ }
+
+ if (FirstDiffType != SecondDiffType) {
+ SourceLocation FirstLoc;
+ SourceRange FirstRange;
+ if (FirstDiffType == EndOfClass) {
+ FirstLoc = FirstRecord->getBraceRange().getEnd();
+ } else {
+ FirstLoc = FirstIt->first->getLocation();
+ FirstRange = FirstIt->first->getSourceRange();
+ }
+ Diag(FirstLoc, diag::err_module_odr_violation_mismatch_decl)
+ << FirstRecord << FirstModule.empty() << FirstModule << FirstRange
+ << FirstDiffType;
+
+ SourceLocation SecondLoc;
+ SourceRange SecondRange;
+ if (SecondDiffType == EndOfClass) {
+ SecondLoc = SecondRecord->getBraceRange().getEnd();
+ } else {
+ SecondLoc = SecondDecl->getLocation();
+ SecondRange = SecondDecl->getSourceRange();
+ }
+ Diag(SecondLoc, diag::note_module_odr_violation_mismatch_decl)
+ << SecondModule << SecondRange << SecondDiffType;
+ Diagnosed = true;
+ break;
+ }
+
+ assert(FirstDiffType == SecondDiffType);
+
+ // These lambdas have the common portions of the ODR diagnostics. This
+ // has the same return as Diag(), so addition parameters can be passed
+ // in with operator<<
+ auto ODRDiagError = [FirstRecord, &FirstModule,
+ this](SourceLocation Loc, SourceRange Range,
+ ODRDeclDifference DiffType) {
+ return Diag(Loc, diag::err_module_odr_violation_mismatch_decl_diff)
+ << FirstRecord << FirstModule.empty() << FirstModule << Range
+ << DiffType;
+ };
+ auto ODRDiagNote = [&SecondModule, this](SourceLocation Loc,
+ SourceRange Range,
+ ODRDeclDifference DiffType) {
+ return Diag(Loc, diag::note_module_odr_violation_mismatch_decl_diff)
+ << SecondModule << Range << DiffType;
+ };
+
+ switch (FirstDiffType) {
+ case EndOfClass:
+ case Other:
+ // C++ only, invalid in this context.
+ case PublicSpecifer:
+ case PrivateSpecifer:
+ case ProtectedSpecifer:
+ case StaticAssert:
+ case CXXMethod:
+ case TypeAlias:
+ case Friend:
+ case FunctionTemplate:
+ llvm_unreachable("Invalid diff type");
+
+ case Field: {
+ FieldDecl *FirstField = cast<FieldDecl>(FirstDecl);
+ FieldDecl *SecondField = cast<FieldDecl>(SecondDecl);
+ IdentifierInfo *FirstII = FirstField->getIdentifier();
+ IdentifierInfo *SecondII = SecondField->getIdentifier();
+ if (FirstII->getName() != SecondII->getName()) {
+ ODRDiagError(FirstField->getLocation(), FirstField->getSourceRange(),
+ FieldName)
+ << FirstII;
+ ODRDiagNote(SecondField->getLocation(), SecondField->getSourceRange(),
+ FieldName)
+ << SecondII;
+
+ Diagnosed = true;
+ break;
+ }
+
+ assert(getContext().hasSameType(FirstField->getType(),
+ SecondField->getType()));
+
+ QualType FirstType = FirstField->getType();
+ QualType SecondType = SecondField->getType();
+ if (ComputeQualTypeODRHash(FirstType) !=
+ ComputeQualTypeODRHash(SecondType)) {
+ ODRDiagError(FirstField->getLocation(), FirstField->getSourceRange(),
+ FieldTypeName)
+ << FirstII << FirstType;
+ ODRDiagNote(SecondField->getLocation(), SecondField->getSourceRange(),
+ FieldTypeName)
+ << SecondII << SecondType;
+
+ Diagnosed = true;
+ break;
+ }
+
+ const bool IsFirstBitField = FirstField->isBitField();
+ const bool IsSecondBitField = SecondField->isBitField();
+ if (IsFirstBitField != IsSecondBitField) {
+ ODRDiagError(FirstField->getLocation(), FirstField->getSourceRange(),
+ FieldSingleBitField)
+ << FirstII << IsFirstBitField;
+ ODRDiagNote(SecondField->getLocation(), SecondField->getSourceRange(),
+ FieldSingleBitField)
+ << SecondII << IsSecondBitField;
+ Diagnosed = true;
+ break;
+ }
+
+ if (IsFirstBitField && IsSecondBitField) {
+ ODRDiagError(FirstField->getLocation(), FirstField->getSourceRange(),
+ FieldDifferentWidthBitField)
+ << FirstII << FirstField->getBitWidth()->getSourceRange();
+ ODRDiagNote(SecondField->getLocation(), SecondField->getSourceRange(),
+ FieldDifferentWidthBitField)
+ << SecondII << SecondField->getBitWidth()->getSourceRange();
+ Diagnosed = true;
+ break;
+ }
+ break;
+ }
+ case TypeDef: {
+ TypedefNameDecl *FirstTD = cast<TypedefNameDecl>(FirstDecl);
+ TypedefNameDecl *SecondTD = cast<TypedefNameDecl>(SecondDecl);
+ auto FirstName = FirstTD->getDeclName();
+ auto SecondName = SecondTD->getDeclName();
+ if (FirstName != SecondName) {
+ ODRDiagError(FirstTD->getLocation(), FirstTD->getSourceRange(),
+ TypedefName)
+ << (FirstDiffType == TypeAlias) << FirstName;
+ ODRDiagNote(SecondTD->getLocation(), SecondTD->getSourceRange(),
+ TypedefName)
+ << (FirstDiffType == TypeAlias) << SecondName;
+ Diagnosed = true;
+ break;
+ }
+
+ QualType FirstType = FirstTD->getUnderlyingType();
+ QualType SecondType = SecondTD->getUnderlyingType();
+ if (ComputeQualTypeODRHash(FirstType) !=
+ ComputeQualTypeODRHash(SecondType)) {
+ ODRDiagError(FirstTD->getLocation(), FirstTD->getSourceRange(),
+ TypedefType)
+ << (FirstDiffType == TypeAlias) << FirstName << FirstType;
+ ODRDiagNote(SecondTD->getLocation(), SecondTD->getSourceRange(),
+ TypedefType)
+ << (FirstDiffType == TypeAlias) << SecondName << SecondType;
+ Diagnosed = true;
+ break;
+ }
+ break;
+ }
+ case Var: {
+ VarDecl *FirstVD = cast<VarDecl>(FirstDecl);
+ VarDecl *SecondVD = cast<VarDecl>(SecondDecl);
+ auto FirstName = FirstVD->getDeclName();
+ auto SecondName = SecondVD->getDeclName();
+ if (FirstName != SecondName) {
+ ODRDiagError(FirstVD->getLocation(), FirstVD->getSourceRange(),
+ VarName)
+ << FirstName;
+ ODRDiagNote(SecondVD->getLocation(), SecondVD->getSourceRange(),
+ VarName)
+ << SecondName;
+ Diagnosed = true;
+ break;
+ }
+
+ QualType FirstType = FirstVD->getType();
+ QualType SecondType = SecondVD->getType();
+ if (ComputeQualTypeODRHash(FirstType) !=
+ ComputeQualTypeODRHash(SecondType)) {
+ ODRDiagError(FirstVD->getLocation(), FirstVD->getSourceRange(),
+ VarType)
+ << FirstName << FirstType;
+ ODRDiagNote(SecondVD->getLocation(), SecondVD->getSourceRange(),
+ VarType)
+ << SecondName << SecondType;
+ Diagnosed = true;
+ break;
+ }
+ break;
+ }
+ }
+
+ if (Diagnosed)
+ continue;
+
+ Diag(FirstDecl->getLocation(),
+ diag::err_module_odr_violation_mismatch_decl_unknown)
+ << FirstRecord << FirstModule.empty() << FirstModule << FirstDiffType
+ << FirstDecl->getSourceRange();
+ Diag(SecondDecl->getLocation(),
+ diag::note_module_odr_violation_mismatch_decl_unknown)
+ << SecondModule << FirstDiffType << SecondDecl->getSourceRange();
+ Diagnosed = true;
+ }
+ }
+
// Issue ODR failures diagnostics for functions.
for (auto &Merge : FunctionOdrMergeFailures) {
enum ODRFunctionDifference {
Index: clang/lib/AST/ODRHash.cpp
===================================================================
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -464,6 +464,21 @@
ODRDeclVisitor(ID, *this).Visit(D);
}
+void ODRHash::AddRecordDecl(const RecordDecl *Record) {
+ AddDecl(Record);
+
+ // Filter out sub-Decls which will not be processed in order to get an
+ // accurate count of Decl's.
+ llvm::SmallVector<const Decl *, 16> Decls;
+ for (Decl *SubDecl : Record->decls())
+ if (isWhitelistedDecl(SubDecl, Record))
+ Decls.push_back(SubDecl);
+
+ ID.AddInteger(Decls.size());
+ for (auto SubDecl : Decls)
+ AddSubDecl(SubDecl);
+}
+
void ODRHash::AddCXXRecordDecl(const CXXRecordDecl *Record) {
assert(Record && Record->hasDefinition() &&
"Expected non-null record to be a definition.");
Index: clang/lib/AST/DeclCXX.cpp
===================================================================
--- clang/lib/AST/DeclCXX.cpp
+++ clang/lib/AST/DeclCXX.cpp
@@ -484,7 +484,7 @@
return DefinitionData->ODRHash;
// Only calculate hash on first call of getODRHash per record.
- ODRHash Hash;
+ class ODRHash Hash;
Hash.AddCXXRecordDecl(getDefinition());
DefinitionData->HasODRHash = true;
DefinitionData->ODRHash = Hash.CalculateHash();
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4360,6 +4360,8 @@
setHasNonTrivialToPrimitiveCopyCUnion(false);
setParamDestroyedInCallee(false);
setArgPassingRestrictions(APK_CanPassInRegs);
+ setHasODRHash(false);
+ ODRHash = 0;
}
RecordDecl *RecordDecl::Create(const ASTContext &C, TagKind TK, DeclContext *DC,
@@ -4507,6 +4509,19 @@
return nullptr;
}
+unsigned RecordDecl::getODRHash() {
+ if (hasODRHash())
+ return ODRHash;
+
+ // Only calculate hash on first call of getODRHash per record.
+ class ODRHash Hash;
+ Hash.AddRecordDecl(this);
+ setHasODRHash();
+ ODRHash = Hash.CalculateHash();
+
+ return ODRHash;
+}
+
//===----------------------------------------------------------------------===//
// BlockDecl Implementation
//===----------------------------------------------------------------------===//
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -1107,6 +1107,10 @@
llvm::SmallDenseMap<EnumDecl *, llvm::SmallVector<EnumDecl *, 2>, 2>
PendingEnumOdrMergeFailures;
+ /// C/ObjC definitions in which the structural equivalence check fails
+ llvm::SmallDenseMap<RecordDecl *, llvm::SmallVector<RecordDecl *, 2>, 2>
+ PendingRecordOdrMergeFailures;
+
/// DeclContexts in which we have diagnosed an ODR violation.
llvm::SmallPtrSet<DeclContext*, 2> DiagnosedOdrMergeFailures;
Index: clang/include/clang/AST/ODRHash.h
===================================================================
--- clang/include/clang/AST/ODRHash.h
+++ clang/include/clang/AST/ODRHash.h
@@ -55,6 +55,10 @@
// more information than the AddDecl class.
void AddCXXRecordDecl(const CXXRecordDecl *Record);
+ // Use this for ODR checking records in C/Objective-C between modules. This
+ // method compares more information than the AddDecl class.
+ void AddRecordDecl(const RecordDecl *Record);
+
// Use this for ODR checking functions between modules. This method compares
// more information than the AddDecl class. SkipBody will process the
// hash as if the function has no body.
Index: clang/include/clang/AST/DeclBase.h
===================================================================
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -1452,10 +1452,13 @@
/// Represents the way this type is passed to a function.
uint64_t ArgPassingRestrictions : 2;
+
+ /// True if a valid hash is stored in ODRHash.
+ uint64_t HasODRHash : 1;
};
/// Number of non-inherited bits in RecordDeclBitfields.
- enum { NumRecordDeclBits = 14 };
+ enum { NumRecordDeclBits = 15 };
/// Stores the bits used by OMPDeclareReductionDecl.
/// If modified NumOMPDeclareReductionDeclBits and the accessor
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3716,6 +3716,7 @@
// to save some space. Use the provided accessors to access it.
public:
friend class DeclContext;
+ friend class ASTDeclReader;
/// Enum that represents the different ways arguments are passed to and
/// returned from function calls. This takes into account the target-specific
/// and version-specific rules along with the rules determined by the
@@ -3960,9 +3961,19 @@
/// nullptr is returned if no named data member exists.
const FieldDecl *findFirstNamedDataMember() const;
+ /// Get precomputed ODRHash or add a new one.
+ unsigned getODRHash();
+
private:
/// Deserialize just the fields.
void LoadFieldsFromExternalStorage() const;
+
+ /// True if a valid hash is stored in ODRHash.
+ bool hasODRHash() const { return RecordDeclBits.HasODRHash; }
+ void setHasODRHash(bool Hash = true) { RecordDeclBits.HasODRHash = Hash; }
+
+ /// Store the ODR hash for this decl.
+ unsigned ODRHash;
};
class FileScopeAsmDecl : public Decl {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
