diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -260,6 +260,7 @@ friend class ASTWriter; friend class DeclContext; friend class LambdaExpr; + friend class ODRDiagsEmitter; friend void FunctionDecl::setPure(bool); friend void TagDecl::startDefinition(); 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 @@ -1845,10 +1845,6 @@ /// if the declaration is not from a module file. ModuleFile *getOwningModuleFile(const Decl *D); - /// Get the best name we know for the module that owns the given - /// declaration, or an empty string if the declaration is not from a module. - std::string getOwningModuleNameForDiagnostic(const Decl *D); - /// Returns the source location for the decl \p ID. SourceLocation getSourceLocationForDeclID(serialization::GlobalDeclID ID); 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 @@ -9186,19 +9186,6 @@ } } -std::string ASTReader::getOwningModuleNameForDiagnostic(const Decl *D) { - // If we know the owning module, use it. - if (Module *M = D->getImportedOwningModule()) - return M->getFullModuleName(); - - // Otherwise, use the name of the top-level module the decl is within. - if (ModuleFile *M = getOwningModuleFile(D)) - return M->ModuleName; - - // Not from a module. - return {}; -} - void ASTReader::finishPendingActions() { while (!PendingIdentifierInfos.empty() || !PendingFunctionTypes.empty() || !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() || @@ -9446,6 +9433,114 @@ PendingMergedDefinitionsToDeduplicate.clear(); } +namespace clang { +class ODRDiagsEmitter { +public: + ODRDiagsEmitter(DiagnosticsEngine &Diags, const ASTContext &Context, + const LangOptions &LangOpts) + : Diags(Diags), Context(Context), LangOpts(LangOpts) {} + + /// Diagnose ODR mismatch between 2 FunctionDecl. + /// + /// Returns true if found a mismatch and diagnosed it. + bool diagnoseMismatch(const FunctionDecl *FirstFunction, + const FunctionDecl *SecondFunction) const; + + /// Diagnose ODR mismatch between 2 EnumDecl. + /// + /// Returns true if found a mismatch and diagnosed it. + bool diagnoseMismatch(const EnumDecl *FirstEnum, + const EnumDecl *SecondEnum) const; + + /// Diagnose ODR mismatch between 2 CXXRecordDecl. + /// + /// Returns true if found a mismatch and diagnosed it. + /// To compare 2 declarations with merged and identical definition data + /// you need to provide pre-merge definition data in \p SecondDD. + bool + diagnoseMismatch(const CXXRecordDecl *FirstRecord, + const CXXRecordDecl *SecondRecord, + const struct CXXRecordDecl::DefinitionData *SecondDD) const; + + /// Get the best name we know for the module that owns the given + /// declaration, or an empty string if the declaration is not from a module. + static std::string getOwningModuleNameForDiagnostic(const Decl *D); + +private: + using DeclHashes = llvm::SmallVector, 4>; + + // 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::isDeclToBeProcessed + enum ODRMismatchDecl { + EndOfClass, + PublicSpecifer, + PrivateSpecifer, + ProtectedSpecifer, + StaticAssert, + Field, + CXXMethod, + TypeAlias, + TypeDef, + Var, + Friend, + FunctionTemplate, + Other + }; + + struct DiffResult { + const Decl *FirstDecl = nullptr, *SecondDecl = nullptr; + ODRMismatchDecl FirstDiffType = Other, SecondDiffType = Other; + }; + + // If there is a diagnoseable difference, FirstDiffType and + // SecondDiffType will not be Other and FirstDecl and SecondDecl will be + // filled in if not EndOfClass. + static DiffResult FindTypeDiffs(DeclHashes &FirstHashes, + DeclHashes &SecondHashes); + + DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) const { + return Diags.Report(Loc, DiagID); + } + + // Use this to diagnose that an unexpected Decl was encountered + // or no difference was detected. This causes a generic error + // message to be emitted. + void diagnoseSubMismatchUnexpected(DiffResult &DR, + const NamedDecl *FirstRecord, + StringRef FirstModule, + const NamedDecl *SecondRecord, + StringRef SecondModule) const; + + void diagnoseSubMismatchDifferentDeclKinds(DiffResult &DR, + const NamedDecl *FirstRecord, + StringRef FirstModule, + const NamedDecl *SecondRecord, + StringRef SecondModule) const; + + bool diagnoseSubMismatchField(const NamedDecl *FirstRecord, + StringRef FirstModule, StringRef SecondModule, + const FieldDecl *FirstField, + const FieldDecl *SecondField) const; + + bool diagnoseSubMismatchTypedef(const NamedDecl *FirstRecord, + StringRef FirstModule, StringRef SecondModule, + const TypedefNameDecl *FirstTD, + const TypedefNameDecl *SecondTD, + bool IsTypeAlias) const; + + bool diagnoseSubMismatchVar(const NamedDecl *FirstRecord, + StringRef FirstModule, StringRef SecondModule, + const VarDecl *FirstVD, + const VarDecl *SecondVD) const; + +private: + DiagnosticsEngine &Diags; + const ASTContext &Context; + const LangOptions &LangOpts; +}; +} // namespace clang + static unsigned computeODRHash(QualType Ty) { ODRHash Hasher; Hasher.AddQualType(Ty); @@ -9471,6 +9566,15 @@ return Hasher.CalculateHash(); } +std::string ODRDiagsEmitter::getOwningModuleNameForDiagnostic(const Decl *D) { + // If we know the owning module, use it. + if (Module *M = D->getImportedOwningModule()) + return M->getFullModuleName(); + + // Not from a module. + return {}; +} + void ASTReader::diagnoseOdrViolations() { if (PendingOdrMergeFailures.empty() && PendingOdrMergeChecks.empty() && PendingFunctionOdrMergeFailures.empty() && @@ -9583,9 +9687,10 @@ Deserializing RecursionGuard(this); std::string CanonDefModule = - getOwningModuleNameForDiagnostic(cast(CanonDef)); + ODRDiagsEmitter::getOwningModuleNameForDiagnostic( + cast(CanonDef)); Diag(D->getLocation(), diag::err_module_odr_violation_missing_decl) - << D << getOwningModuleNameForDiagnostic(D) + << D << ODRDiagsEmitter::getOwningModuleNameForDiagnostic(D) << CanonDef << CanonDefModule.empty() << CanonDefModule; if (Candidates.empty()) @@ -9609,33 +9714,77 @@ // Ensure we don't accidentally recursively enter deserialization while // we're producing our diagnostics. Deserializing RecursionGuard(this); + ODRDiagsEmitter DiagsEmitter(Diags, getContext(), + getPreprocessor().getLangOpts()); - // 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::isDeclToBeProcessed - enum ODRMismatchDecl { - EndOfClass, - PublicSpecifer, - PrivateSpecifer, - ProtectedSpecifer, - StaticAssert, - Field, - CXXMethod, - TypeAlias, - TypeDef, - Var, - Friend, - FunctionTemplate, - Other - }; + // Issue any pending ODR-failure diagnostics. + for (auto &Merge : OdrMergeFailures) { + // 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; - // 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 ODRDiagField = [this](NamedDecl *FirstRecord, StringRef FirstModule, - StringRef SecondModule, - const FieldDecl *FirstField, - const FieldDecl *SecondField) { + bool Diagnosed = false; + CXXRecordDecl *FirstRecord = Merge.first; + for (auto &RecordPair : Merge.second) { + if (DiagsEmitter.diagnoseMismatch(FirstRecord, RecordPair.first, + RecordPair.second)) { + Diagnosed = true; + break; + } + } + + if (!Diagnosed) { + // All definitions are updates to the same declaration. This happens if a + // module instantiates the declaration of a class template specialization + // and two or more other modules instantiate its definition. + // + // FIXME: Indicate which modules had instantiations of this definition. + // FIXME: How can this even happen? + Diag(Merge.first->getLocation(), + diag::err_module_odr_violation_different_instantiations) + << Merge.first; + } + } + + // Issue ODR failures diagnostics for functions. + for (auto &Merge : FunctionOdrMergeFailures) { + FunctionDecl *FirstFunction = Merge.first; + bool Diagnosed = false; + for (auto &SecondFunction : Merge.second) { + if (DiagsEmitter.diagnoseMismatch(FirstFunction, SecondFunction)) { + Diagnosed = true; + break; + } + } + (void)Diagnosed; + assert(Diagnosed && "Unable to emit ODR diagnostic."); + } + + // Issue ODR failures diagnostics for enums. + for (auto &Merge : EnumOdrMergeFailures) { + // If we've already pointed out a specific problem with this enum, don't + // bother issuing a general "something's different" diagnostic. + if (!DiagnosedOdrMergeFailures.insert(Merge.first).second) + continue; + + EnumDecl *FirstEnum = Merge.first; + bool Diagnosed = false; + for (auto &SecondEnum : Merge.second) { + if (DiagsEmitter.diagnoseMismatch(FirstEnum, SecondEnum)) { + Diagnosed = true; + break; + } + } + (void)Diagnosed; + assert(Diagnosed && "Unable to emit ODR diagnostic."); + } +} + +// clang-format off + bool ODRDiagsEmitter::diagnoseSubMismatchField(const NamedDecl *FirstRecord, + StringRef FirstModule, StringRef SecondModule, + const FieldDecl *FirstField, const FieldDecl *SecondField) const { enum ODRFieldDifference { FieldName, FieldTypeName, @@ -9668,8 +9817,7 @@ return true; } - assert(getContext().hasSameType(FirstField->getType(), - SecondField->getType())); + assert(Context.hasSameType(FirstField->getType(), SecondField->getType())); QualType FirstType = FirstField->getType(); QualType SecondType = SecondField->getType(); @@ -9699,7 +9847,7 @@ } } - if (!PP.getLangOpts().CPlusPlus) + if (!LangOpts.CPlusPlus) return false; const bool IsFirstMutable = FirstField->isMutable(); @@ -9734,12 +9882,12 @@ } return false; - }; + } - auto ODRDiagTypeDefOrAlias = - [this](NamedDecl *FirstRecord, StringRef FirstModule, - StringRef SecondModule, const TypedefNameDecl *FirstTD, - const TypedefNameDecl *SecondTD, bool IsTypeAlias) { + bool ODRDiagsEmitter::diagnoseSubMismatchTypedef(const NamedDecl *FirstRecord, + StringRef FirstModule, StringRef SecondModule, + const TypedefNameDecl *FirstTD, const TypedefNameDecl *SecondTD, + bool IsTypeAlias) const { enum ODRTypedefDifference { TypedefName, TypedefType, @@ -9774,13 +9922,14 @@ DiagNote(TypedefType) << IsTypeAlias << SecondName << SecondType; return true; } - return false; - }; + } - auto ODRDiagVar = [this](NamedDecl *FirstRecord, StringRef FirstModule, - StringRef SecondModule, const VarDecl *FirstVD, - const VarDecl *SecondVD) { + bool ODRDiagsEmitter::diagnoseSubMismatchVar(const NamedDecl *FirstRecord, + StringRef FirstModule, + StringRef SecondModule, + const VarDecl *FirstVD, + const VarDecl *SecondVD) const { enum ODRVarDifference { VarName, VarType, @@ -9818,7 +9967,7 @@ return true; } - if (!PP.getLangOpts().CPlusPlus) + if (!LangOpts.CPlusPlus) return false; const Expr *FirstInit = FirstVD->getInit(); @@ -9850,28 +9999,12 @@ return true; } return false; - }; - - using DeclHashes = llvm::SmallVector, 4>; - auto PopulateHashes = [](DeclHashes &Hashes, RecordDecl *Record, - const DeclContext *DC) { - for (auto *D : Record->decls()) { - if (!ODRHash::isDeclToBeProcessed(D, DC)) - continue; - Hashes.emplace_back(D, computeODRHash(D)); - } - }; - - struct DiffResult { - Decl *FirstDecl = nullptr, *SecondDecl = nullptr; - ODRMismatchDecl FirstDiffType = Other, SecondDiffType = Other; - }; + } - // If there is a diagnoseable difference, FirstDiffType and - // SecondDiffType will not be Other and FirstDecl and SecondDecl will be - // filled in if not EndOfClass. - auto FindTypeDiffs = [](DeclHashes &FirstHashes, DeclHashes &SecondHashes) { - auto DifferenceSelector = [](Decl *D) { + ODRDiagsEmitter::DiffResult + ODRDiagsEmitter::FindTypeDiffs(DeclHashes &FirstHashes, + DeclHashes &SecondHashes) { + auto DifferenceSelector = [](const Decl *D) { assert(D && "valid Decl required"); switch (D->getKind()) { default: @@ -9931,15 +10064,11 @@ return DR; } return DR; - }; + } - // Use this to diagnose that an unexpected Decl was encountered - // or no difference was detected. This causes a generic error - // message to be emitted. - auto DiagnoseODRUnexpected = [this](DiffResult &DR, NamedDecl *FirstRecord, - StringRef FirstModule, - NamedDecl *SecondRecord, - StringRef SecondModule) { + void ODRDiagsEmitter::diagnoseSubMismatchUnexpected( + DiffResult &DR, const NamedDecl *FirstRecord, StringRef FirstModule, + const NamedDecl *SecondRecord, StringRef SecondModule) const { Diag(FirstRecord->getLocation(), diag::err_module_odr_violation_different_definitions) << FirstRecord << FirstModule.empty() << FirstModule; @@ -9957,12 +10086,11 @@ Diag(DR.SecondDecl->getLocation(), diag::note_second_module_difference) << DR.SecondDecl->getSourceRange(); } - }; + } - auto DiagnoseODRMismatch = [this](DiffResult &DR, NamedDecl *FirstRecord, - StringRef FirstModule, - NamedDecl *SecondRecord, - StringRef SecondModule) { + void ODRDiagsEmitter::diagnoseSubMismatchDifferentDeclKinds( + DiffResult &DR, const NamedDecl *FirstRecord, StringRef FirstModule, + const NamedDecl *SecondRecord, StringRef SecondModule) const { auto GetMismatchedDeclLoc = [](const NamedDecl *Container, ODRMismatchDecl DiffType, const Decl *D) { SourceLocation Loc; @@ -9987,34 +10115,26 @@ GetMismatchedDeclLoc(SecondRecord, DR.SecondDiffType, DR.SecondDecl); Diag(SecondDiagInfo.first, diag::note_module_odr_violation_mismatch_decl) << SecondModule << SecondDiagInfo.second << DR.SecondDiffType; - }; - - // Issue any pending ODR-failure diagnostics. - for (auto &Merge : OdrMergeFailures) { - // 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; - CXXRecordDecl *FirstRecord = Merge.first; - std::string FirstModule = getOwningModuleNameForDiagnostic(FirstRecord); - for (auto &RecordPair : Merge.second) { - CXXRecordDecl *SecondRecord = RecordPair.first; + bool ODRDiagsEmitter::diagnoseMismatch( + const CXXRecordDecl *FirstRecord, const CXXRecordDecl *SecondRecord, + const struct CXXRecordDecl::DefinitionData *SecondDD) const { // Multiple different declarations got merged together; tell the user // where they came from. if (FirstRecord == SecondRecord) - continue; + return false; + std::string FirstModule = getOwningModuleNameForDiagnostic(FirstRecord); std::string SecondModule = getOwningModuleNameForDiagnostic(SecondRecord); - auto *FirstDD = FirstRecord->DefinitionData; - auto *SecondDD = RecordPair.second; - + const struct CXXRecordDecl::DefinitionData *FirstDD = + FirstRecord->DefinitionData; assert(FirstDD && SecondDD && "Definitions without DefinitionData"); // Diagnostics from DefinitionData are emitted here. if (FirstDD != SecondDD) { + // Keep in sync with err_module_odr_violation_definition_data. enum ODRDefinitionDataDifference { NumBases, NumVBases, @@ -10022,20 +10142,20 @@ BaseVirtual, BaseAccess, }; - auto ODRDiagBaseError = [FirstRecord, &FirstModule, + auto DiagBaseError = [FirstRecord, &FirstModule, this](SourceLocation Loc, SourceRange Range, ODRDefinitionDataDifference DiffType) { return Diag(Loc, diag::err_module_odr_violation_definition_data) << FirstRecord << FirstModule.empty() << FirstModule << Range << DiffType; }; - auto ODRDiagBaseNote = [&SecondModule, + auto DiagBaseNote = [&SecondModule, this](SourceLocation Loc, SourceRange Range, ODRDefinitionDataDifference DiffType) { return Diag(Loc, diag::note_module_odr_violation_definition_data) << SecondModule << Range << DiffType; }; - auto GetSourceRange = [](struct CXXRecordDecl::DefinitionData *DD) { + auto GetSourceRange = [](const struct CXXRecordDecl::DefinitionData *DD) { unsigned NumBases = DD->NumBases; if (NumBases == 0) return SourceRange(); ArrayRef bases = DD->bases(); @@ -10048,72 +10168,64 @@ unsigned SecondNumBases = SecondDD->NumBases; unsigned SecondNumVBases = SecondDD->NumVBases; if (FirstNumBases != SecondNumBases) { - ODRDiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD), + DiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD), NumBases) << FirstNumBases; - ODRDiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD), + DiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD), NumBases) << SecondNumBases; - Diagnosed = true; - break; + return true; } if (FirstNumVBases != SecondNumVBases) { - ODRDiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD), + DiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD), NumVBases) << FirstNumVBases; - ODRDiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD), + DiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD), NumVBases) << SecondNumVBases; - Diagnosed = true; - break; + return true; } ArrayRef FirstBases = FirstDD->bases(); ArrayRef SecondBases = SecondDD->bases(); - unsigned I = 0; - for (I = 0; I < FirstNumBases; ++I) { + for (unsigned I = 0; I < FirstNumBases; ++I) { const CXXBaseSpecifier FirstBase = FirstBases[I]; const CXXBaseSpecifier SecondBase = SecondBases[I]; if (computeODRHash(FirstBase.getType()) != computeODRHash(SecondBase.getType())) { - ODRDiagBaseError(FirstRecord->getLocation(), + DiagBaseError(FirstRecord->getLocation(), FirstBase.getSourceRange(), BaseType) << (I + 1) << FirstBase.getType(); - ODRDiagBaseNote(SecondRecord->getLocation(), + DiagBaseNote(SecondRecord->getLocation(), SecondBase.getSourceRange(), BaseType) << (I + 1) << SecondBase.getType(); - break; + return true; } if (FirstBase.isVirtual() != SecondBase.isVirtual()) { - ODRDiagBaseError(FirstRecord->getLocation(), + DiagBaseError(FirstRecord->getLocation(), FirstBase.getSourceRange(), BaseVirtual) << (I + 1) << FirstBase.isVirtual() << FirstBase.getType(); - ODRDiagBaseNote(SecondRecord->getLocation(), + DiagBaseNote(SecondRecord->getLocation(), SecondBase.getSourceRange(), BaseVirtual) << (I + 1) << SecondBase.isVirtual() << SecondBase.getType(); - break; + return true; } if (FirstBase.getAccessSpecifierAsWritten() != SecondBase.getAccessSpecifierAsWritten()) { - ODRDiagBaseError(FirstRecord->getLocation(), + DiagBaseError(FirstRecord->getLocation(), FirstBase.getSourceRange(), BaseAccess) << (I + 1) << FirstBase.getType() << (int)FirstBase.getAccessSpecifierAsWritten(); - ODRDiagBaseNote(SecondRecord->getLocation(), + DiagBaseNote(SecondRecord->getLocation(), SecondBase.getSourceRange(), BaseAccess) << (I + 1) << SecondBase.getType() << (int)SecondBase.getAccessSpecifierAsWritten(); - break; + return true; } } - - if (I != FirstNumBases) { - Diagnosed = true; - break; - } } const ClassTemplateDecl *FirstTemplate = @@ -10125,32 +10237,18 @@ "Both pointers should be null or non-null"); if (FirstTemplate && SecondTemplate) { - DeclHashes FirstTemplateHashes; - DeclHashes SecondTemplateHashes; - - auto PopulateTemplateParameterHashs = [](DeclHashes &Hashes, - const ClassTemplateDecl *TD) { - for (auto *D : TD->getTemplateParameters()->asArray()) { - Hashes.emplace_back(D, computeODRHash(D)); - } - }; - - PopulateTemplateParameterHashs(FirstTemplateHashes, FirstTemplate); - PopulateTemplateParameterHashs(SecondTemplateHashes, SecondTemplate); - - assert(FirstTemplateHashes.size() == SecondTemplateHashes.size() && + ArrayRef FirstTemplateParams = + FirstTemplate->getTemplateParameters()->asArray(); + ArrayRef SecondTemplateParams = + SecondTemplate->getTemplateParameters()->asArray(); + assert(FirstTemplateParams.size() == SecondTemplateParams.size() && "Number of template parameters should be equal."); - - auto FirstIt = FirstTemplateHashes.begin(); - auto FirstEnd = FirstTemplateHashes.end(); - auto SecondIt = SecondTemplateHashes.begin(); - for (; FirstIt != FirstEnd; ++FirstIt, ++SecondIt) { - if (FirstIt->second == SecondIt->second) + for (auto Pair : llvm::zip(FirstTemplateParams, SecondTemplateParams)) { + const NamedDecl *FirstDecl = std::get<0>(Pair); + const NamedDecl *SecondDecl = std::get<1>(Pair); + if (computeODRHash(FirstDecl) == computeODRHash(SecondDecl)) continue; - const NamedDecl* FirstDecl = cast(FirstIt->first); - const NamedDecl* SecondDecl = cast(SecondIt->first); - assert(FirstDecl->getKind() == SecondDecl->getKind() && "Parameter Decl's should be the same kind."); @@ -10202,14 +10300,18 @@ diag::note_module_odr_violation_template_parameter) << SecondModule << SecondDecl->getSourceRange() << NoteDiffType << hasSecondArg << SecondName; - break; + return true; } + } - if (FirstIt != FirstEnd) { - Diagnosed = true; - break; + auto PopulateHashes = [](DeclHashes &Hashes, const RecordDecl *Record, + const DeclContext *DC) { + for (const Decl *D : Record->decls()) { + if (!ODRHash::isDeclToBeProcessed(D, DC)) + continue; + Hashes.emplace_back(D, computeODRHash(D)); } - } + }; DeclHashes FirstHashes; DeclHashes SecondHashes; @@ -10224,17 +10326,15 @@ const Decl *SecondDecl = DR.SecondDecl; if (FirstDiffType == Other || SecondDiffType == Other) { - DiagnoseODRUnexpected(DR, FirstRecord, FirstModule, SecondRecord, - SecondModule); - Diagnosed = true; - break; + diagnoseSubMismatchUnexpected(DR, FirstRecord, FirstModule, SecondRecord, + SecondModule); + return true; } if (FirstDiffType != SecondDiffType) { - DiagnoseODRMismatch(DR, FirstRecord, FirstModule, SecondRecord, - SecondModule); - Diagnosed = true; - break; + diagnoseSubMismatchDifferentDeclKinds(DR, FirstRecord, FirstModule, + SecondRecord, SecondModule); + return true; } // Used with err_module_odr_violation_record and @@ -10272,14 +10372,14 @@ FunctionTemplateParameterDifferentType, FunctionTemplatePackParameter, }; - auto ODRDiagDeclError = [FirstRecord, &FirstModule, + auto DiagError = [FirstRecord, &FirstModule, this](SourceLocation Loc, SourceRange Range, ODRCXXRecordDifference DiffType) { return Diag(Loc, diag::err_module_odr_violation_record) << FirstRecord << FirstModule.empty() << FirstModule << Range << DiffType; }; - auto ODRDiagDeclNote = [&SecondModule, + auto DiagNote = [&SecondModule, this](SourceLocation Loc, SourceRange Range, ODRCXXRecordDifference DiffType) { return Diag(Loc, diag::note_module_odr_violation_record) @@ -10304,12 +10404,11 @@ unsigned FirstODRHash = computeODRHash(FirstExpr); unsigned SecondODRHash = computeODRHash(SecondExpr); if (FirstODRHash != SecondODRHash) { - ODRDiagDeclError(FirstExpr->getBeginLoc(), + DiagError(FirstExpr->getBeginLoc(), FirstExpr->getSourceRange(), StaticAssertCondition); - ODRDiagDeclNote(SecondExpr->getBeginLoc(), + DiagNote(SecondExpr->getBeginLoc(), SecondExpr->getSourceRange(), StaticAssertCondition); - Diagnosed = true; - break; + return true; } const StringLiteral *FirstStr = FirstSA->getMessage(); @@ -10332,29 +10431,28 @@ SecondLoc = SecondSA->getBeginLoc(); SecondRange = SecondSA->getSourceRange(); } - ODRDiagDeclError(FirstLoc, FirstRange, StaticAssertOnlyMessage) + DiagError(FirstLoc, FirstRange, StaticAssertOnlyMessage) << (FirstStr == nullptr); - ODRDiagDeclNote(SecondLoc, SecondRange, StaticAssertOnlyMessage) + DiagNote(SecondLoc, SecondRange, StaticAssertOnlyMessage) << (SecondStr == nullptr); - Diagnosed = true; - break; + return true; } if (FirstStr && SecondStr && FirstStr->getString() != SecondStr->getString()) { - ODRDiagDeclError(FirstStr->getBeginLoc(), FirstStr->getSourceRange(), + DiagError(FirstStr->getBeginLoc(), FirstStr->getSourceRange(), StaticAssertMessage); - ODRDiagDeclNote(SecondStr->getBeginLoc(), SecondStr->getSourceRange(), + DiagNote(SecondStr->getBeginLoc(), SecondStr->getSourceRange(), StaticAssertMessage); - Diagnosed = true; - break; + return true; } break; } case Field: { - Diagnosed = ODRDiagField(FirstRecord, FirstModule, SecondModule, - cast(FirstDecl), - cast(SecondDecl)); + if (diagnoseSubMismatchField(FirstRecord, FirstModule, SecondModule, + cast(FirstDecl), + cast(SecondDecl))) + return true; break; } case CXXMethod: { @@ -10375,15 +10473,15 @@ SecondMethodType = GetMethodTypeForDiagnostics(SecondMethod); DeclarationName FirstName = FirstMethod->getDeclName(); DeclarationName SecondName = SecondMethod->getDeclName(); - auto DiagMethodError = [&ODRDiagDeclError, FirstMethod, FirstMethodType, + auto DiagMethodError = [&DiagError, FirstMethod, FirstMethodType, FirstName](ODRCXXRecordDifference DiffType) { - return ODRDiagDeclError(FirstMethod->getLocation(), + return DiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), DiffType) << FirstMethodType << FirstName; }; - auto DiagMethodNote = [&ODRDiagDeclNote, SecondMethod, SecondMethodType, + auto DiagMethodNote = [&DiagNote, SecondMethod, SecondMethodType, SecondName](ODRCXXRecordDifference DiffType) { - return ODRDiagDeclNote(SecondMethod->getLocation(), + return DiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), DiffType) << SecondMethodType << SecondName; }; @@ -10391,8 +10489,7 @@ if (FirstMethodType != SecondMethodType || FirstName != SecondName) { DiagMethodError(MethodName); DiagMethodNote(MethodName); - Diagnosed = true; - break; + return true; } const bool FirstDeleted = FirstMethod->isDeletedAsWritten(); @@ -10400,8 +10497,7 @@ if (FirstDeleted != SecondDeleted) { DiagMethodError(MethodDeleted) << FirstDeleted; DiagMethodNote(MethodDeleted) << SecondDeleted; - Diagnosed = true; - break; + return true; } const bool FirstDefaulted = FirstMethod->isExplicitlyDefaulted(); @@ -10409,8 +10505,7 @@ if (FirstDefaulted != SecondDefaulted) { DiagMethodError(MethodDefaulted) << FirstDefaulted; DiagMethodNote(MethodDefaulted) << SecondDefaulted; - Diagnosed = true; - break; + return true; } const bool FirstVirtual = FirstMethod->isVirtualAsWritten(); @@ -10421,8 +10516,7 @@ (FirstVirtual != SecondVirtual || FirstPure != SecondPure)) { DiagMethodError(MethodVirtual) << FirstPure << FirstVirtual; DiagMethodNote(MethodVirtual) << SecondPure << SecondVirtual; - Diagnosed = true; - break; + return true; } // CXXMethodDecl::isStatic uses the canonical Decl. With Decl merging, @@ -10435,8 +10529,7 @@ if (FirstStatic != SecondStatic) { DiagMethodError(MethodStatic) << FirstStatic; DiagMethodNote(MethodStatic) << SecondStatic; - Diagnosed = true; - break; + return true; } const bool FirstVolatile = FirstMethod->isVolatile(); @@ -10444,8 +10537,7 @@ if (FirstVolatile != SecondVolatile) { DiagMethodError(MethodVolatile) << FirstVolatile; DiagMethodNote(MethodVolatile) << SecondVolatile; - Diagnosed = true; - break; + return true; } const bool FirstConst = FirstMethod->isConst(); @@ -10453,8 +10545,7 @@ if (FirstConst != SecondConst) { DiagMethodError(MethodConst) << FirstConst; DiagMethodNote(MethodConst) << SecondConst; - Diagnosed = true; - break; + return true; } const bool FirstInline = FirstMethod->isInlineSpecified(); @@ -10462,8 +10553,7 @@ if (FirstInline != SecondInline) { DiagMethodError(MethodInline) << FirstInline; DiagMethodNote(MethodInline) << SecondInline; - Diagnosed = true; - break; + return true; } const unsigned FirstNumParameters = FirstMethod->param_size(); @@ -10471,12 +10561,9 @@ if (FirstNumParameters != SecondNumParameters) { DiagMethodError(MethodNumberParameters) << FirstNumParameters; DiagMethodNote(MethodNumberParameters) << SecondNumParameters; - Diagnosed = true; - break; + return true; } - // Need this status boolean to know when break out of the switch. - bool ParameterMismatch = false; for (unsigned I = 0; I < FirstNumParameters; ++I) { const ParmVarDecl *FirstParam = FirstMethod->getParamDecl(I); const ParmVarDecl *SecondParam = SecondMethod->getParamDecl(I); @@ -10505,8 +10592,7 @@ DiagMethodNote(MethodParameterType) << (I + 1) << SecondParamType << false; } - ParameterMismatch = true; - break; + return true; } DeclarationName FirstParamName = FirstParam->getDeclName(); @@ -10514,8 +10600,7 @@ if (FirstParamName != SecondParamName) { DiagMethodError(MethodParameterName) << (I + 1) << FirstParamName; DiagMethodNote(MethodParameterName) << (I + 1) << SecondParamName; - ParameterMismatch = true; - break; + return true; } const Expr *FirstInit = FirstParam->getInit(); @@ -10527,8 +10612,7 @@ DiagMethodNote(MethodParameterSingleDefaultArgument) << (I + 1) << (SecondInit == nullptr) << (SecondInit ? SecondInit->getSourceRange() : SourceRange()); - ParameterMismatch = true; - break; + return true; } if (FirstInit && SecondInit && @@ -10537,16 +10621,10 @@ << (I + 1) << FirstInit->getSourceRange(); DiagMethodNote(MethodParameterDifferentDefaultArgument) << (I + 1) << SecondInit->getSourceRange(); - ParameterMismatch = true; - break; + return true; } } - if (ParameterMismatch) { - Diagnosed = true; - break; - } - const TemplateArgumentList *FirstTemplateArgs = FirstMethod->getTemplateSpecializationArgs(); const TemplateArgumentList *SecondTemplateArgs = @@ -10558,8 +10636,7 @@ << (FirstTemplateArgs != nullptr); DiagMethodNote(MethodNoTemplateArguments) << (SecondTemplateArgs != nullptr); - Diagnosed = true; - break; + return true; } if (FirstTemplateArgs && SecondTemplateArgs) { @@ -10587,11 +10664,9 @@ << (unsigned)FirstExpandedList.size(); DiagMethodNote(MethodDifferentNumberTemplateArguments) << (unsigned)SecondExpandedList.size(); - Diagnosed = true; - break; + return true; } - bool TemplateArgumentMismatch = false; for (unsigned i = 0, e = FirstExpandedList.size(); i != e; ++i) { const TemplateArgument &FirstTA = *FirstExpandedList[i], &SecondTA = *SecondExpandedList[i]; @@ -10603,13 +10678,7 @@ << FirstTA << i + 1; DiagMethodNote(MethodDifferentTemplateArgument) << SecondTA << i + 1; - TemplateArgumentMismatch = true; - break; - } - - if (TemplateArgumentMismatch) { - Diagnosed = true; - break; + return true; } } @@ -10631,31 +10700,32 @@ if (HasFirstBody != HasSecondBody) { DiagMethodError(MethodSingleBody) << HasFirstBody; DiagMethodNote(MethodSingleBody) << HasSecondBody; - Diagnosed = true; - break; + return true; } if (HasFirstBody && HasSecondBody) { DiagMethodError(MethodDifferentBody); DiagMethodNote(MethodDifferentBody); - Diagnosed = true; - break; + return true; } break; } + case TypeAlias: case TypeDef: { - Diagnosed = ODRDiagTypeDefOrAlias( - FirstRecord, FirstModule, SecondModule, - cast(FirstDecl), cast(SecondDecl), - FirstDiffType == TypeAlias); + if (diagnoseSubMismatchTypedef(FirstRecord, FirstModule, SecondModule, + cast(FirstDecl), + cast(SecondDecl), + FirstDiffType == TypeAlias)) + return true; break; } case Var: { - Diagnosed = - ODRDiagVar(FirstRecord, FirstModule, SecondModule, - cast(FirstDecl), cast(SecondDecl)); + if (diagnoseSubMismatchVar(FirstRecord, FirstModule, SecondModule, + cast(FirstDecl), + cast(SecondDecl))) + return true; break; } case Friend: { @@ -10669,14 +10739,13 @@ TypeSourceInfo *SecondTSI = SecondFriend->getFriendType(); if (FirstND && SecondND) { - ODRDiagDeclError(FirstFriend->getFriendLoc(), + DiagError(FirstFriend->getFriendLoc(), FirstFriend->getSourceRange(), FriendFunction) << FirstND; - ODRDiagDeclNote(SecondFriend->getFriendLoc(), + DiagNote(SecondFriend->getFriendLoc(), SecondFriend->getSourceRange(), FriendFunction) << SecondND; - Diagnosed = true; - break; + return true; } if (FirstTSI && SecondTSI) { @@ -10684,24 +10753,22 @@ QualType SecondFriendType = SecondTSI->getType(); assert(computeODRHash(FirstFriendType) != computeODRHash(SecondFriendType)); - ODRDiagDeclError(FirstFriend->getFriendLoc(), + DiagError(FirstFriend->getFriendLoc(), FirstFriend->getSourceRange(), FriendType) << FirstFriendType; - ODRDiagDeclNote(SecondFriend->getFriendLoc(), + DiagNote(SecondFriend->getFriendLoc(), SecondFriend->getSourceRange(), FriendType) << SecondFriendType; - Diagnosed = true; - break; + return true; } - ODRDiagDeclError(FirstFriend->getFriendLoc(), + DiagError(FirstFriend->getFriendLoc(), FirstFriend->getSourceRange(), FriendTypeFunction) << (FirstTSI == nullptr); - ODRDiagDeclNote(SecondFriend->getFriendLoc(), + DiagNote(SecondFriend->getFriendLoc(), SecondFriend->getSourceRange(), FriendTypeFunction) << (SecondTSI == nullptr); - Diagnosed = true; - break; + return true; } case FunctionTemplate: { const FunctionTemplateDecl *FirstTemplate = @@ -10714,15 +10781,15 @@ TemplateParameterList *SecondTPL = SecondTemplate->getTemplateParameters(); - auto DiagTemplateError = [&ODRDiagDeclError, FirstTemplate]( + auto DiagTemplateError = [&DiagError, FirstTemplate]( ODRCXXRecordDifference DiffType) { - return ODRDiagDeclError(FirstTemplate->getLocation(), + return DiagError(FirstTemplate->getLocation(), FirstTemplate->getSourceRange(), DiffType) << FirstTemplate; }; - auto DiagTemplateNote = [&ODRDiagDeclNote, SecondTemplate]( + auto DiagTemplateNote = [&DiagNote, SecondTemplate]( ODRCXXRecordDifference DiffType) { - return ODRDiagDeclNote(SecondTemplate->getLocation(), + return DiagNote(SecondTemplate->getLocation(), SecondTemplate->getSourceRange(), DiffType) << SecondTemplate; }; @@ -10732,11 +10799,9 @@ << FirstTPL->size(); DiagTemplateNote(FunctionTemplateDifferentNumberParameters) << SecondTPL->size(); - Diagnosed = true; - break; + return true; } - bool ParameterMismatch = false; for (unsigned i = 0, e = FirstTPL->size(); i != e; ++i) { NamedDecl *FirstParam = FirstTPL->getParam(i); NamedDecl *SecondParam = SecondTPL->getParam(i); @@ -10764,8 +10829,7 @@ << (i + 1) << GetParamType(FirstParam); DiagTemplateNote(FunctionTemplateParameterDifferentKind) << (i + 1) << GetParamType(SecondParam); - ParameterMismatch = true; - break; + return true; } if (FirstParam->getName() != SecondParam->getName()) { @@ -10773,8 +10837,7 @@ << (i + 1) << (bool)FirstParam->getIdentifier() << FirstParam; DiagTemplateNote(FunctionTemplateParameterName) << (i + 1) << (bool)SecondParam->getIdentifier() << SecondParam; - ParameterMismatch = true; - break; + return true; } if (isa(FirstParam) && @@ -10794,8 +10857,7 @@ << (i + 1) << HasFirstDefaultArgument; DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument) << (i + 1) << HasSecondDefaultArgument; - ParameterMismatch = true; - break; + return true; } if (HasFirstDefaultArgument && HasSecondDefaultArgument) { @@ -10808,8 +10870,7 @@ DiagTemplateNote( FunctionTemplateParameterDifferentDefaultArgument) << (i + 1) << SecondType; - ParameterMismatch = true; - break; + return true; } } @@ -10819,8 +10880,7 @@ << (i + 1) << FirstTTPD->isParameterPack(); DiagTemplateNote(FunctionTemplatePackParameter) << (i + 1) << SecondTTPD->isParameterPack(); - ParameterMismatch = true; - break; + return true; } } @@ -10850,8 +10910,7 @@ << (i + 1); DiagTemplateNote(FunctionTemplateParameterDifferentType) << (i + 1); - ParameterMismatch = true; - break; + return true; } bool HasFirstDefaultArgument = @@ -10865,8 +10924,7 @@ << (i + 1) << HasFirstDefaultArgument; DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument) << (i + 1) << HasSecondDefaultArgument; - ParameterMismatch = true; - break; + return true; } if (HasFirstDefaultArgument && HasSecondDefaultArgument) { @@ -10881,8 +10939,7 @@ DiagTemplateNote( FunctionTemplateParameterDifferentDefaultArgument) << (i + 1) << SecondTA; - ParameterMismatch = true; - break; + return true; } } @@ -10892,8 +10949,7 @@ << (i + 1) << FirstTTPD->isParameterPack(); DiagTemplateNote(FunctionTemplatePackParameter) << (i + 1) << SecondTTPD->isParameterPack(); - ParameterMismatch = true; - break; + return true; } } @@ -10911,8 +10967,7 @@ << (i + 1); DiagTemplateNote(FunctionTemplateParameterDifferentType) << (i + 1); - ParameterMismatch = true; - break; + return true; } bool HasFirstDefaultArgument = @@ -10926,8 +10981,7 @@ << (i + 1) << HasFirstDefaultArgument; DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument) << (i + 1) << HasSecondDefaultArgument; - ParameterMismatch = true; - break; + return true; } if (HasFirstDefaultArgument && HasSecondDefaultArgument) { @@ -10941,8 +10995,7 @@ DiagTemplateNote( FunctionTemplateParameterDifferentDefaultArgument) << (i + 1) << SecondDefaultArgument; - ParameterMismatch = true; - break; + return true; } } @@ -10952,24 +11005,14 @@ << (i + 1) << FirstNTTPD->isParameterPack(); DiagTemplateNote(FunctionTemplatePackParameter) << (i + 1) << SecondNTTPD->isParameterPack(); - ParameterMismatch = true; - break; + return true; } } } - - if (ParameterMismatch) { - Diagnosed = true; - break; - } - break; } } - if (Diagnosed) - continue; - Diag(FirstDecl->getLocation(), diag::err_module_odr_violation_mismatch_decl_unknown) << FirstRecord << FirstModule.empty() << FirstModule << FirstDiffType @@ -10977,24 +11020,16 @@ Diag(SecondDecl->getLocation(), diag::note_module_odr_violation_mismatch_decl_unknown) << SecondModule << FirstDiffType << SecondDecl->getSourceRange(); - Diagnosed = true; + return true; } - if (!Diagnosed) { - // All definitions are updates to the same declaration. This happens if a - // module instantiates the declaration of a class template specialization - // and two or more other modules instantiate its definition. - // - // FIXME: Indicate which modules had instantiations of this definition. - // FIXME: How can this even happen? - Diag(Merge.first->getLocation(), - diag::err_module_odr_violation_different_instantiations) - << Merge.first; - } - } + bool ODRDiagsEmitter::diagnoseMismatch( + const FunctionDecl *FirstFunction, + const FunctionDecl *SecondFunction) const { + if (FirstFunction == SecondFunction) + return false; - // Issue ODR failures diagnostics for functions. - for (auto &Merge : FunctionOdrMergeFailures) { + // Keep in sync with select options in err_module_odr_violation_function. enum ODRFunctionDifference { ReturnType, ParameterName, @@ -11004,26 +11039,17 @@ FunctionBody, }; - FunctionDecl *FirstFunction = Merge.first; std::string FirstModule = getOwningModuleNameForDiagnostic(FirstFunction); + std::string SecondModule = getOwningModuleNameForDiagnostic(SecondFunction); - bool Diagnosed = false; - for (auto &SecondFunction : Merge.second) { - - if (FirstFunction == SecondFunction) - continue; - - std::string SecondModule = - getOwningModuleNameForDiagnostic(SecondFunction); - - auto ODRDiagError = [FirstFunction, &FirstModule, + auto DiagError = [FirstFunction, &FirstModule, this](SourceLocation Loc, SourceRange Range, ODRFunctionDifference DiffType) { return Diag(Loc, diag::err_module_odr_violation_function) << FirstFunction << FirstModule.empty() << FirstModule << Range << DiffType; }; - auto ODRDiagNote = [&SecondModule, this](SourceLocation Loc, + auto DiagNote = [&SecondModule, this](SourceLocation Loc, SourceRange Range, ODRFunctionDifference DiffType) { return Diag(Loc, diag::note_module_odr_violation_function) @@ -11032,38 +11058,35 @@ if (computeODRHash(FirstFunction->getReturnType()) != computeODRHash(SecondFunction->getReturnType())) { - ODRDiagError(FirstFunction->getReturnTypeSourceRange().getBegin(), + DiagError(FirstFunction->getReturnTypeSourceRange().getBegin(), FirstFunction->getReturnTypeSourceRange(), ReturnType) << FirstFunction->getReturnType(); - ODRDiagNote(SecondFunction->getReturnTypeSourceRange().getBegin(), + DiagNote(SecondFunction->getReturnTypeSourceRange().getBegin(), SecondFunction->getReturnTypeSourceRange(), ReturnType) << SecondFunction->getReturnType(); - Diagnosed = true; - break; + return true; } assert(FirstFunction->param_size() == SecondFunction->param_size() && "Merged functions with different number of parameters"); size_t ParamSize = FirstFunction->param_size(); - bool ParameterMismatch = false; for (unsigned I = 0; I < ParamSize; ++I) { const ParmVarDecl *FirstParam = FirstFunction->getParamDecl(I); const ParmVarDecl *SecondParam = SecondFunction->getParamDecl(I); - assert(getContext().hasSameType(FirstParam->getType(), + assert(Context.hasSameType(FirstParam->getType(), SecondParam->getType()) && "Merged function has different parameter types."); if (FirstParam->getDeclName() != SecondParam->getDeclName()) { - ODRDiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), + DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), ParameterName) << I + 1 << FirstParam->getDeclName(); - ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), + DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), ParameterName) << I + 1 << SecondParam->getDeclName(); - ParameterMismatch = true; - break; + return true; }; QualType FirstParamType = FirstParam->getType(); @@ -11072,82 +11095,74 @@ computeODRHash(FirstParamType) != computeODRHash(SecondParamType)) { if (const DecayedType *ParamDecayedType = FirstParamType->getAs()) { - ODRDiagError(FirstParam->getLocation(), + DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), ParameterType) << (I + 1) << FirstParamType << true << ParamDecayedType->getOriginalType(); } else { - ODRDiagError(FirstParam->getLocation(), + DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), ParameterType) << (I + 1) << FirstParamType << false; } if (const DecayedType *ParamDecayedType = SecondParamType->getAs()) { - ODRDiagNote(SecondParam->getLocation(), + DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), ParameterType) << (I + 1) << SecondParamType << true << ParamDecayedType->getOriginalType(); } else { - ODRDiagNote(SecondParam->getLocation(), + DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), ParameterType) << (I + 1) << SecondParamType << false; } - ParameterMismatch = true; - break; + return true; } const Expr *FirstInit = FirstParam->getInit(); const Expr *SecondInit = SecondParam->getInit(); if ((FirstInit == nullptr) != (SecondInit == nullptr)) { - ODRDiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), + DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), ParameterSingleDefaultArgument) << (I + 1) << (FirstInit == nullptr) << (FirstInit ? FirstInit->getSourceRange() : SourceRange()); - ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), + DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), ParameterSingleDefaultArgument) << (I + 1) << (SecondInit == nullptr) << (SecondInit ? SecondInit->getSourceRange() : SourceRange()); - ParameterMismatch = true; - break; + return true; } if (FirstInit && SecondInit && computeODRHash(FirstInit) != computeODRHash(SecondInit)) { - ODRDiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), + DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), ParameterDifferentDefaultArgument) << (I + 1) << FirstInit->getSourceRange(); - ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), + DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), ParameterDifferentDefaultArgument) << (I + 1) << SecondInit->getSourceRange(); - ParameterMismatch = true; - break; + return true; } assert(computeODRHash(FirstParam) == computeODRHash(SecondParam) && "Undiagnosed parameter difference."); } - if (ParameterMismatch) { - Diagnosed = true; - break; - } - // If no error has been generated before now, assume the problem is in // the body and generate a message. - ODRDiagError(FirstFunction->getLocation(), + DiagError(FirstFunction->getLocation(), FirstFunction->getSourceRange(), FunctionBody); - ODRDiagNote(SecondFunction->getLocation(), + DiagNote(SecondFunction->getLocation(), SecondFunction->getSourceRange(), FunctionBody); - Diagnosed = true; - break; - } - (void)Diagnosed; - assert(Diagnosed && "Unable to emit ODR diagnostic."); + return true; } - // Issue ODR failures diagnostics for enums. - for (auto &Merge : EnumOdrMergeFailures) { + bool ODRDiagsEmitter::diagnoseMismatch(const EnumDecl *FirstEnum, + const EnumDecl *SecondEnum) const { + if (FirstEnum == SecondEnum) + return false; + + // Keep in sync with select options in err_module_odr_violation_enum. enum ODREnumDifference { SingleScopedEnum, EnumTagKeywordMismatch, @@ -11159,38 +11174,10 @@ EnumConstantDifferentInitializer, }; - // If we've already pointed out a specific problem with this enum, don't - // bother issuing a general "something's different" diagnostic. - if (!DiagnosedOdrMergeFailures.insert(Merge.first).second) - continue; - - EnumDecl *FirstEnum = Merge.first; std::string FirstModule = getOwningModuleNameForDiagnostic(FirstEnum); + std::string SecondModule = getOwningModuleNameForDiagnostic(SecondEnum); - using DeclHashes = - llvm::SmallVector, 4>; - auto PopulateHashes = [FirstEnum](DeclHashes &Hashes, EnumDecl *Enum) { - for (auto *D : Enum->decls()) { - // Due to decl merging, the first EnumDecl is the parent of - // Decls in both records. - if (!ODRHash::isDeclToBeProcessed(D, FirstEnum)) - continue; - assert(isa(D) && "Unexpected Decl kind"); - Hashes.emplace_back(cast(D), computeODRHash(D)); - } - }; - DeclHashes FirstHashes; - PopulateHashes(FirstHashes, FirstEnum); - bool Diagnosed = false; - for (auto &SecondEnum : Merge.second) { - - if (FirstEnum == SecondEnum) - continue; - - std::string SecondModule = - getOwningModuleNameForDiagnostic(SecondEnum); - - auto ODRDiagError = [FirstEnum, &FirstModule, + auto DiagError = [FirstEnum, &FirstModule, this](const auto *DiagAnchor, ODREnumDifference DiffType) { return Diag(DiagAnchor->getLocation(), @@ -11198,7 +11185,7 @@ << FirstEnum << FirstModule.empty() << FirstModule << DiagAnchor->getSourceRange() << DiffType; }; - auto ODRDiagNote = [&SecondModule, this](const auto *DiagAnchor, + auto DiagNote = [&SecondModule, this](const auto *DiagAnchor, ODREnumDifference DiffType) { return Diag(DiagAnchor->getLocation(), diag::note_module_odr_violation_enum) @@ -11206,21 +11193,19 @@ }; if (FirstEnum->isScoped() != SecondEnum->isScoped()) { - ODRDiagError(FirstEnum, SingleScopedEnum) << FirstEnum->isScoped(); - ODRDiagNote(SecondEnum, SingleScopedEnum) << SecondEnum->isScoped(); - Diagnosed = true; - continue; + DiagError(FirstEnum, SingleScopedEnum) << FirstEnum->isScoped(); + DiagNote(SecondEnum, SingleScopedEnum) << SecondEnum->isScoped(); + return true; } if (FirstEnum->isScoped() && SecondEnum->isScoped()) { if (FirstEnum->isScopedUsingClassTag() != SecondEnum->isScopedUsingClassTag()) { - ODRDiagError(FirstEnum, EnumTagKeywordMismatch) + DiagError(FirstEnum, EnumTagKeywordMismatch) << FirstEnum->isScopedUsingClassTag(); - ODRDiagNote(SecondEnum, EnumTagKeywordMismatch) + DiagNote(SecondEnum, EnumTagKeywordMismatch) << SecondEnum->isScopedUsingClassTag(); - Diagnosed = true; - continue; + return true; } } @@ -11233,52 +11218,62 @@ ? SecondEnum->getIntegerTypeSourceInfo()->getType() : QualType(); if (FirstUnderlyingType.isNull() != SecondUnderlyingType.isNull()) { - ODRDiagError(FirstEnum, SingleSpecifiedType) + DiagError(FirstEnum, SingleSpecifiedType) << !FirstUnderlyingType.isNull(); - ODRDiagNote(SecondEnum, SingleSpecifiedType) + DiagNote(SecondEnum, SingleSpecifiedType) << !SecondUnderlyingType.isNull(); - Diagnosed = true; - continue; + return true; } if (!FirstUnderlyingType.isNull() && !SecondUnderlyingType.isNull()) { if (computeODRHash(FirstUnderlyingType) != computeODRHash(SecondUnderlyingType)) { - ODRDiagError(FirstEnum, DifferentSpecifiedTypes) + DiagError(FirstEnum, DifferentSpecifiedTypes) << FirstUnderlyingType; - ODRDiagNote(SecondEnum, DifferentSpecifiedTypes) + DiagNote(SecondEnum, DifferentSpecifiedTypes) << SecondUnderlyingType; - Diagnosed = true; - continue; + return true; } } + // Compare enum constants. + using DeclHashes = + llvm::SmallVector, 4>; + auto PopulateHashes = [FirstEnum](DeclHashes &Hashes, const EnumDecl *Enum) { + for (const Decl *D : Enum->decls()) { + // Due to decl merging, the first EnumDecl is the parent of + // Decls in both records. + if (!ODRHash::isDeclToBeProcessed(D, FirstEnum)) + continue; + assert(isa(D) && "Unexpected Decl kind"); + Hashes.emplace_back(cast(D), computeODRHash(D)); + } + }; + DeclHashes FirstHashes; + PopulateHashes(FirstHashes, FirstEnum); DeclHashes SecondHashes; PopulateHashes(SecondHashes, SecondEnum); if (FirstHashes.size() != SecondHashes.size()) { - ODRDiagError(FirstEnum, DifferentNumberEnumConstants) + DiagError(FirstEnum, DifferentNumberEnumConstants) << (int)FirstHashes.size(); - ODRDiagNote(SecondEnum, DifferentNumberEnumConstants) + DiagNote(SecondEnum, DifferentNumberEnumConstants) << (int)SecondHashes.size(); - Diagnosed = true; - continue; + return true; } - for (unsigned I = 0; I < FirstHashes.size(); ++I) { + for (unsigned I = 0, N = FirstHashes.size(); I < N; ++I) { if (FirstHashes[I].second == SecondHashes[I].second) continue; const EnumConstantDecl *FirstConstant = FirstHashes[I].first; const EnumConstantDecl *SecondConstant = SecondHashes[I].first; if (FirstConstant->getDeclName() != SecondConstant->getDeclName()) { - - ODRDiagError(FirstConstant, EnumConstantName) + DiagError(FirstConstant, EnumConstantName) << I + 1 << FirstConstant; - ODRDiagNote(SecondConstant, EnumConstantName) + DiagNote(SecondConstant, EnumConstantName) << I + 1 << SecondConstant; - Diagnosed = true; - break; + return true; } const Expr *FirstInit = FirstConstant->getInitExpr(); @@ -11287,29 +11282,24 @@ continue; if (!FirstInit || !SecondInit) { - ODRDiagError(FirstConstant, EnumConstantSingleInitializer) + DiagError(FirstConstant, EnumConstantSingleInitializer) << I + 1 << FirstConstant << (FirstInit != nullptr); - ODRDiagNote(SecondConstant, EnumConstantSingleInitializer) + DiagNote(SecondConstant, EnumConstantSingleInitializer) << I + 1 << SecondConstant << (SecondInit != nullptr); - Diagnosed = true; - break; + return true; } if (computeODRHash(FirstInit) != computeODRHash(SecondInit)) { - ODRDiagError(FirstConstant, EnumConstantDifferentInitializer) + DiagError(FirstConstant, EnumConstantDifferentInitializer) << I + 1 << FirstConstant; - ODRDiagNote(SecondConstant, EnumConstantDifferentInitializer) + DiagNote(SecondConstant, EnumConstantDifferentInitializer) << I + 1 << SecondConstant; - Diagnosed = true; - break; - } + return true; } } - - (void)Diagnosed; - assert(Diagnosed && "Unable to emit ODR diagnostic."); - } + return false; } +// clang-format on void ASTReader::StartedDeserializing() { if (++NumCurrentElementsDeserializing == 1 && ReadTimer.get())