Author: rtrieu Date: Tue Apr 11 16:31:00 2017 New Revision: 299989 URL: http://llvm.org/viewvc/llvm-project?rev=299989&view=rev Log: [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. Modified: cfe/trunk/include/clang/AST/DeclCXX.h cfe/trunk/lib/AST/DeclCXX.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Serialization/ASTReaderDecl.cpp cfe/trunk/lib/Serialization/ASTWriter.cpp Modified: cfe/trunk/include/clang/AST/DeclCXX.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=299989&r1=299988&r2=299989&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/DeclCXX.h (original) +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Apr 11 16:31:00 2017 @@ -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); Modified: cfe/trunk/lib/AST/DeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=299989&r1=299988&r2=299989&view=diff ============================================================================== --- cfe/trunk/lib/AST/DeclCXX.cpp (original) +++ cfe/trunk/lib/AST/DeclCXX.cpp Tue Apr 11 16:31:00 2017 @@ -73,8 +73,9 @@ CXXRecordDecl::DefinitionData::Definitio 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 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 Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=299989&r1=299988&r2=299989&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Apr 11 16:31:00 2017 @@ -13797,10 +13797,8 @@ void Sema::ActOnTagFinishDefinition(Scop 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. Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=299989&r1=299988&r2=299989&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Tue Apr 11 16:31:00 2017 @@ -1536,6 +1536,7 @@ void ASTDeclReader::ReadCXXDefinitionDat 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); } Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=299989&r1=299988&r2=299989&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Apr 11 16:31:00 2017 @@ -5769,7 +5769,10 @@ void ASTRecordWriter::AddCXXDefinitionDa 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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits