diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 2aa8fea4ad67c..9d68be469dac3 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -62,6 +62,11 @@ C++ Language Changes C++20 Feature Support ^^^^^^^^^^^^^^^^^^^^^ +- Clang won't perform ODR checks for decls in the global module fragment any + more to ease the implementation and improve the user's using experience. + This follows the MSVC's behavior. + (`#79240 `_). + C++23 Feature Support ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index dd1451bbf2d2c..ba06ab0cd4509 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -2452,6 +2452,10 @@ class BitsUnpacker { uint32_t CurrentBitsIndex = ~0; }; +inline bool isFromExplicitGMF(const Decl *D) { + return D->getOwningModule() && D->getOwningModule()->isExplicitGlobalModule(); +} + } // namespace clang #endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 89b044a61a7b1..2abe5e44e2e98 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9747,6 +9747,9 @@ void ASTReader::finishPendingActions() { if (!FD->isLateTemplateParsed() && !NonConstDefn->isLateTemplateParsed() && + // We only perform ODR checks for decls not in the explicit + // global module fragment. + !isFromExplicitGMF(FD) && 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 867f4c47eaece..29ece2a132bf8 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -804,8 +804,10 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) { ED->setScopedUsingClassTag(EnumDeclBits.getNextBit()); ED->setFixed(EnumDeclBits.getNextBit()); - ED->setHasODRHash(true); - ED->ODRHash = Record.readInt(); + if (!isFromExplicitGMF(ED)) { + ED->setHasODRHash(true); + ED->ODRHash = Record.readInt(); + } // If this is a definition subject to the ODR, and we already have a // definition, merge this one into it. @@ -827,7 +829,9 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) { Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef)); ED->demoteThisDefinitionToDeclaration(); Reader.mergeDefinitionVisibility(OldDef, ED); - if (OldDef->getODRHash() != ED->getODRHash()) + // We don't want to check the ODR hash value for declarations from global + // module fragment. + if (!isFromExplicitGMF(ED) && OldDef->getODRHash() != ED->getODRHash()) Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED); } else { OldDef = ED; @@ -866,6 +870,9 @@ ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) { 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(!isFromExplicitGMF(RD)); RD->setODRHash(Record.readInt()); // Maintain the invariant of a redeclaration chain containing only @@ -1094,8 +1101,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) { if (FD->isExplicitlyDefaulted()) FD->setDefaultLoc(readSourceLocation()); - FD->ODRHash = Record.readInt(); - FD->setHasODRHash(true); + if (!isFromExplicitGMF(FD)) { + FD->ODRHash = Record.readInt(); + FD->setHasODRHash(true); + } if (FD->isDefaulted()) { if (unsigned NumLookups = Record.readInt()) { @@ -1971,9 +1980,12 @@ void ASTDeclReader::ReadCXXDefinitionData( #include "clang/AST/CXXRecordDeclDefinitionBits.def" #undef FIELD - // Note: the caller has deserialized the IsLambda bit already. - Data.ODRHash = Record.readInt(); - Data.HasODRHash = true; + // We only perform ODR checks for decls not in GMF. + if (!isFromExplicitGMF(D)) { + // Note: the caller has deserialized the IsLambda bit already. + Data.ODRHash = Record.readInt(); + Data.HasODRHash = true; + } if (Record.readInt()) { Reader.DefinitionSource[D] = @@ -2134,6 +2146,10 @@ void ASTDeclReader::MergeDefinitionData( } } + // We don't want to check ODR for decls in the global module fragment. + if (isFromExplicitGMF(MergeDD.Definition)) + return; + if (D->getODRHash() != MergeDD.ODRHash) { DetectedOdrViolation = true; } @@ -3498,10 +3514,13 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { // If this declaration is from a merged context, make a note that we need to // check that the canonical definition of that context contains the decl. // + // Note that we don't perform ODR checks for decls from the global module + // fragment. + // // FIXME: We should do something similar if we merge two definitions of the // same template specialization into the same CXXRecordDecl. auto MergedDCIt = Reader.MergedDeclContexts.find(D->getLexicalDeclContext()); - if (MergedDCIt != Reader.MergedDeclContexts.end() && + if (MergedDCIt != Reader.MergedDeclContexts.end() && !isFromExplicitGMF(D) && MergedDCIt->second == D->getDeclContext()) Reader.PendingOdrMergeChecks.push_back(D); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index eb2f8e2371099..dcb18ad338932 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -6027,8 +6027,12 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) { Record->push_back(DefinitionBits); - // getODRHash will compute the ODRHash if it has not been previously computed. - Record->push_back(D->getODRHash()); + // We only perform ODR checks for decls not in GMF. + if (!isFromExplicitGMF(D)) { + // getODRHash will compute the ODRHash if it has not been previously + // computed. + Record->push_back(D->getODRHash()); + } bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType(); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index bb1f51786d281..86d40436a9bac 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -493,7 +493,9 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { EnumDeclBits.addBit(D->isFixed()); Record.push_back(EnumDeclBits); - Record.push_back(D->getODRHash()); + // We only perform ODR checks for decls not in GMF. + if (!isFromExplicitGMF(D)) + Record.push_back(D->getODRHash()); if (MemberSpecializationInfo *MemberInfo = D->getMemberSpecializationInfo()) { Record.AddDeclRef(MemberInfo->getInstantiatedFrom()); @@ -510,7 +512,7 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { !D->isTopLevelDeclInObjCContainer() && !CXXRecordDecl::classofKind(D->getKind()) && !D->getIntegerTypeSourceInfo() && !D->getMemberSpecializationInfo() && - !needsAnonymousDeclarationNumber(D) && + !needsAnonymousDeclarationNumber(D) && !isFromExplicitGMF(D) && D->getDeclName().getNameKind() == DeclarationName::Identifier) AbbrevToUse = Writer.getDeclEnumAbbrev(); @@ -701,7 +703,9 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { if (D->isExplicitlyDefaulted()) Record.AddSourceLocation(D->getDefaultLoc()); - Record.push_back(D->getODRHash()); + // We only perform ODR checks for decls not in GMF. + if (!isFromExplicitGMF(D)) + Record.push_back(D->getODRHash()); if (D->isDefaulted()) { if (auto *FDI = D->getDefaultedFunctionInfo()) { @@ -1506,7 +1510,8 @@ void ASTDeclWriter::VisitCXXMethodDecl(CXXMethodDecl *D) { D->getFirstDecl() == D->getMostRecentDecl() && !D->isInvalidDecl() && !D->hasAttrs() && !D->isTopLevelDeclInObjCContainer() && D->getDeclName().getNameKind() == DeclarationName::Identifier && - !D->hasExtInfo() && !D->isExplicitlyDefaulted()) { + !isFromExplicitGMF(D) && !D->hasExtInfo() && + !D->isExplicitlyDefaulted()) { if (D->getTemplatedKind() == FunctionDecl::TK_NonTemplate || D->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate || D->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization || diff --git a/clang/test/Modules/concept.cppm b/clang/test/Modules/concept.cppm index 0e85a46411a54..a343c9cd76a11 100644 --- a/clang/test/Modules/concept.cppm +++ b/clang/test/Modules/concept.cppm @@ -70,13 +70,6 @@ module; export module B; import A; -#ifdef DIFFERENT -// expected-error@foo.h:41 {{'__fn::operator()' from module 'A.' is not present in definition of '__fn' provided earlier}} -// expected-note@* 1+{{declaration of 'operator()' does not match}} -#else -// expected-no-diagnostics -#endif - template struct U { auto operator+(U) { return 0; } @@ -94,3 +87,10 @@ void foo() { __fn{}(U(), U()); } + +#ifdef DIFFERENT +// expected-error@B.cppm:* {{call to object of type '__fn' is ambiguous}} +// expected-note@* 1+{{candidate function}} +#else +// expected-no-diagnostics +#endif diff --git a/clang/test/Modules/no-eager-load.cppm b/clang/test/Modules/no-eager-load.cppm index 6632cc60c8eb8..8a2c7656bca2b 100644 --- a/clang/test/Modules/no-eager-load.cppm +++ b/clang/test/Modules/no-eager-load.cppm @@ -9,19 +9,10 @@ // RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/d.cpp \ // RUN: -fprebuilt-module-path=%t // -// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/e.cppm -o %t/e.pcm -// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/f.cppm -o %t/f.pcm -// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/g.cpp \ -// RUN: -fprebuilt-module-path=%t -// // RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/h.cppm \ // RUN: -fprebuilt-module-path=%t -o %t/h.pcm -// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/i.cppm \ -// RUN: -fprebuilt-module-path=%t -o %t/i.pcm // RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/j.cpp \ // RUN: -fprebuilt-module-path=%t -// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/k.cpp \ -// RUN: -fprebuilt-module-path=%t //--- a.cppm export module a; @@ -53,58 +44,14 @@ void use() { // expected-note@* {{but in 'a' found a different body}} } -//--- foo.h -void foo() { - -} - -//--- bar.h -void bar(); -void foo() { - bar(); -} - -//--- e.cppm -module; -#include "foo.h" -export module e; -export using ::foo; - -//--- f.cppm -module; -#include "bar.h" -export module f; -export using ::foo; - -//--- g.cpp -import e; -import f; -void use() { - foo(); // expected-error@* {{'foo' has different definitions in different modules;}} - // expected-note@* {{but in 'e.' found a different body}} -} - //--- h.cppm export module h; export import a; export import b; -//--- i.cppm -export module i; -export import e; -export import f; - //--- j.cpp import h; void use() { foo(); // expected-error@* {{'foo' has different definitions in different modules;}} // expected-note@* {{but in 'a' found a different body}} } - -//--- k.cpp -import i; -void use() { - foo(); // expected-error@* {{'foo' has different definitions in different modules;}} - // expected-note@* {{but in 'e.' found a different body}} -} - diff --git a/clang/test/Modules/polluted-operator.cppm b/clang/test/Modules/polluted-operator.cppm index b24464aa6ad21..d4b0041b5d346 100644 --- a/clang/test/Modules/polluted-operator.cppm +++ b/clang/test/Modules/polluted-operator.cppm @@ -46,12 +46,10 @@ module; export module a; //--- b.cppm +// This is actually an ODR violation. But given https://github.com/llvm/llvm-project/issues/79240, +// we don't count it as an ODR violation any more. +// expected-no-diagnostics module; #include "bar.h" export module b; import a; - -// expected-error@* {{has different definitions in different modules; first difference is defined here found data member '_S_copy_ctor' with an initializer}} -// expected-note@* {{but in 'a.' found data member '_S_copy_ctor' with a different initializer}} -// expected-error@* {{from module 'a.' is not present in definition of 'variant<_Types...>' provided earlier}} -// expected-note@* {{declaration of 'swap' does not match}} diff --git a/clang/test/Modules/pr76638.cppm b/clang/test/Modules/pr76638.cppm index 8cc807961421b..28d03384634ab 100644 --- a/clang/test/Modules/pr76638.cppm +++ b/clang/test/Modules/pr76638.cppm @@ -57,6 +57,9 @@ export module mod3; export using std::align_val_t; //--- mod4.cppm +// This is actually an ODR violation. But given https://github.com/llvm/llvm-project/issues/79240, +// we don't count it as an ODR violation now. +// expected-no-diagnostics module; #include "signed_size_t.h" #include "csize_t" @@ -64,6 +67,3 @@ module; export module mod4; import mod3; export using std::align_val_t; - -// expected-error@align.h:* {{'std::align_val_t' has different definitions in different modules; defined here first difference is enum with specified type 'size_t' (aka 'int')}} -// expected-note@align.h:* {{but in 'mod3.' found enum with specified type 'size_t' (aka 'unsigned int')}}