[ODRHash] Improve handling of hash values
Calculating the hash in Sema::ActOnTagFinishDefinition could happen before
all sub-Decls were parsed or processed, which would produce the wrong hash
value. Change to calculating the hash on the first use and storing the value
instead. Also, avoid using the macros that were only for Boolean fields and
use an explicit checker during the DefintionData merge. No functional change,
but was this blocking other ODRHash patches.
llvm-svn: 299989
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index d663dbe..13921a1 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -464,6 +464,8 @@
/// \brief Whether we are currently parsing base specifiers.
unsigned IsParsingBaseSpecifiers : 1;
+ unsigned HasODRHash : 1;
+
/// \brief A hash of parts of the class to help in ODR checking.
unsigned ODRHash;
@@ -712,8 +714,7 @@
return data().IsParsingBaseSpecifiers;
}
- void computeODRHash();
- unsigned getODRHash() const { return data().ODRHash; }
+ unsigned getODRHash() const;
/// \brief Sets the base classes of this struct or class.
void setBases(CXXBaseSpecifier const * const *Bases, unsigned NumBases);
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index a1aed33..2e5cec9 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -73,8 +73,9 @@
ImplicitCopyAssignmentHasConstParam(true),
HasDeclaredCopyConstructorWithConstParam(false),
HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
- IsParsingBaseSpecifiers(false), ODRHash(0), NumBases(0), NumVBases(0),
- Bases(), VBases(), Definition(D), FirstFriend() {}
+ IsParsingBaseSpecifiers(false), HasODRHash(false), ODRHash(0),
+ NumBases(0), NumVBases(0), Bases(), VBases(), Definition(D),
+ FirstFriend() {}
CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
return Bases.get(Definition->getASTContext().getExternalSource());
@@ -381,16 +382,23 @@
data().IsParsingBaseSpecifiers = false;
}
-void CXXRecordDecl::computeODRHash() {
- if (!DefinitionData)
- return;
+unsigned CXXRecordDecl::getODRHash() const {
+ assert(hasDefinition() && "ODRHash only for records with definitions");
+ // Previously calculated hash is stored in DefinitionData.
+ if (DefinitionData->HasODRHash)
+ return DefinitionData->ODRHash;
+
+ // Only calculate hash on first call of getODRHash per record.
ODRHash Hash;
- Hash.AddCXXRecordDecl(this);
-
+ Hash.AddCXXRecordDecl(getDefinition());
+ DefinitionData->HasODRHash = true;
DefinitionData->ODRHash = Hash.CalculateHash();
+
+ return DefinitionData->ODRHash;
}
+
void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) {
// C++11 [class.copy]p11:
// A defaulted copy/move constructor for a class X is defined as
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 02cbca8..c6a0b01 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13797,10 +13797,8 @@
RD->completeDefinition();
}
- if (auto *RD = dyn_cast<CXXRecordDecl>(Tag)) {
+ if (isa<CXXRecordDecl>(Tag)) {
FieldCollector->FinishClass();
- if (Context.getLangOpts().Modules)
- RD->computeODRHash();
}
// Exit this scope of this tag's definition.
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index db7d55e..2fb882c 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1536,6 +1536,7 @@
Data.HasDeclaredCopyConstructorWithConstParam = Record.readInt();
Data.HasDeclaredCopyAssignmentWithConstParam = Record.readInt();
Data.ODRHash = Record.readInt();
+ Data.HasODRHash = true;
if (Record.readInt()) {
Reader.BodySource[D] = Loc.F->Kind == ModuleKind::MK_MainFile
@@ -1673,7 +1674,6 @@
OR_FIELD(HasDeclaredCopyConstructorWithConstParam)
OR_FIELD(HasDeclaredCopyAssignmentWithConstParam)
MATCH_FIELD(IsLambda)
- MATCH_FIELD(ODRHash)
#undef OR_FIELD
#undef MATCH_FIELD
@@ -1697,6 +1697,10 @@
// when they occur within the body of a function template specialization).
}
+ if (D->getODRHash() != MergeDD.ODRHash) {
+ DetectedOdrViolation = true;
+ }
+
if (DetectedOdrViolation)
Reader.PendingOdrMergeFailures[DD.Definition].push_back(MergeDD.Definition);
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index ec59250..6f0d0ee 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5769,7 +5769,10 @@
Record->push_back(Data.ImplicitCopyAssignmentHasConstParam);
Record->push_back(Data.HasDeclaredCopyConstructorWithConstParam);
Record->push_back(Data.HasDeclaredCopyAssignmentWithConstParam);
- Record->push_back(Data.ODRHash);
+
+ // getODRHash will compute the ODRHash if it has not been previously computed.
+ Record->push_back(D->getODRHash());
+
bool ModularCodegen = Writer->Context->getLangOpts().ModularCodegen &&
Writer->WritingModule && !D->isDependentType();
Record->push_back(ModularCodegen);