[clang] 279c7a2 - Revert "[C++20] [Modules] Don't load declaration eagerly for named modules"

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-03-29T11:15:38+08:00
New Revision: 279c7a2f17937836ed13e359c3fb381bef7defaf

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

LOG: Revert "[C++20] [Modules] Don't load declaration eagerly for named modules"

This reverts commit af86957cbbffd3dfff3c6750ebddf118aebd0069.

Close https://github.com/llvm/llvm-project/issues/61733.

Previously I banned the eagerly loading for declarations from named
modules to speedup the process of reading modules. But I didn't think
about special decls like PragmaCommentDecl and PragmaDetectMismatchDecl.
So here is the issue https://github.com/llvm/llvm-project/issues/61733.

Note that the current behavior is still incorrect. Given:

```
// mod.cppm
module;

export module mod;
```

and

```
// user.cpp
import mod;
```

Now the IR of `user.cpp` will contain the metadata '!0 =
!{!"msvcprt.lib"}' incorrectly. The root cause of the problem is that
`EagerlyDeserializedDecls` is designed for headers and it didn't take
care for named modules. We need to redesign a new mechanism for named
modules.

Added: 
    

Modified: 
    clang/lib/Serialization/ASTWriterDecl.cpp

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


################################################################################
diff  --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index bbd3c36df2f96..69d192612bccf 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2468,13 +2468,7 @@ void ASTWriter::WriteDeclAbbrevs() {
 /// relatively painless since they would presumably only do it for top-level
 /// decls.
 static bool isRequiredDecl(const Decl *D, ASTContext &Context,
-                           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;
-
+                           bool WritingModule) {
   // 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
deleted file mode 100644
index 6632cc60c8eb8..0000000000000
--- a/clang/test/Modules/no-eager-load.cppm
+++ /dev/null
@@ -1,110 +0,0 @@
-// 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}}
-}
-