[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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@299989 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index d663dbeea870f748d4cbfadb73b9c4a252ff844d..13921a132cfb8043d3cb1db242cb1058dbe98a5c 100644 (file)
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -464,6 +464,8 @@ class CXXRecordDecl : public RecordDecl {
     /// \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 @@ public:
     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/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index a1aed332a13db88055fbc9aa10393f08168302f1..2e5cec9c108f1a23f1678961987a075b0344025a 100644 (file)
--- a/lib/AST/DeclCXX.cpp
+++ b/lib/AST/DeclCXX.cpp
@@ -73,8 +73,9 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
       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 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
   data().IsParsingBaseSpecifiers = false;
 }
 
-void CXXRecordDecl::computeODRHash() {
-  if (!DefinitionData)
-    return;
+unsigned CXXRecordDecl::getODRHash() const {
+  assert(hasDefinition() && "ODRHash only for records with definitions");
 
-  ODRHash Hash;
-  Hash.AddCXXRecordDecl(this);
+  // 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(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/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 02cbca8b2bbbac0b329398eae40e773237791e5e..c6a0b0101d374465a2da763c7c133a0d1954d3c9 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -13797,10 +13797,8 @@ void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
       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/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index db7d55ec0b877c395b6b84d2c82a284d68fe0fc0..2fb882c1bec42d3e6eec5aedaf5c32f2d4f707ad 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -1536,6 +1536,7 @@ void ASTDeclReader::ReadCXXDefinitionData(
   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 @@ void ASTDeclReader::MergeDefinitionData(
   OR_FIELD(HasDeclaredCopyConstructorWithConstParam)
   OR_FIELD(HasDeclaredCopyAssignmentWithConstParam)
   MATCH_FIELD(IsLambda)
-  MATCH_FIELD(ODRHash)
 #undef OR_FIELD
 #undef MATCH_FIELD
 
@@ -1697,6 +1697,10 @@ void ASTDeclReader::MergeDefinitionData(
     // 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/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index ec59250e1921808207c7f2b8a1e734cdb09fc5f6..6f0d0eed5b9dce90e430e8f265e65d3b98886a27 100644 (file)
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -5769,7 +5769,10 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
   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);