diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h --- a/clang/include/clang/AST/Decl.h +++ b/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 { diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h --- a/clang/include/clang/AST/DeclBase.h +++ b/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 diff --git a/clang/include/clang/AST/ODRHash.h b/clang/include/clang/AST/ODRHash.h --- a/clang/include/clang/AST/ODRHash.h +++ b/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. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1107,6 +1107,10 @@ llvm::SmallDenseMap, 2> PendingEnumOdrMergeFailures; + /// C/ObjC definitions in which the structural equivalence check fails + llvm::SmallDenseMap, 2> + PendingRecordOdrMergeFailures; + /// DeclContexts in which we have diagnosed an ODR violation. llvm::SmallPtrSet DiagnosedOdrMergeFailures; diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp --- a/clang/lib/AST/Decl.cpp +++ b/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 //===----------------------------------------------------------------------===// diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp --- a/clang/lib/AST/DeclCXX.cpp +++ b/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(); diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp --- a/clang/lib/AST/ODRHash.cpp +++ b/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 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."); diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/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(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, 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(FirstDecl); + FieldDecl *SecondField = cast(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(FirstDecl); + TypedefNameDecl *SecondTD = cast(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(FirstDecl); + VarDecl *SecondVD = cast(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 { diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -723,8 +723,11 @@ llvm_unreachable("unexpected tag info kind"); } - if (!isa(TD)) - mergeRedeclarable(TD, Redecl); + if (isa(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(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(ND)) + if (isa(ND) || isa(ND)) return true; return false; } @@ -3197,6 +3216,10 @@ return ED->getASTContext().getLangOpts().CPlusPlus? ED->getDefinition() : nullptr; + if (auto *RD = dyn_cast(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(DC)) diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/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 diff --git a/clang/test/Modules/odr_hash-record.c b/clang/test/Modules/odr_hash-record.c new file mode 100644 --- /dev/null +++ b/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