[clang] 5bbadec - Recommit [C++20] [Modules] Don't load declaration eagerly for named modules
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 5 20:02:23 PDT 2023
Author: Chuanqi Xu
Date: 2023-04-06T11:01:58+08:00
New Revision: 5bbadec2d1e0d5d0d25295759d49f6cd420d0968
URL: https://github.com/llvm/llvm-project/commit/5bbadec2d1e0d5d0d25295759d49f6cd420d0968
DIFF: https://github.com/llvm/llvm-project/commit/5bbadec2d1e0d5d0d25295759d49f6cd420d0968.diff
LOG: Recommit [C++20] [Modules] Don't load declaration eagerly for named modules
Close https://github.com/llvm/llvm-project/issues/61064.
The root cause of the issue is that we will deserilize some declarations
eagerly when reading the BMI. However, many declarations in the BMI are
not necessary for the importer. So it wastes a lot of time.
The new commit handles the MSVC's extension #pragma comment and #pragma
detect_mismatch to follow MSVC's behavior. See pr61783 for details.
Added:
clang/test/Modules/no-eager-load.cppm
clang/test/Modules/pr61783.cppm
Modified:
clang/lib/Serialization/ASTWriterDecl.cpp
Removed:
################################################################################
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index abb6bb747bfa9..54b5e3877782d 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2500,7 +2500,20 @@ void ASTWriter::WriteDeclAbbrevs() {
/// relatively painless since they would presumably only do it for top-level
/// decls.
static bool isRequiredDecl(const Decl *D, ASTContext &Context,
- bool WritingModule) {
+ Module *WritingModule) {
+ // Named modules have
diff erent semantics than header modules. Every named
+ // module units owns a translation unit. So the importer of named modules
+ // doesn't need to deserilize everything ahead of time.
+ if (WritingModule && WritingModule->isModulePurview()) {
+ // The PragmaCommentDecl and PragmaDetectMismatchDecl are MSVC's extension.
+ // And the behavior of MSVC for such cases will leak this to the module
+ // users. Given pragma is not a standard thing, the compiler has the space
+ // to do their own decision. Let's follow MSVC here.
+ if (isa<PragmaCommentDecl, PragmaDetectMismatchDecl>(D))
+ return true;
+ return false;
+ }
+
// An ObjCMethodDecl is never considered as "required" because its
// implementation container always is.
diff --git a/clang/test/Modules/no-eager-load.cppm b/clang/test/Modules/no-eager-load.cppm
new file mode 100644
index 0000000000000..6632cc60c8eb8
--- /dev/null
+++ b/clang/test/Modules/no-eager-load.cppm
@@ -0,0 +1,110 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/b.cppm -o %t/b.pcm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/c.cpp \
+// RUN: -fprebuilt-module-path=%t
+// 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;
+export void foo() {
+
+}
+
+//--- b.cppm
+export module b;
+void bar();
+export void foo() {
+ bar();
+}
+
+//--- c.cpp
+// expected-no-diagnostics
+// Since we will load all the declaration lazily, we won't be able to find
+// the ODR violation here.
+import a;
+import b;
+
+//--- d.cpp
+import a;
+import b;
+// Test that we can still check the odr violation if we call the function
+// actually.
+void use() {
+ foo(); // expected-error@* {{'foo' has
diff erent definitions in
diff erent modules;}}
+ // expected-note@* {{but in 'a' found a
diff erent 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
diff erent definitions in
diff erent modules;}}
+ // expected-note@* {{but in 'e.<global>' found a
diff erent 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
diff erent definitions in
diff erent modules;}}
+ // expected-note@* {{but in 'a' found a
diff erent body}}
+}
+
+//--- k.cpp
+import i;
+void use() {
+ foo(); // expected-error@* {{'foo' has
diff erent definitions in
diff erent modules;}}
+ // expected-note@* {{but in 'e.<global>' found a
diff erent body}}
+}
+
diff --git a/clang/test/Modules/pr61783.cppm b/clang/test/Modules/pr61783.cppm
new file mode 100644
index 0000000000000..9cf773b0b282b
--- /dev/null
+++ b/clang/test/Modules/pr61783.cppm
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions %t/mod.cppm -emit-module-interface \
+// RUN: -o %t/mod.pcm
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions %t/mod.pcm -S -emit-llvm -o - | \
+// RUN: FileCheck %t/mod.cppm
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions %t/user.cpp -fmodule-file=mod=%t/mod.pcm \
+// RUN: -S -emit-llvm -o - | FileCheck %t/user.cpp
+
+//--- mod.cppm
+module;
+
+#pragma comment(lib, "msvcprt.lib")
+#pragma detect_mismatch("myLib_version", "9")
+
+export module mod;
+
+// CHECK: ![[NUM:[0-9]+]] ={{.*}}msvcprt.lib
+// CHECK: ![[NUM:[0-9]+]] ={{.*}}FAILIFMISMATCH{{.*}}myLib_version=9
+
+//--- user.cpp
+#pragma detect_mismatch("myLib_version", "1")
+import mod;
+
+// CHECK: ![[NUM:[0-9]+]] ={{.*}}FAILIFMISMATCH{{.*}}myLib_version=1
+// CHECK: ![[NUM:[0-9]+]] ={{.*}}msvcprt.lib
+// CHECK: ![[NUM:[0-9]+]] ={{.*}}FAILIFMISMATCH{{.*}}myLib_version=9
More information about the cfe-commits
mailing list