[clang] [C++20] [Modules] Enable -fmodules-embed-all-files by default for named modules (PR #74419)

[Chuanqi Xu via cfe-commits]

https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/74419

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

It looks incorrect or odd for the BMIs to dependent on the real files. It prevents we moving the BMIs and the distributed builds. And it looks like there is nothing beneficial we got by not enabling this. Also the cost is relatively small. See the discussion in the above issue for details.

>From b28f94d795582ef13e6fe8ce111c9638f4ed7f30 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Tue, 5 Dec 2023 14:24:10 +0800
Subject: [PATCH] [C++20] [Modules] Enable -fmodules-embed-all-files by default
 for named modules

---
 clang/include/clang/Driver/Types.h            |  3 +++
 clang/lib/Driver/ToolChains/Clang.cpp         |  7 ++++++
 clang/lib/Driver/Types.cpp                    |  4 ++++
 .../test/Driver/modules-embed-all-files.cppm  | 22 +++++++++++++++++++
 .../Serialization/ForceCheckFileInputTest.cpp |  2 ++
 5 files changed, 38 insertions(+)
 create mode 100644 clang/test/Driver/modules-embed-all-files.cppm

diff --git a/clang/include/clang/Driver/Types.h b/clang/include/clang/Driver/Types.h
index 121b58a6b477d..53340bdc64f70 100644
--- a/clang/include/clang/Driver/Types.h
+++ b/clang/include/clang/Driver/Types.h
@@ -80,6 +80,9 @@ namespace types {
   /// isCXX - Is this a "C++" input (C++ and Obj-C++ sources and headers).
   bool isCXX(ID Id);
 
+  /// isCXXModuleUnit - Is this a "C++ module unit" input.
+  bool isCXXModuleUnit(ID Id);
+
   /// Is this LLVM IR.
   bool isLLVMIR(ID Id);
 
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index f02f7c841b91f..9081258626eaa 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3817,6 +3817,13 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
     Args.ClaimAllArgs(options::OPT_fmodules_disable_diagnostic_validation);
   }
 
+  // '-fmodules-embed-all-files' should be enabled by default for C++20 named
+  // modules.
+  // See the discussion in https://github.com/llvm/llvm-project/issues/72383.
+  if (types::isCXXModuleUnit(Input.getType()) && HaveModules) {
+    CmdArgs.push_back("-fmodules-embed-all-files");
+  }
+
   // Claim `-fmodule-output` and `-fmodule-output=` to avoid unused warnings.
   Args.ClaimAllArgs(options::OPT_fmodule_output);
   Args.ClaimAllArgs(options::OPT_fmodule_output_EQ);
diff --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index 08df34ade7b65..52e5d9748d375 100644
--- a/clang/lib/Driver/Types.cpp
+++ b/clang/lib/Driver/Types.cpp
@@ -249,6 +249,10 @@ bool types::isCXX(ID Id) {
   }
 }
 
+bool types::isCXXModuleUnit(ID Id) {
+  return Id == TY_CXXModule || Id == TY_PP_CXXModule;
+}
+
 bool types::isLLVMIR(ID Id) {
   switch (Id) {
   default:
diff --git a/clang/test/Driver/modules-embed-all-files.cppm b/clang/test/Driver/modules-embed-all-files.cppm
new file mode 100644
index 0000000000000..86a4389fb4319
--- /dev/null
+++ b/clang/test/Driver/modules-embed-all-files.cppm
@@ -0,0 +1,22 @@
+// Tests that we'll enable -fmodules-embed-all-files for C++20 module units.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang %t/a.cppm -### 2>&1 | FileCheck %t/a.cppm --check-prefix=PRE20
+// RUN: %clang -std=c++20 %t/a.cppm -### 2>&1 | FileCheck %t/a.cppm
+
+// RUN: %clang %t/a.cpp -### 2>&1 | FileCheck %t/a.cpp --check-prefix=NO-CXX-MODULE
+// RUN: %clang -std=c++20 %t/a.cpp -### 2>&1 | FileCheck %t/a.cpp --check-prefix=NO-CXX-MODULE
+// RUN: %clang -std=c++20 -x c++-module %t/a.cpp -### 2>&1 | FileCheck %t/a.cpp
+
+//--- a.cppm
+
+// PRE20-NOT: -fmodules-embed-all-files
+// CHECK: -fmodules-embed-all-files
+
+//--- a.cpp
+
+// NO-CXX-MODULE-NOT: -fmodules-embed-all-files
+// CHECK: -fmodules-embed-all-files
diff --git a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
index ed0daa43436eb..eeb77914107cd 100644
--- a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
+++ b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
@@ -76,6 +76,7 @@ export int aa = 43;
         createInvocation(Args, CIOpts);
     EXPECT_TRUE(Invocation);
     Invocation->getFrontendOpts().DisableFree = false;
+    Invocation->getFrontendOpts().ModulesEmbedAllFiles = false;
 
     auto Buf = CIOpts.VFS->getBufferForFile("a.cppm");
     EXPECT_TRUE(Buf);
@@ -115,6 +116,7 @@ export int aa = 43;
         createInvocation(Args, CIOpts);
     EXPECT_TRUE(Invocation);
     Invocation->getFrontendOpts().DisableFree = false;
+    Invocation->getFrontendOpts().ModulesEmbedAllFiles = false;
 
     CompilerInstance Clang;