ChuanqiXu created this revision.
ChuanqiXu added reviewers: urnathan, iains.
ChuanqiXu added a project: clang.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.
It is simpler to search for module unit by `-fprebuilt-module-path` option.
However, the separator ':' of partitions is not friendly. According to the
discussion in https://reviews.llvm.org/D118586, I think we get consensus to use
'-' as the separator instead. The '-' is the choice of GCC too.
Previously I thought it would be better to add an option. But I feel it is
over-engineering now. Another reason here is that there are too many options
for modules (for clang module mainly) now. Given it is not bad to use '-' when
searching, I think it is acceptable to not add an option.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120874
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/search-partitions.cppm
Index: clang/test/Modules/search-partitions.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/search-partitions.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition1.cpp \
+// RUN: -o %t/A-Part1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition2.cpp \
+// RUN: -o %t/A-Part2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition3.cpp \
+// RUN: -o %t/A-Part3.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/moduleA.cpp \
+// RUN: -fprebuilt-module-path=%t
+
+// expected-no-diagnostics
+
+//--- partition1.cpp
+export module A : Part1;
+
+int part1();
+
+//--- partition2.cpp
+
+export module A : Part2;
+
+int part2();
+
+//--- partition3.cpp
+
+export module A : Part3;
+
+int part3();
+
+//--- moduleA.cpp
+
+export module A;
+
+import : Part1;
+export import : Part2;
+import : Part3;
+
+int foo();
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -194,10 +194,20 @@
for (const std::string &Dir : HSOpts->PrebuiltModulePaths) {
SmallString<256> Result(Dir);
llvm::sys::fs::make_absolute(Result);
- llvm::sys::path::append(Result, ModuleName + ".pcm");
+ if (ModuleName.contains(':'))
+ // The separator of C++20 modules partitions (':') is not good for file
+ // systems, here we choose '-' by default since it is not a valid
+ // character of C++ indentifiers. So we could avoid conflicts. BTW, '-'
is
+ // the choice of GCC too.
+ llvm::sys::path::append(Result, ModuleName.split(':').first + "-" +
+ ModuleName.split(':').second +
+ ".pcm");
+ else
+ llvm::sys::path::append(Result, ModuleName + ".pcm");
if (getFileMgr().getFile(Result.str()))
return std::string(Result);
}
+
return {};
}
Index: clang/test/Modules/search-partitions.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/search-partitions.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition1.cpp \
+// RUN: -o %t/A-Part1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition2.cpp \
+// RUN: -o %t/A-Part2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition3.cpp \
+// RUN: -o %t/A-Part3.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/moduleA.cpp \
+// RUN: -fprebuilt-module-path=%t
+
+// expected-no-diagnostics
+
+//--- partition1.cpp
+export module A : Part1;
+
+int part1();
+
+//--- partition2.cpp
+
+export module A : Part2;
+
+int part2();
+
+//--- partition3.cpp
+
+export module A : Part3;
+
+int part3();
+
+//--- moduleA.cpp
+
+export module A;
+
+import : Part1;
+export import : Part2;
+import : Part3;
+
+int foo();
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -194,10 +194,20 @@
for (const std::string &Dir : HSOpts->PrebuiltModulePaths) {
SmallString<256> Result(Dir);
llvm::sys::fs::make_absolute(Result);
- llvm::sys::path::append(Result, ModuleName + ".pcm");
+ if (ModuleName.contains(':'))
+ // The separator of C++20 modules partitions (':') is not good for file
+ // systems, here we choose '-' by default since it is not a valid
+ // character of C++ indentifiers. So we could avoid conflicts. BTW, '-' is
+ // the choice of GCC too.
+ llvm::sys::path::append(Result, ModuleName.split(':').first + "-" +
+ ModuleName.split(':').second +
+ ".pcm");
+ else
+ llvm::sys::path::append(Result, ModuleName + ".pcm");
if (getFileMgr().getFile(Result.str()))
return std::string(Result);
}
+
return {};
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
