Skip to content

Commit 5bbadec

Browse files
committed
Recommit [C++20] [Modules] Don't load declaration eagerly for named modules
Close #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.
1 parent b24b179 commit 5bbadec

File tree

3 files changed

+153
-1
lines changed

3 files changed

+153
-1
lines changed

clang/lib/Serialization/ASTWriterDecl.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2500,7 +2500,20 @@ void ASTWriter::WriteDeclAbbrevs() {
25002500
/// relatively painless since they would presumably only do it for top-level
25012501
/// decls.
25022502
static bool isRequiredDecl(const Decl *D, ASTContext &Context,
2503-
bool WritingModule) {
2503+
Module *WritingModule) {
2504+
// Named modules have different semantics than header modules. Every named
2505+
// module units owns a translation unit. So the importer of named modules
2506+
// doesn't need to deserilize everything ahead of time.
2507+
if (WritingModule && WritingModule->isModulePurview()) {
2508+
// The PragmaCommentDecl and PragmaDetectMismatchDecl are MSVC's extension.
2509+
// And the behavior of MSVC for such cases will leak this to the module
2510+
// users. Given pragma is not a standard thing, the compiler has the space
2511+
// to do their own decision. Let's follow MSVC here.
2512+
if (isa<PragmaCommentDecl, PragmaDetectMismatchDecl>(D))
2513+
return true;
2514+
return false;
2515+
}
2516+
25042517
// An ObjCMethodDecl is never considered as "required" because its
25052518
// implementation container always is.
25062519

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
// RUN: cd %t
4+
//
5+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a.cppm -o %t/a.pcm
6+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/b.cppm -o %t/b.pcm
7+
// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/c.cpp \
8+
// RUN: -fprebuilt-module-path=%t
9+
// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/d.cpp \
10+
// RUN: -fprebuilt-module-path=%t
11+
//
12+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/e.cppm -o %t/e.pcm
13+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/f.cppm -o %t/f.pcm
14+
// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/g.cpp \
15+
// RUN: -fprebuilt-module-path=%t
16+
//
17+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/h.cppm \
18+
// RUN: -fprebuilt-module-path=%t -o %t/h.pcm
19+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/i.cppm \
20+
// RUN: -fprebuilt-module-path=%t -o %t/i.pcm
21+
// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/j.cpp \
22+
// RUN: -fprebuilt-module-path=%t
23+
// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %t/k.cpp \
24+
// RUN: -fprebuilt-module-path=%t
25+
26+
//--- a.cppm
27+
export module a;
28+
export void foo() {
29+
30+
}
31+
32+
//--- b.cppm
33+
export module b;
34+
void bar();
35+
export void foo() {
36+
bar();
37+
}
38+
39+
//--- c.cpp
40+
// expected-no-diagnostics
41+
// Since we will load all the declaration lazily, we won't be able to find
42+
// the ODR violation here.
43+
import a;
44+
import b;
45+
46+
//--- d.cpp
47+
import a;
48+
import b;
49+
// Test that we can still check the odr violation if we call the function
50+
// actually.
51+
void use() {
52+
foo(); // expected-error@* {{'foo' has different definitions in different modules;}}
53+
// expected-note@* {{but in 'a' found a different body}}
54+
}
55+
56+
//--- foo.h
57+
void foo() {
58+
59+
}
60+
61+
//--- bar.h
62+
void bar();
63+
void foo() {
64+
bar();
65+
}
66+
67+
//--- e.cppm
68+
module;
69+
#include "foo.h"
70+
export module e;
71+
export using ::foo;
72+
73+
//--- f.cppm
74+
module;
75+
#include "bar.h"
76+
export module f;
77+
export using ::foo;
78+
79+
//--- g.cpp
80+
import e;
81+
import f;
82+
void use() {
83+
foo(); // expected-error@* {{'foo' has different definitions in different modules;}}
84+
// expected-note@* {{but in 'e.<global>' found a different body}}
85+
}
86+
87+
//--- h.cppm
88+
export module h;
89+
export import a;
90+
export import b;
91+
92+
//--- i.cppm
93+
export module i;
94+
export import e;
95+
export import f;
96+
97+
//--- j.cpp
98+
import h;
99+
void use() {
100+
foo(); // expected-error@* {{'foo' has different definitions in different modules;}}
101+
// expected-note@* {{but in 'a' found a different body}}
102+
}
103+
104+
//--- k.cpp
105+
import i;
106+
void use() {
107+
foo(); // expected-error@* {{'foo' has different definitions in different modules;}}
108+
// expected-note@* {{but in 'e.<global>' found a different body}}
109+
}
110+

clang/test/Modules/pr61783.cppm

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// RUN: rm -rf %t
2+
// RUN: mkdir -p %t
3+
// RUN: split-file %s %t
4+
//
5+
// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions %t/mod.cppm -emit-module-interface \
6+
// RUN: -o %t/mod.pcm
7+
// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions %t/mod.pcm -S -emit-llvm -o - | \
8+
// RUN: FileCheck %t/mod.cppm
9+
// 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 \
10+
// RUN: -S -emit-llvm -o - | FileCheck %t/user.cpp
11+
12+
//--- mod.cppm
13+
module;
14+
15+
#pragma comment(lib, "msvcprt.lib")
16+
#pragma detect_mismatch("myLib_version", "9")
17+
18+
export module mod;
19+
20+
// CHECK: ![[NUM:[0-9]+]] ={{.*}}msvcprt.lib
21+
// CHECK: ![[NUM:[0-9]+]] ={{.*}}FAILIFMISMATCH{{.*}}myLib_version=9
22+
23+
//--- user.cpp
24+
#pragma detect_mismatch("myLib_version", "1")
25+
import mod;
26+
27+
// CHECK: ![[NUM:[0-9]+]] ={{.*}}FAILIFMISMATCH{{.*}}myLib_version=1
28+
// CHECK: ![[NUM:[0-9]+]] ={{.*}}msvcprt.lib
29+
// CHECK: ![[NUM:[0-9]+]] ={{.*}}FAILIFMISMATCH{{.*}}myLib_version=9

0 commit comments

Comments
 (0)