diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 9a4736019d1b1b..eb7a1a32060077 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -673,6 +673,16 @@ class alignas(8) Decl { /// fragment. See [module.global.frag]p3,4 for details. bool isDiscardedInGlobalModuleFragment() const { return false; } + /// Check if we should skip checking ODRHash for declaration \param D. + /// + /// The existing ODRHash mechanism seems to be not stable enough and + /// the false positive ODR violation reports are annoying and we rarely see + /// true ODR violation reports. Also we learned that MSVC disabled ODR checks + /// for declarations in GMF. So we try to disable ODR checks in the GMF to + /// get better user experiences before we make the ODR violation checks stable + /// enough. + bool shouldSkipCheckingODR() const; + /// Return true if this declaration has an attribute which acts as /// definition of the entity, such as 'alias' or 'ifunc'. bool hasDefiningAttr() const; diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index cd28226c295b32..62c25f5b7a0df8 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -2451,13 +2451,6 @@ class BitsUnpacker { uint32_t Value; uint32_t CurrentBitsIndex = ~0; }; - -inline bool shouldSkipCheckingODR(const Decl *D) { - return D->getOwningModule() && - D->getASTContext().getLangOpts().SkipODRCheckInGMF && - D->getOwningModule()->isExplicitGlobalModule(); -} - } // namespace clang #endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 26fdfa040796ed..1ee33fd7576d7d 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -4476,7 +4476,7 @@ unsigned FunctionDecl::getODRHash() { } class ODRHash Hash; - Hash.AddFunctionDecl(this); + Hash.AddFunctionDecl(this, /*SkipBody=*/shouldSkipCheckingODR()); setHasODRHash(true); ODRHash = Hash.CalculateHash(); return ODRHash; diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 8163f9bdaf8d97..6b3c13ff206d23 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1102,6 +1102,11 @@ bool Decl::isInAnotherModuleUnit() const { return M != getASTContext().getCurrentNamedModule(); } +bool Decl::shouldSkipCheckingODR() const { + return getASTContext().getLangOpts().SkipODRCheckInGMF && getOwningModule() && + getOwningModule()->isExplicitGlobalModule(); +} + static Decl::Kind getKind(const Decl *D) { return D->getKind(); } static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 028610deb3001e..490b8cb10a4841 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9745,7 +9745,7 @@ void ASTReader::finishPendingActions() { !NonConstDefn->isLateTemplateParsed() && // We only perform ODR checks for decls not in the explicit // global module fragment. - !shouldSkipCheckingODR(FD) && + !FD->shouldSkipCheckingODR() && FD->getODRHash() != NonConstDefn->getODRHash()) { if (!isa(FD)) { PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn); diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 321c11e55c14e3..110f55f8c0f49a 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -832,7 +832,7 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) { Reader.mergeDefinitionVisibility(OldDef, ED); // We don't want to check the ODR hash value for declarations from global // module fragment. - if (!shouldSkipCheckingODR(ED) && + if (!ED->shouldSkipCheckingODR() && OldDef->getODRHash() != ED->getODRHash()) Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED); } else { @@ -874,7 +874,7 @@ void ASTDeclReader::VisitRecordDecl(RecordDecl *RD) { VisitRecordDeclImpl(RD); // We should only reach here if we're in C/Objective-C. There is no // global module fragment. - assert(!shouldSkipCheckingODR(RD)); + assert(!RD->shouldSkipCheckingODR()); RD->setODRHash(Record.readInt()); // Maintain the invariant of a redeclaration chain containing only @@ -2152,7 +2152,7 @@ void ASTDeclReader::MergeDefinitionData( } // We don't want to check ODR for decls in the global module fragment. - if (shouldSkipCheckingODR(MergeDD.Definition)) + if (MergeDD.Definition->shouldSkipCheckingODR()) return; if (D->getODRHash() != MergeDD.ODRHash) { @@ -3526,7 +3526,7 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { // same template specialization into the same CXXRecordDecl. auto MergedDCIt = Reader.MergedDeclContexts.find(D->getLexicalDeclContext()); if (MergedDCIt != Reader.MergedDeclContexts.end() && - !shouldSkipCheckingODR(D) && MergedDCIt->second == D->getDeclContext()) + !D->shouldSkipCheckingODR() && MergedDCIt->second == D->getDeclContext()) Reader.PendingOdrMergeChecks.push_back(D); return FindExistingResult(Reader, D, /*Existing=*/nullptr, diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 73018c1170d8f2..378a1f86bd5342 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -6010,7 +6010,7 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) { BitsPacker DefinitionBits; - bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + bool ShouldSkipCheckingODR = D->shouldSkipCheckingODR(); DefinitionBits.addBit(ShouldSkipCheckingODR); #define FIELD(Name, Width, Merge) \ diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index e73800100e3ccf..42583c09f009e0 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -488,7 +488,7 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { BitsPacker EnumDeclBits; EnumDeclBits.addBits(D->getNumPositiveBits(), /*BitWidth=*/8); EnumDeclBits.addBits(D->getNumNegativeBits(), /*BitWidth=*/8); - bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + bool ShouldSkipCheckingODR = D->shouldSkipCheckingODR(); EnumDeclBits.addBit(ShouldSkipCheckingODR); EnumDeclBits.addBit(D->isScoped()); EnumDeclBits.addBit(D->isScopedUsingClassTag()); @@ -514,7 +514,7 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { !D->isTopLevelDeclInObjCContainer() && !CXXRecordDecl::classofKind(D->getKind()) && !D->getIntegerTypeSourceInfo() && !D->getMemberSpecializationInfo() && - !needsAnonymousDeclarationNumber(D) && !shouldSkipCheckingODR(D) && + !needsAnonymousDeclarationNumber(D) && !D->shouldSkipCheckingODR() && D->getDeclName().getNameKind() == DeclarationName::Identifier) AbbrevToUse = Writer.getDeclEnumAbbrev(); @@ -680,7 +680,7 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { // FIXME: stable encoding FunctionDeclBits.addBits(llvm::to_underlying(D->getLinkageInternal()), 3); FunctionDeclBits.addBits((uint32_t)D->getStorageClass(), /*BitWidth=*/3); - bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + bool ShouldSkipCheckingODR = D->shouldSkipCheckingODR(); FunctionDeclBits.addBit(ShouldSkipCheckingODR); FunctionDeclBits.addBit(D->isInlineSpecified()); FunctionDeclBits.addBit(D->isInlined()); @@ -1514,7 +1514,7 @@ void ASTDeclWriter::VisitCXXMethodDecl(CXXMethodDecl *D) { D->getFirstDecl() == D->getMostRecentDecl() && !D->isInvalidDecl() && !D->hasAttrs() && !D->isTopLevelDeclInObjCContainer() && D->getDeclName().getNameKind() == DeclarationName::Identifier && - !shouldSkipCheckingODR(D) && !D->hasExtInfo() && + !D->shouldSkipCheckingODR() && !D->hasExtInfo() && !D->isExplicitlyDefaulted()) { if (D->getTemplatedKind() == FunctionDecl::TK_NonTemplate || D->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate || diff --git a/clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm b/clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm new file mode 100644 index 00000000000000..8db53c0ace8796 --- /dev/null +++ b/clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm @@ -0,0 +1,67 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/A.cppm -emit-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/B.cppm -emit-module-interface -o %t/B.pcm +// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/test.cpp -fprebuilt-module-path=%t -fsyntax-only -verify + +//--- header.h +#pragma once +template +class Optional {}; + +template +concept C = requires(const _Tp& __t) { + [](const Optional<_Up>&) {}(__t); +}; + +//--- func.h +#include "header.h" +template +void func() {} + +//--- duplicated_func.h +#include "header.h" +template +void duplicated_func() {} + +//--- test_func.h +#include "func.h" + +void test_func() { + func>(); +} + +//--- test_duplicated_func.h +#include "duplicated_func.h" + +void test_duplicated_func() { + duplicated_func>(); +} + +//--- A.cppm +module; +#include "header.h" +#include "test_duplicated_func.h" +export module A; +export using ::test_duplicated_func; + +//--- B.cppm +module; +#include "header.h" +#include "test_func.h" +#include "test_duplicated_func.h" +export module B; +export using ::test_func; +export using ::test_duplicated_func; + +//--- test.cpp +// expected-no-diagnostics +import A; +import B; + +void test() { + test_func(); + test_duplicated_func(); +}