Author: Chuanqi Xu Date: 2024-07-19T13:38:57+08:00 New Revision: 2f0910d2d74419ef1ebf814b471af721ee78b464
URL: https://github.com/llvm/llvm-project/commit/2f0910d2d74419ef1ebf814b471af721ee78b464 DIFF: https://github.com/llvm/llvm-project/commit/2f0910d2d74419ef1ebf814b471af721ee78b464.diff LOG: [C++20] [Modules] Skip ODR checks if either declaration comes from GMF This patch tries to workaround the case that: - in a module unit that imports another module unit - both the module units including overlapped headers - the compiler emits false positive ODR violation diagnostics for the overlapped headers if ODR check is enabled - the current module units enables PCH For the third point, we disabled ODR check if the declarations comes from GMF. However, due to the forth point, the check whether the declaration comes from GMF failed. Then we still going to check it and then the users get false positive checks. What's worse is that, this always happens in clangd, where will generate the PCH automatically before parsing the input files. The root cause of the problem we mixed the modules in the semantical level and the module in the serialization level. The problem is pretty fundamental and we need time to fix that. But 19.x is going to be branched and I hope to give clangd better user experience. So I decided to land this workaround even if it is pretyy niche and may only work for the case of clangd's pattern. Added: clang/test/Modules/pch-in-module-units.cppm Modified: clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTReaderDecl.cpp Removed: ################################################################################ diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index afdeccaf93a9d..3cb96df12e4da 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9875,6 +9875,7 @@ void ASTReader::finishPendingActions() { // We only perform ODR checks for decls not in the explicit // global module fragment. !shouldSkipCheckingODR(FD) && + !shouldSkipCheckingODR(NonConstDefn) && FD->getODRHash() != NonConstDefn->getODRHash()) { if (!isa<CXXMethodDecl>(FD)) { PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn); diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 3de9d327f1b3b..31ab6c651d59f 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -821,7 +821,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 (!shouldSkipCheckingODR(ED) && !shouldSkipCheckingODR(OldDef) && OldDef->getODRHash() != ED->getODRHash()) Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED); } else { @@ -2134,7 +2134,7 @@ void ASTDeclReader::MergeDefinitionData( } // We don't want to check ODR for decls in the global module fragment. - if (shouldSkipCheckingODR(MergeDD.Definition)) + if (shouldSkipCheckingODR(MergeDD.Definition) || shouldSkipCheckingODR(D)) return; if (D->getODRHash() != MergeDD.ODRHash) { diff --git a/clang/test/Modules/pch-in-module-units.cppm b/clang/test/Modules/pch-in-module-units.cppm new file mode 100644 index 0000000000000..790a3af09a0ca --- /dev/null +++ b/clang/test/Modules/pch-in-module-units.cppm @@ -0,0 +1,51 @@ +// Test that we will skip ODR checks for declarations from PCH if they +// were from GMF. +// +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/A.cppm \ +// RUN: -o %t/A.pcm -fskip-odr-check-in-gmf +// RUN: %clang_cc1 -std=c++20 -DDIFF -x c++-header %t/foo.h \ +// RUN: -emit-pch -o %t/foo.pch -fskip-odr-check-in-gmf +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -fmodule-file=A=%t/A.pcm -include-pch \ +// RUN: %t/foo.pch -verify -fsyntax-only -fskip-odr-check-in-gmf + +//--- foo.h +#ifndef FOO_H +#define FOO_H +inline int foo() { +#ifndef DIFF + return 43; +#else + return 45; +#endif +} + +class f { +public: + int mem() { +#ifndef DIFF + return 47; +#else + return 45; +#endif + } +}; +#endif + +//--- A.cppm +module; +#include "foo.h" +export module A; +export using ::foo; +export using ::f; + +//--- B.cppm +// expected-no-diagnostics +module; +#include "foo.h" +export module B; +import A; +export int b = foo() + f().mem(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
