[clang] af86957 - [C++20] [Modules] Don't load declaration eagerly for named modules

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-03-10T14:57:21+08:00
New Revision: af86957cbbffd3dfff3c6750ebddf118aebd0069

URL: https://github.com/llvm/llvm-project/commit/af86957cbbffd3dfff3c6750ebddf118aebd0069
DIFF: https://github.com/llvm/llvm-project/commit/af86957cbbffd3dfff3c6750ebddf118aebd0069.diff

LOG: [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.

Added: 
    clang/test/Modules/no-eager-load.cppm

Modified: 
    clang/lib/Serialization/ASTWriterDecl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 69d192612bcc..bbd3c36df2f9 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2468,7 +2468,13 @@ 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())
+    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 000000000000..6632cc60c8eb
--- /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}}
+}
+