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/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9445,6 +9445,110 @@ 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; + +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); @@ -9470,6 +9574,15 @@ return Hasher.CalculateHash(); } +static std::string 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() && @@ -9609,32 +9722,80 @@ // we're producing our diagnostics. Deserializing RecursionGuard(this); - // 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; + + bool Diagnosed = false; + ODRDiagsEmitter DiagsEmitter(Diags, getContext(), + getPreprocessor().getLangOpts()); + CXXRecordDecl *FirstRecord = Merge.first; + for (auto &RecordPair : Merge.second) { + if (DiagsEmitter.diagnoseMismatch(FirstRecord, RecordPair.first, + RecordPair.second)) { + Diagnosed = true; + break; + } + } - // 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) { + 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) { + ODRDiagsEmitter DiagsEmitter(Diags, getContext(), + getPreprocessor().getLangOpts()); + 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; + + ODRDiagsEmitter DiagsEmitter(Diags, getContext(), + getPreprocessor().getLangOpts()); + 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, @@ -9667,8 +9828,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(); @@ -9698,7 +9858,7 @@ } } - if (!PP.getLangOpts().CPlusPlus) + if (!LangOpts.CPlusPlus) return false; const bool IsFirstMutable = FirstField->isMutable(); @@ -9733,12 +9893,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, @@ -9773,13 +9933,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, @@ -9817,7 +9978,7 @@ return true; } - if (!PP.getLangOpts().CPlusPlus) + if (!LangOpts.CPlusPlus) return false; const Expr *FirstInit = FirstVD->getInit(); @@ -9849,28 +10010,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: @@ -9930,15 +10075,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; @@ -9956,12 +10097,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; @@ -9986,34 +10126,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, @@ -10021,20 +10153,20 @@ BaseVirtual, BaseAccess, }; - auto ODRDiagBaseError = [FirstRecord, &FirstModule, - this](SourceLocation Loc, SourceRange Range, - ODRDefinitionDataDifference DiffType) { + 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, - this](SourceLocation Loc, SourceRange Range, - ODRDefinitionDataDifference DiffType) { + 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(); @@ -10047,72 +10179,64 @@ unsigned SecondNumBases = SecondDD->NumBases; unsigned SecondNumVBases = SecondDD->NumVBases; if (FirstNumBases != SecondNumBases) { - ODRDiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD), - NumBases) + DiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD), + NumBases) << FirstNumBases; - ODRDiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD), - NumBases) + DiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD), + NumBases) << SecondNumBases; - Diagnosed = true; - break; + return true; } if (FirstNumVBases != SecondNumVBases) { - ODRDiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD), - NumVBases) + DiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD), + NumVBases) << FirstNumVBases; - ODRDiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD), - NumVBases) + 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(), - FirstBase.getSourceRange(), BaseType) + DiagBaseError(FirstRecord->getLocation(), + FirstBase.getSourceRange(), BaseType) << (I + 1) << FirstBase.getType(); - ODRDiagBaseNote(SecondRecord->getLocation(), - SecondBase.getSourceRange(), BaseType) + DiagBaseNote(SecondRecord->getLocation(), + SecondBase.getSourceRange(), BaseType) << (I + 1) << SecondBase.getType(); - break; + return true; } if (FirstBase.isVirtual() != SecondBase.isVirtual()) { - ODRDiagBaseError(FirstRecord->getLocation(), - FirstBase.getSourceRange(), BaseVirtual) + DiagBaseError(FirstRecord->getLocation(), + FirstBase.getSourceRange(), BaseVirtual) << (I + 1) << FirstBase.isVirtual() << FirstBase.getType(); - ODRDiagBaseNote(SecondRecord->getLocation(), - SecondBase.getSourceRange(), BaseVirtual) + DiagBaseNote(SecondRecord->getLocation(), + SecondBase.getSourceRange(), BaseVirtual) << (I + 1) << SecondBase.isVirtual() << SecondBase.getType(); - break; + return true; } if (FirstBase.getAccessSpecifierAsWritten() != SecondBase.getAccessSpecifierAsWritten()) { - ODRDiagBaseError(FirstRecord->getLocation(), - FirstBase.getSourceRange(), BaseAccess) + DiagBaseError(FirstRecord->getLocation(), + FirstBase.getSourceRange(), BaseAccess) << (I + 1) << FirstBase.getType() << (int)FirstBase.getAccessSpecifierAsWritten(); - ODRDiagBaseNote(SecondRecord->getLocation(), - SecondBase.getSourceRange(), BaseAccess) + 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 = @@ -10124,32 +10248,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."); @@ -10201,14 +10311,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; @@ -10223,17 +10337,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 @@ -10271,16 +10383,16 @@ FunctionTemplateParameterDifferentType, FunctionTemplatePackParameter, }; - auto ODRDiagDeclError = [FirstRecord, &FirstModule, - this](SourceLocation Loc, SourceRange Range, - ODRCXXRecordDifference DiffType) { + 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, - this](SourceLocation Loc, SourceRange Range, - ODRCXXRecordDifference DiffType) { + auto DiagNote = [&SecondModule, + this](SourceLocation Loc, SourceRange Range, + ODRCXXRecordDifference DiffType) { return Diag(Loc, diag::note_module_odr_violation_record) << SecondModule << Range << DiffType; }; @@ -10303,12 +10415,11 @@ unsigned FirstODRHash = computeODRHash(FirstExpr); unsigned SecondODRHash = computeODRHash(SecondExpr); if (FirstODRHash != SecondODRHash) { - ODRDiagDeclError(FirstExpr->getBeginLoc(), - FirstExpr->getSourceRange(), StaticAssertCondition); - ODRDiagDeclNote(SecondExpr->getBeginLoc(), - SecondExpr->getSourceRange(), StaticAssertCondition); - Diagnosed = true; - break; + DiagError(FirstExpr->getBeginLoc(), + FirstExpr->getSourceRange(), StaticAssertCondition); + DiagNote(SecondExpr->getBeginLoc(), + SecondExpr->getSourceRange(), StaticAssertCondition); + return true; } const StringLiteral *FirstStr = FirstSA->getMessage(); @@ -10331,29 +10442,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(), - StaticAssertMessage); - ODRDiagDeclNote(SecondStr->getBeginLoc(), SecondStr->getSourceRange(), - StaticAssertMessage); - Diagnosed = true; - break; + DiagError(FirstStr->getBeginLoc(), FirstStr->getSourceRange(), + StaticAssertMessage); + DiagNote(SecondStr->getBeginLoc(), SecondStr->getSourceRange(), + StaticAssertMessage); + 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: { @@ -10374,24 +10484,23 @@ 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(), - FirstMethod->getSourceRange(), DiffType) + 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(), - SecondMethod->getSourceRange(), DiffType) + return DiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), DiffType) << SecondMethodType << SecondName; }; if (FirstMethodType != SecondMethodType || FirstName != SecondName) { DiagMethodError(MethodName); DiagMethodNote(MethodName); - Diagnosed = true; - break; + return true; } const bool FirstDeleted = FirstMethod->isDeletedAsWritten(); @@ -10399,8 +10508,7 @@ if (FirstDeleted != SecondDeleted) { DiagMethodError(MethodDeleted) << FirstDeleted; DiagMethodNote(MethodDeleted) << SecondDeleted; - Diagnosed = true; - break; + return true; } const bool FirstDefaulted = FirstMethod->isExplicitlyDefaulted(); @@ -10408,8 +10516,7 @@ if (FirstDefaulted != SecondDefaulted) { DiagMethodError(MethodDefaulted) << FirstDefaulted; DiagMethodNote(MethodDefaulted) << SecondDefaulted; - Diagnosed = true; - break; + return true; } const bool FirstVirtual = FirstMethod->isVirtualAsWritten(); @@ -10420,8 +10527,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, @@ -10434,8 +10540,7 @@ if (FirstStatic != SecondStatic) { DiagMethodError(MethodStatic) << FirstStatic; DiagMethodNote(MethodStatic) << SecondStatic; - Diagnosed = true; - break; + return true; } const bool FirstVolatile = FirstMethod->isVolatile(); @@ -10443,8 +10548,7 @@ if (FirstVolatile != SecondVolatile) { DiagMethodError(MethodVolatile) << FirstVolatile; DiagMethodNote(MethodVolatile) << SecondVolatile; - Diagnosed = true; - break; + return true; } const bool FirstConst = FirstMethod->isConst(); @@ -10452,8 +10556,7 @@ if (FirstConst != SecondConst) { DiagMethodError(MethodConst) << FirstConst; DiagMethodNote(MethodConst) << SecondConst; - Diagnosed = true; - break; + return true; } const bool FirstInline = FirstMethod->isInlineSpecified(); @@ -10461,8 +10564,7 @@ if (FirstInline != SecondInline) { DiagMethodError(MethodInline) << FirstInline; DiagMethodNote(MethodInline) << SecondInline; - Diagnosed = true; - break; + return true; } const unsigned FirstNumParameters = FirstMethod->param_size(); @@ -10470,12 +10572,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); @@ -10504,8 +10603,7 @@ DiagMethodNote(MethodParameterType) << (I + 1) << SecondParamType << false; } - ParameterMismatch = true; - break; + return true; } DeclarationName FirstParamName = FirstParam->getDeclName(); @@ -10513,8 +10611,7 @@ if (FirstParamName != SecondParamName) { DiagMethodError(MethodParameterName) << (I + 1) << FirstParamName; DiagMethodNote(MethodParameterName) << (I + 1) << SecondParamName; - ParameterMismatch = true; - break; + return true; } const Expr *FirstInit = FirstParam->getInit(); @@ -10526,8 +10623,7 @@ DiagMethodNote(MethodParameterSingleDefaultArgument) << (I + 1) << (SecondInit == nullptr) << (SecondInit ? SecondInit->getSourceRange() : SourceRange()); - ParameterMismatch = true; - break; + return true; } if (FirstInit && SecondInit && @@ -10536,16 +10632,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 = @@ -10557,8 +10647,7 @@ << (FirstTemplateArgs != nullptr); DiagMethodNote(MethodNoTemplateArguments) << (SecondTemplateArgs != nullptr); - Diagnosed = true; - break; + return true; } if (FirstTemplateArgs && SecondTemplateArgs) { @@ -10586,11 +10675,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]; @@ -10602,13 +10689,7 @@ << FirstTA << i + 1; DiagMethodNote(MethodDifferentTemplateArgument) << SecondTA << i + 1; - TemplateArgumentMismatch = true; - break; - } - - if (TemplateArgumentMismatch) { - Diagnosed = true; - break; + return true; } } @@ -10630,31 +10711,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: { @@ -10668,14 +10750,13 @@ TypeSourceInfo *SecondTSI = SecondFriend->getFriendType(); if (FirstND && SecondND) { - ODRDiagDeclError(FirstFriend->getFriendLoc(), - FirstFriend->getSourceRange(), FriendFunction) + DiagError(FirstFriend->getFriendLoc(), + FirstFriend->getSourceRange(), FriendFunction) << FirstND; - ODRDiagDeclNote(SecondFriend->getFriendLoc(), - SecondFriend->getSourceRange(), FriendFunction) + DiagNote(SecondFriend->getFriendLoc(), + SecondFriend->getSourceRange(), FriendFunction) << SecondND; - Diagnosed = true; - break; + return true; } if (FirstTSI && SecondTSI) { @@ -10683,24 +10764,22 @@ QualType SecondFriendType = SecondTSI->getType(); assert(computeODRHash(FirstFriendType) != computeODRHash(SecondFriendType)); - ODRDiagDeclError(FirstFriend->getFriendLoc(), - FirstFriend->getSourceRange(), FriendType) + DiagError(FirstFriend->getFriendLoc(), + FirstFriend->getSourceRange(), FriendType) << FirstFriendType; - ODRDiagDeclNote(SecondFriend->getFriendLoc(), - SecondFriend->getSourceRange(), FriendType) + DiagNote(SecondFriend->getFriendLoc(), + SecondFriend->getSourceRange(), FriendType) << SecondFriendType; - Diagnosed = true; - break; + return true; } - ODRDiagDeclError(FirstFriend->getFriendLoc(), - FirstFriend->getSourceRange(), FriendTypeFunction) + DiagError(FirstFriend->getFriendLoc(), + FirstFriend->getSourceRange(), FriendTypeFunction) << (FirstTSI == nullptr); - ODRDiagDeclNote(SecondFriend->getFriendLoc(), - SecondFriend->getSourceRange(), FriendTypeFunction) + DiagNote(SecondFriend->getFriendLoc(), + SecondFriend->getSourceRange(), FriendTypeFunction) << (SecondTSI == nullptr); - Diagnosed = true; - break; + return true; } case FunctionTemplate: { const FunctionTemplateDecl *FirstTemplate = @@ -10713,16 +10792,16 @@ TemplateParameterList *SecondTPL = SecondTemplate->getTemplateParameters(); - auto DiagTemplateError = [&ODRDiagDeclError, FirstTemplate]( + auto DiagTemplateError = [&DiagError, FirstTemplate]( ODRCXXRecordDifference DiffType) { - return ODRDiagDeclError(FirstTemplate->getLocation(), - FirstTemplate->getSourceRange(), DiffType) + return DiagError(FirstTemplate->getLocation(), + FirstTemplate->getSourceRange(), DiffType) << FirstTemplate; }; - auto DiagTemplateNote = [&ODRDiagDeclNote, SecondTemplate]( + auto DiagTemplateNote = [&DiagNote, SecondTemplate]( ODRCXXRecordDifference DiffType) { - return ODRDiagDeclNote(SecondTemplate->getLocation(), - SecondTemplate->getSourceRange(), DiffType) + return DiagNote(SecondTemplate->getLocation(), + SecondTemplate->getSourceRange(), DiffType) << SecondTemplate; }; @@ -10731,11 +10810,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); @@ -10763,8 +10840,7 @@ << (i + 1) << GetParamType(FirstParam); DiagTemplateNote(FunctionTemplateParameterDifferentKind) << (i + 1) << GetParamType(SecondParam); - ParameterMismatch = true; - break; + return true; } if (FirstParam->getName() != SecondParam->getName()) { @@ -10772,8 +10848,7 @@ << (i + 1) << (bool)FirstParam->getIdentifier() << FirstParam; DiagTemplateNote(FunctionTemplateParameterName) << (i + 1) << (bool)SecondParam->getIdentifier() << SecondParam; - ParameterMismatch = true; - break; + return true; } if (isa(FirstParam) && @@ -10793,8 +10868,7 @@ << (i + 1) << HasFirstDefaultArgument; DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument) << (i + 1) << HasSecondDefaultArgument; - ParameterMismatch = true; - break; + return true; } if (HasFirstDefaultArgument && HasSecondDefaultArgument) { @@ -10807,8 +10881,7 @@ DiagTemplateNote( FunctionTemplateParameterDifferentDefaultArgument) << (i + 1) << SecondType; - ParameterMismatch = true; - break; + return true; } } @@ -10818,8 +10891,7 @@ << (i + 1) << FirstTTPD->isParameterPack(); DiagTemplateNote(FunctionTemplatePackParameter) << (i + 1) << SecondTTPD->isParameterPack(); - ParameterMismatch = true; - break; + return true; } } @@ -10849,8 +10921,7 @@ << (i + 1); DiagTemplateNote(FunctionTemplateParameterDifferentType) << (i + 1); - ParameterMismatch = true; - break; + return true; } bool HasFirstDefaultArgument = @@ -10864,8 +10935,7 @@ << (i + 1) << HasFirstDefaultArgument; DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument) << (i + 1) << HasSecondDefaultArgument; - ParameterMismatch = true; - break; + return true; } if (HasFirstDefaultArgument && HasSecondDefaultArgument) { @@ -10880,8 +10950,7 @@ DiagTemplateNote( FunctionTemplateParameterDifferentDefaultArgument) << (i + 1) << SecondTA; - ParameterMismatch = true; - break; + return true; } } @@ -10891,8 +10960,7 @@ << (i + 1) << FirstTTPD->isParameterPack(); DiagTemplateNote(FunctionTemplatePackParameter) << (i + 1) << SecondTTPD->isParameterPack(); - ParameterMismatch = true; - break; + return true; } } @@ -10910,8 +10978,7 @@ << (i + 1); DiagTemplateNote(FunctionTemplateParameterDifferentType) << (i + 1); - ParameterMismatch = true; - break; + return true; } bool HasFirstDefaultArgument = @@ -10925,8 +10992,7 @@ << (i + 1) << HasFirstDefaultArgument; DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument) << (i + 1) << HasSecondDefaultArgument; - ParameterMismatch = true; - break; + return true; } if (HasFirstDefaultArgument && HasSecondDefaultArgument) { @@ -10940,8 +11006,7 @@ DiagTemplateNote( FunctionTemplateParameterDifferentDefaultArgument) << (i + 1) << SecondDefaultArgument; - ParameterMismatch = true; - break; + return true; } } @@ -10951,24 +11016,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 @@ -10976,24 +11031,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, @@ -11003,66 +11050,54 @@ 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, - this](SourceLocation Loc, SourceRange Range, - ODRFunctionDifference DiffType) { + 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, - SourceRange Range, - ODRFunctionDifference DiffType) { + auto DiagNote = [&SecondModule, this](SourceLocation Loc, + SourceRange Range, + ODRFunctionDifference DiffType) { return Diag(Loc, diag::note_module_odr_violation_function) << SecondModule << Range << DiffType; }; if (computeODRHash(FirstFunction->getReturnType()) != computeODRHash(SecondFunction->getReturnType())) { - ODRDiagError(FirstFunction->getReturnTypeSourceRange().getBegin(), - FirstFunction->getReturnTypeSourceRange(), ReturnType) + DiagError(FirstFunction->getReturnTypeSourceRange().getBegin(), + FirstFunction->getReturnTypeSourceRange(), ReturnType) << FirstFunction->getReturnType(); - ODRDiagNote(SecondFunction->getReturnTypeSourceRange().getBegin(), - SecondFunction->getReturnTypeSourceRange(), ReturnType) + 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(), - ParameterName) + DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), + ParameterName) << I + 1 << FirstParam->getDeclName(); - ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), - ParameterName) + DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), + ParameterName) << I + 1 << SecondParam->getDeclName(); - ParameterMismatch = true; - break; + return true; }; QualType FirstParamType = FirstParam->getType(); @@ -11071,82 +11106,74 @@ computeODRHash(FirstParamType) != computeODRHash(SecondParamType)) { if (const DecayedType *ParamDecayedType = FirstParamType->getAs()) { - ODRDiagError(FirstParam->getLocation(), - FirstParam->getSourceRange(), ParameterType) + DiagError(FirstParam->getLocation(), + FirstParam->getSourceRange(), ParameterType) << (I + 1) << FirstParamType << true << ParamDecayedType->getOriginalType(); } else { - ODRDiagError(FirstParam->getLocation(), - FirstParam->getSourceRange(), ParameterType) + DiagError(FirstParam->getLocation(), + FirstParam->getSourceRange(), ParameterType) << (I + 1) << FirstParamType << false; } if (const DecayedType *ParamDecayedType = SecondParamType->getAs()) { - ODRDiagNote(SecondParam->getLocation(), - SecondParam->getSourceRange(), ParameterType) + DiagNote(SecondParam->getLocation(), + SecondParam->getSourceRange(), ParameterType) << (I + 1) << SecondParamType << true << ParamDecayedType->getOriginalType(); } else { - ODRDiagNote(SecondParam->getLocation(), - SecondParam->getSourceRange(), ParameterType) + 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(), - ParameterSingleDefaultArgument) + DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), + ParameterSingleDefaultArgument) << (I + 1) << (FirstInit == nullptr) << (FirstInit ? FirstInit->getSourceRange() : SourceRange()); - ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), - ParameterSingleDefaultArgument) + 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(), - ParameterDifferentDefaultArgument) + DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(), + ParameterDifferentDefaultArgument) << (I + 1) << FirstInit->getSourceRange(); - ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(), - ParameterDifferentDefaultArgument) + 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(), - FirstFunction->getSourceRange(), FunctionBody); - ODRDiagNote(SecondFunction->getLocation(), - SecondFunction->getSourceRange(), FunctionBody); - Diagnosed = true; - break; - } - (void)Diagnosed; - assert(Diagnosed && "Unable to emit ODR diagnostic."); + DiagError(FirstFunction->getLocation(), + FirstFunction->getSourceRange(), FunctionBody); + DiagNote(SecondFunction->getLocation(), + SecondFunction->getSourceRange(), FunctionBody); + 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, @@ -11158,68 +11185,38 @@ 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, - this](const auto *DiagAnchor, - ODREnumDifference DiffType) { + auto DiagError = [FirstEnum, &FirstModule, + this](const auto *DiagAnchor, + ODREnumDifference DiffType) { return Diag(DiagAnchor->getLocation(), diag::err_module_odr_violation_enum) << FirstEnum << FirstModule.empty() << FirstModule << DiagAnchor->getSourceRange() << DiffType; }; - auto ODRDiagNote = [&SecondModule, this](const auto *DiagAnchor, - ODREnumDifference DiffType) { + auto DiagNote = [&SecondModule, this](const auto *DiagAnchor, + ODREnumDifference DiffType) { return Diag(DiagAnchor->getLocation(), diag::note_module_odr_violation_enum) << SecondModule << DiagAnchor->getSourceRange() << DiffType; }; 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; } } @@ -11232,52 +11229,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(); @@ -11286,29 +11293,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())