Skip to content

[C++20] [Modules] Convert '-fexperimental-modules-reduced-bmi' to '-fmodules-reduced-bmi' #114382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 10, 2024

Conversation

ChuanqiXu9
Copy link
Member

According to our previous consensus in https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi, the reduced BMI will be the default and recommend users to use the new option.

The -fexperimental-modules-reduced-bmi option is introduced in #85050 in Mar13 and released in 19.x. And now we are in 20's release cycle. Also I rarely receive issue reports about reduced BMI. No matter it is due to the quality of reduced BMI is really good or no one uses it. I think we should speed up the process to get more people get involved.

This patch literally did the second point in https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Oct 31, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2024

@llvm/pr-subscribers-libcxx

@llvm/pr-subscribers-clang

Author: Chuanqi Xu (ChuanqiXu9)

Changes

According to our previous consensus in https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi, the reduced BMI will be the default and recommend users to use the new option.

The -fexperimental-modules-reduced-bmi option is introduced in #85050 in Mar13 and released in 19.x. And now we are in 20's release cycle. Also I rarely receive issue reports about reduced BMI. No matter it is due to the quality of reduced BMI is really good or no one uses it. I think we should speed up the process to get more people get involved.

This patch literally did the second point in https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi


Full diff: https://github.com/llvm/llvm-project/pull/114382.diff

5 Files Affected:

  • (modified) clang/docs/StandardCPlusPlusModules.rst (+9-9)
  • (modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+8)
  • (modified) clang/include/clang/Driver/Options.td (+4-1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+10-2)
  • (modified) clang/test/Driver/module-fgen-reduced-bmi.cppm (+24-2)
diff --git a/clang/docs/StandardCPlusPlusModules.rst b/clang/docs/StandardCPlusPlusModules.rst
index 8e22adad15106e..26b29337ce42cb 100644
--- a/clang/docs/StandardCPlusPlusModules.rst
+++ b/clang/docs/StandardCPlusPlusModules.rst
@@ -602,16 +602,16 @@ unnecessary dependencies for the BMI. To mitigate the problem, Clang has a
 compiler option to reduce the information contained in the BMI. These two
 formats are known as Full BMI and Reduced BMI, respectively.
 
-Users can use the ``-fexperimental-modules-reduced-bmi`` option to produce a
+Users can use the ``-fmodules-reduced-bmi`` option to produce a
 Reduced BMI.
 
 For the one-phase compilation model (CMake implements this model), with
-``-fexperimental-modules-reduced-bmi``, the generated BMI will be a Reduced
+``-fmodules-reduced-bmi``, the generated BMI will be a Reduced
 BMI automatically. (The output path of the BMI is specified by
 ``-fmodule-output=`` as usual with the one-phase compilation model).
 
 It is also possible to produce a Reduced BMI with the two-phase compilation
-model. When ``-fexperimental-modules-reduced-bmi``, ``--precompile``, and
+model. When ``-fmodules-reduced-bmi``, ``--precompile``, and
 ``-fmodule-output=`` are specified, the generated BMI specified by ``-o`` will
 be a full BMI and the BMI specified by ``-fmodule-output=`` will be a Reduced
 BMI. The dependency graph in this case would look like:
@@ -625,7 +625,7 @@ BMI. The dependency graph in this case would look like:
                                                -> ...
                                                -> consumer_n.cpp
 
-Clang does not emit diagnostics when ``-fexperimental-modules-reduced-bmi`` is
+Clang does not emit diagnostics when ``-fmodules-reduced-bmi`` is
 used with a non-module unit. This design permits users of the one-phase
 compilation model to try using reduced BMIs without needing to modify the build
 system. The two-phase compilation module requires build system support.
@@ -691,7 +691,7 @@ ensure it is reachable, e.g. ``using N::g;``.
 Support for Reduced BMIs is still experimental, but it may become the default
 in the future. The expected roadmap for Reduced BMIs as of Clang 19.x is:
 
-1. ``-fexperimental-modules-reduced-bmi`` is opt-in for 1~2 releases. The period depends
+1. ``-fmodules-reduced-bmi`` is opt-in for 1~2 releases. The period depends
    on user feedback and may be extended.
 2. Announce that Reduced BMIs are no longer experimental and introduce
    ``-fmodules-reduced-bmi`` as a new option, and recommend use of the new
@@ -814,8 +814,8 @@ With reduced BMI, non-cascading changes can be more powerful. For example,
 
 .. code-block:: console
 
-  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fexperimental-modules-reduced-bmi -o A.o
-  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fexperimental-modules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
+  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fmodules-reduced-bmi -o A.o
+  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fmodules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
   $ md5sum B.pcm
   6c2bd452ca32ab418bf35cd141b060b9  B.pcm
 
@@ -831,8 +831,8 @@ and recompile the example:
 
 .. code-block:: console
 
-  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fexperimental-modules-reduced-bmi -o A.o
-  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fexperimental-modules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
+  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fmodules-reduced-bmi -o A.o
+  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fmodules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
   $ md5sum B.pcm
   6c2bd452ca32ab418bf35cd141b060b9  B.pcm
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 65551bd7761a9d..7589c6f52419dd 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -556,6 +556,14 @@ def err_test_module_file_extension_format : Error<
 def err_drv_module_output_with_multiple_arch : Error<
   "option '-fmodule-output' can't be used with multiple arch options">;
 
+def warn_drv_module_reduced_bmi_not_enabled : Warning<
+  "reduced BMI is expected to be enabled by default in Clang 21. It is encouraged to "
+  "enable it ahead of time to avoid potential breaking change. You can enable it "
+  "by offering '-fmodules-reduced-bmi' option in one phase compilation model (e.g., CMake). "
+  "Or if your build system support two phase compilation model, please contact the "
+  "build system authors to support reduced BMI and turn off the warning temporarily">,
+  InGroup<DiagGroup<"missing-reduced-bmi">>;
+
 def warn_drv_delayed_template_parsing_after_cxx20 : Warning<
   "-fdelayed-template-parsing is deprecated after C++20">,
   InGroup<DiagGroup<"delayed-template-parsing-in-cxx20">>;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2b9ee1a0e669ed..bd4a716db29b0a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3194,11 +3194,14 @@ defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf",
           "Perform ODR checks for decls in the global module fragment.">>,
   Group<f_Group>;
 
-def modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">,
+def modules_reduced_bmi : Flag<["-"], "fmodules-reduced-bmi">,
   Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
   HelpText<"Generate the reduced BMI">,
   MarshallingInfoFlag<FrontendOpts<"GenReducedBMI">>;
 
+def experimental_modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">, 
+  Group<f_Group>, Visibility<[ClangOption, CC1Option]>, Alias<modules_reduced_bmi>;
+
 def fmodules_embed_all_files : Joined<["-"], "fmodules-embed-all-files">,
   Visibility<[ClangOption, CC1Option, CLOption]>,
   HelpText<"Embed the contents of all files read by this compilation into "
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 4c6f508f1f24a6..b67993d7b84d42 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4255,7 +4255,7 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
   if (Args.hasArg(options::OPT_modules_reduced_bmi) &&
       (Input.getType() == driver::types::TY_CXXModule ||
        Input.getType() == driver::types::TY_PP_CXXModule)) {
-    CmdArgs.push_back("-fexperimental-modules-reduced-bmi");
+    CmdArgs.push_back("-fmodules-reduced-bmi");
 
     if (Args.hasArg(options::OPT_fmodule_output_EQ))
       Args.AddLastArg(CmdArgs, options::OPT_fmodule_output_EQ);
@@ -4265,7 +4265,15 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
           getCXX20NamedModuleOutputPath(Args, Input.getBaseInput())));
   }
 
-  // Noop if we see '-fexperimental-modules-reduced-bmi' with other translation
+  if ((Input.getType() == driver::types::TY_CXXModule ||
+       Input.getType() == driver::types::TY_PP_CXXModule) &&
+      (Args.hasArg(options::OPT_fmodule_output) ||
+       Args.hasArg(options::OPT_fmodule_output_EQ) ||
+       Args.hasArg(options::OPT__precompile)) &&
+      !Args.hasArg(options::OPT_modules_reduced_bmi))
+    D.Diag(diag::warn_drv_module_reduced_bmi_not_enabled);
+
+  // Noop if we see '-fmodules-reduced-bmi' with other translation
   // units than module units. This is more user friendly to allow end uers to
   // enable this feature without asking for help from build systems.
   Args.ClaimAllArgs(options::OPT_modules_reduced_bmi);
diff --git a/clang/test/Driver/module-fgen-reduced-bmi.cppm b/clang/test/Driver/module-fgen-reduced-bmi.cppm
index 1223189fb49b72..af98684b5f95f9 100644
--- a/clang/test/Driver/module-fgen-reduced-bmi.cppm
+++ b/clang/test/Driver/module-fgen-reduced-bmi.cppm
@@ -29,13 +29,35 @@
 //
 // RUN: %clang -std=c++20 Hello.cc -fexperimental-modules-reduced-bmi -Wall -Werror \
 // RUN:     -c -o Hello.o -### 2>&1 | FileCheck Hello.cc
+//
+// RUN: %clang -std=c++20 Hello.cppm -fmodule-output=Hello.pcm \
+// RUN:     -fmodules-reduced-bmi -c -o Hello.o -### 2>&1 | FileCheck Hello.cppm
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -o Hello.o -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-UNSPECIFIED
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-NO-O
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -o AnotherName.o -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-ANOTHER-NAME
+//
+// RUN: %clang -std=c++20 Hello.cppm --precompile -fmodules-reduced-bmi \
+// RUN:     -o Hello.full.pcm -### 2>&1 | FileCheck Hello.cppm \
+// RUN:     --check-prefix=CHECK-EMIT-MODULE-INTERFACE
+//
+// RUN: %clang -std=c++20 Hello.cc -fmodules-reduced-bmi -Wall -Werror \
+// RUN:     -c -o Hello.o -### 2>&1 | FileCheck Hello.cc
 
 //--- Hello.cppm
 export module Hello;
 
 // Test that we won't generate the emit-module-interface as 2 phase compilation model.
 // CHECK-NOT: -emit-module-interface
-// CHECK: "-fexperimental-modules-reduced-bmi"
+// CHECK: "-fmodules-reduced-bmi"
 
 // CHECK-UNSPECIFIED: -fmodule-output=Hello.pcm
 
@@ -48,4 +70,4 @@ export module Hello;
 
 //--- Hello.cc
 
-// CHECK-NOT: "-fexperimental-modules-reduced-bmi"
+// CHECK-NOT: "-fmodules-reduced-bmi"

@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2024

@llvm/pr-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

Changes

According to our previous consensus in https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi, the reduced BMI will be the default and recommend users to use the new option.

The -fexperimental-modules-reduced-bmi option is introduced in #85050 in Mar13 and released in 19.x. And now we are in 20's release cycle. Also I rarely receive issue reports about reduced BMI. No matter it is due to the quality of reduced BMI is really good or no one uses it. I think we should speed up the process to get more people get involved.

This patch literally did the second point in https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi


Full diff: https://github.com/llvm/llvm-project/pull/114382.diff

5 Files Affected:

  • (modified) clang/docs/StandardCPlusPlusModules.rst (+9-9)
  • (modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+8)
  • (modified) clang/include/clang/Driver/Options.td (+4-1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+10-2)
  • (modified) clang/test/Driver/module-fgen-reduced-bmi.cppm (+24-2)
diff --git a/clang/docs/StandardCPlusPlusModules.rst b/clang/docs/StandardCPlusPlusModules.rst
index 8e22adad15106e..26b29337ce42cb 100644
--- a/clang/docs/StandardCPlusPlusModules.rst
+++ b/clang/docs/StandardCPlusPlusModules.rst
@@ -602,16 +602,16 @@ unnecessary dependencies for the BMI. To mitigate the problem, Clang has a
 compiler option to reduce the information contained in the BMI. These two
 formats are known as Full BMI and Reduced BMI, respectively.
 
-Users can use the ``-fexperimental-modules-reduced-bmi`` option to produce a
+Users can use the ``-fmodules-reduced-bmi`` option to produce a
 Reduced BMI.
 
 For the one-phase compilation model (CMake implements this model), with
-``-fexperimental-modules-reduced-bmi``, the generated BMI will be a Reduced
+``-fmodules-reduced-bmi``, the generated BMI will be a Reduced
 BMI automatically. (The output path of the BMI is specified by
 ``-fmodule-output=`` as usual with the one-phase compilation model).
 
 It is also possible to produce a Reduced BMI with the two-phase compilation
-model. When ``-fexperimental-modules-reduced-bmi``, ``--precompile``, and
+model. When ``-fmodules-reduced-bmi``, ``--precompile``, and
 ``-fmodule-output=`` are specified, the generated BMI specified by ``-o`` will
 be a full BMI and the BMI specified by ``-fmodule-output=`` will be a Reduced
 BMI. The dependency graph in this case would look like:
@@ -625,7 +625,7 @@ BMI. The dependency graph in this case would look like:
                                                -> ...
                                                -> consumer_n.cpp
 
-Clang does not emit diagnostics when ``-fexperimental-modules-reduced-bmi`` is
+Clang does not emit diagnostics when ``-fmodules-reduced-bmi`` is
 used with a non-module unit. This design permits users of the one-phase
 compilation model to try using reduced BMIs without needing to modify the build
 system. The two-phase compilation module requires build system support.
@@ -691,7 +691,7 @@ ensure it is reachable, e.g. ``using N::g;``.
 Support for Reduced BMIs is still experimental, but it may become the default
 in the future. The expected roadmap for Reduced BMIs as of Clang 19.x is:
 
-1. ``-fexperimental-modules-reduced-bmi`` is opt-in for 1~2 releases. The period depends
+1. ``-fmodules-reduced-bmi`` is opt-in for 1~2 releases. The period depends
    on user feedback and may be extended.
 2. Announce that Reduced BMIs are no longer experimental and introduce
    ``-fmodules-reduced-bmi`` as a new option, and recommend use of the new
@@ -814,8 +814,8 @@ With reduced BMI, non-cascading changes can be more powerful. For example,
 
 .. code-block:: console
 
-  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fexperimental-modules-reduced-bmi -o A.o
-  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fexperimental-modules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
+  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fmodules-reduced-bmi -o A.o
+  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fmodules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
   $ md5sum B.pcm
   6c2bd452ca32ab418bf35cd141b060b9  B.pcm
 
@@ -831,8 +831,8 @@ and recompile the example:
 
 .. code-block:: console
 
-  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fexperimental-modules-reduced-bmi -o A.o
-  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fexperimental-modules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
+  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fmodules-reduced-bmi -o A.o
+  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fmodules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
   $ md5sum B.pcm
   6c2bd452ca32ab418bf35cd141b060b9  B.pcm
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 65551bd7761a9d..7589c6f52419dd 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -556,6 +556,14 @@ def err_test_module_file_extension_format : Error<
 def err_drv_module_output_with_multiple_arch : Error<
   "option '-fmodule-output' can't be used with multiple arch options">;
 
+def warn_drv_module_reduced_bmi_not_enabled : Warning<
+  "reduced BMI is expected to be enabled by default in Clang 21. It is encouraged to "
+  "enable it ahead of time to avoid potential breaking change. You can enable it "
+  "by offering '-fmodules-reduced-bmi' option in one phase compilation model (e.g., CMake). "
+  "Or if your build system support two phase compilation model, please contact the "
+  "build system authors to support reduced BMI and turn off the warning temporarily">,
+  InGroup<DiagGroup<"missing-reduced-bmi">>;
+
 def warn_drv_delayed_template_parsing_after_cxx20 : Warning<
   "-fdelayed-template-parsing is deprecated after C++20">,
   InGroup<DiagGroup<"delayed-template-parsing-in-cxx20">>;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2b9ee1a0e669ed..bd4a716db29b0a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3194,11 +3194,14 @@ defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf",
           "Perform ODR checks for decls in the global module fragment.">>,
   Group<f_Group>;
 
-def modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">,
+def modules_reduced_bmi : Flag<["-"], "fmodules-reduced-bmi">,
   Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
   HelpText<"Generate the reduced BMI">,
   MarshallingInfoFlag<FrontendOpts<"GenReducedBMI">>;
 
+def experimental_modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">, 
+  Group<f_Group>, Visibility<[ClangOption, CC1Option]>, Alias<modules_reduced_bmi>;
+
 def fmodules_embed_all_files : Joined<["-"], "fmodules-embed-all-files">,
   Visibility<[ClangOption, CC1Option, CLOption]>,
   HelpText<"Embed the contents of all files read by this compilation into "
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 4c6f508f1f24a6..b67993d7b84d42 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4255,7 +4255,7 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
   if (Args.hasArg(options::OPT_modules_reduced_bmi) &&
       (Input.getType() == driver::types::TY_CXXModule ||
        Input.getType() == driver::types::TY_PP_CXXModule)) {
-    CmdArgs.push_back("-fexperimental-modules-reduced-bmi");
+    CmdArgs.push_back("-fmodules-reduced-bmi");
 
     if (Args.hasArg(options::OPT_fmodule_output_EQ))
       Args.AddLastArg(CmdArgs, options::OPT_fmodule_output_EQ);
@@ -4265,7 +4265,15 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
           getCXX20NamedModuleOutputPath(Args, Input.getBaseInput())));
   }
 
-  // Noop if we see '-fexperimental-modules-reduced-bmi' with other translation
+  if ((Input.getType() == driver::types::TY_CXXModule ||
+       Input.getType() == driver::types::TY_PP_CXXModule) &&
+      (Args.hasArg(options::OPT_fmodule_output) ||
+       Args.hasArg(options::OPT_fmodule_output_EQ) ||
+       Args.hasArg(options::OPT__precompile)) &&
+      !Args.hasArg(options::OPT_modules_reduced_bmi))
+    D.Diag(diag::warn_drv_module_reduced_bmi_not_enabled);
+
+  // Noop if we see '-fmodules-reduced-bmi' with other translation
   // units than module units. This is more user friendly to allow end uers to
   // enable this feature without asking for help from build systems.
   Args.ClaimAllArgs(options::OPT_modules_reduced_bmi);
diff --git a/clang/test/Driver/module-fgen-reduced-bmi.cppm b/clang/test/Driver/module-fgen-reduced-bmi.cppm
index 1223189fb49b72..af98684b5f95f9 100644
--- a/clang/test/Driver/module-fgen-reduced-bmi.cppm
+++ b/clang/test/Driver/module-fgen-reduced-bmi.cppm
@@ -29,13 +29,35 @@
 //
 // RUN: %clang -std=c++20 Hello.cc -fexperimental-modules-reduced-bmi -Wall -Werror \
 // RUN:     -c -o Hello.o -### 2>&1 | FileCheck Hello.cc
+//
+// RUN: %clang -std=c++20 Hello.cppm -fmodule-output=Hello.pcm \
+// RUN:     -fmodules-reduced-bmi -c -o Hello.o -### 2>&1 | FileCheck Hello.cppm
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -o Hello.o -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-UNSPECIFIED
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-NO-O
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -o AnotherName.o -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-ANOTHER-NAME
+//
+// RUN: %clang -std=c++20 Hello.cppm --precompile -fmodules-reduced-bmi \
+// RUN:     -o Hello.full.pcm -### 2>&1 | FileCheck Hello.cppm \
+// RUN:     --check-prefix=CHECK-EMIT-MODULE-INTERFACE
+//
+// RUN: %clang -std=c++20 Hello.cc -fmodules-reduced-bmi -Wall -Werror \
+// RUN:     -c -o Hello.o -### 2>&1 | FileCheck Hello.cc
 
 //--- Hello.cppm
 export module Hello;
 
 // Test that we won't generate the emit-module-interface as 2 phase compilation model.
 // CHECK-NOT: -emit-module-interface
-// CHECK: "-fexperimental-modules-reduced-bmi"
+// CHECK: "-fmodules-reduced-bmi"
 
 // CHECK-UNSPECIFIED: -fmodule-output=Hello.pcm
 
@@ -48,4 +70,4 @@ export module Hello;
 
 //--- Hello.cc
 
-// CHECK-NOT: "-fexperimental-modules-reduced-bmi"
+// CHECK-NOT: "-fmodules-reduced-bmi"

@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2024

@llvm/pr-subscribers-clang-driver

Author: Chuanqi Xu (ChuanqiXu9)

Changes

According to our previous consensus in https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi, the reduced BMI will be the default and recommend users to use the new option.

The -fexperimental-modules-reduced-bmi option is introduced in #85050 in Mar13 and released in 19.x. And now we are in 20's release cycle. Also I rarely receive issue reports about reduced BMI. No matter it is due to the quality of reduced BMI is really good or no one uses it. I think we should speed up the process to get more people get involved.

This patch literally did the second point in https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi


Full diff: https://github.com/llvm/llvm-project/pull/114382.diff

5 Files Affected:

  • (modified) clang/docs/StandardCPlusPlusModules.rst (+9-9)
  • (modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+8)
  • (modified) clang/include/clang/Driver/Options.td (+4-1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+10-2)
  • (modified) clang/test/Driver/module-fgen-reduced-bmi.cppm (+24-2)
diff --git a/clang/docs/StandardCPlusPlusModules.rst b/clang/docs/StandardCPlusPlusModules.rst
index 8e22adad15106e..26b29337ce42cb 100644
--- a/clang/docs/StandardCPlusPlusModules.rst
+++ b/clang/docs/StandardCPlusPlusModules.rst
@@ -602,16 +602,16 @@ unnecessary dependencies for the BMI. To mitigate the problem, Clang has a
 compiler option to reduce the information contained in the BMI. These two
 formats are known as Full BMI and Reduced BMI, respectively.
 
-Users can use the ``-fexperimental-modules-reduced-bmi`` option to produce a
+Users can use the ``-fmodules-reduced-bmi`` option to produce a
 Reduced BMI.
 
 For the one-phase compilation model (CMake implements this model), with
-``-fexperimental-modules-reduced-bmi``, the generated BMI will be a Reduced
+``-fmodules-reduced-bmi``, the generated BMI will be a Reduced
 BMI automatically. (The output path of the BMI is specified by
 ``-fmodule-output=`` as usual with the one-phase compilation model).
 
 It is also possible to produce a Reduced BMI with the two-phase compilation
-model. When ``-fexperimental-modules-reduced-bmi``, ``--precompile``, and
+model. When ``-fmodules-reduced-bmi``, ``--precompile``, and
 ``-fmodule-output=`` are specified, the generated BMI specified by ``-o`` will
 be a full BMI and the BMI specified by ``-fmodule-output=`` will be a Reduced
 BMI. The dependency graph in this case would look like:
@@ -625,7 +625,7 @@ BMI. The dependency graph in this case would look like:
                                                -> ...
                                                -> consumer_n.cpp
 
-Clang does not emit diagnostics when ``-fexperimental-modules-reduced-bmi`` is
+Clang does not emit diagnostics when ``-fmodules-reduced-bmi`` is
 used with a non-module unit. This design permits users of the one-phase
 compilation model to try using reduced BMIs without needing to modify the build
 system. The two-phase compilation module requires build system support.
@@ -691,7 +691,7 @@ ensure it is reachable, e.g. ``using N::g;``.
 Support for Reduced BMIs is still experimental, but it may become the default
 in the future. The expected roadmap for Reduced BMIs as of Clang 19.x is:
 
-1. ``-fexperimental-modules-reduced-bmi`` is opt-in for 1~2 releases. The period depends
+1. ``-fmodules-reduced-bmi`` is opt-in for 1~2 releases. The period depends
    on user feedback and may be extended.
 2. Announce that Reduced BMIs are no longer experimental and introduce
    ``-fmodules-reduced-bmi`` as a new option, and recommend use of the new
@@ -814,8 +814,8 @@ With reduced BMI, non-cascading changes can be more powerful. For example,
 
 .. code-block:: console
 
-  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fexperimental-modules-reduced-bmi -o A.o
-  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fexperimental-modules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
+  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fmodules-reduced-bmi -o A.o
+  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fmodules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
   $ md5sum B.pcm
   6c2bd452ca32ab418bf35cd141b060b9  B.pcm
 
@@ -831,8 +831,8 @@ and recompile the example:
 
 .. code-block:: console
 
-  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fexperimental-modules-reduced-bmi -o A.o
-  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fexperimental-modules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
+  $ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm  -fmodules-reduced-bmi -o A.o
+  $ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm  -fmodules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
   $ md5sum B.pcm
   6c2bd452ca32ab418bf35cd141b060b9  B.pcm
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 65551bd7761a9d..7589c6f52419dd 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -556,6 +556,14 @@ def err_test_module_file_extension_format : Error<
 def err_drv_module_output_with_multiple_arch : Error<
   "option '-fmodule-output' can't be used with multiple arch options">;
 
+def warn_drv_module_reduced_bmi_not_enabled : Warning<
+  "reduced BMI is expected to be enabled by default in Clang 21. It is encouraged to "
+  "enable it ahead of time to avoid potential breaking change. You can enable it "
+  "by offering '-fmodules-reduced-bmi' option in one phase compilation model (e.g., CMake). "
+  "Or if your build system support two phase compilation model, please contact the "
+  "build system authors to support reduced BMI and turn off the warning temporarily">,
+  InGroup<DiagGroup<"missing-reduced-bmi">>;
+
 def warn_drv_delayed_template_parsing_after_cxx20 : Warning<
   "-fdelayed-template-parsing is deprecated after C++20">,
   InGroup<DiagGroup<"delayed-template-parsing-in-cxx20">>;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2b9ee1a0e669ed..bd4a716db29b0a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3194,11 +3194,14 @@ defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf",
           "Perform ODR checks for decls in the global module fragment.">>,
   Group<f_Group>;
 
-def modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">,
+def modules_reduced_bmi : Flag<["-"], "fmodules-reduced-bmi">,
   Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
   HelpText<"Generate the reduced BMI">,
   MarshallingInfoFlag<FrontendOpts<"GenReducedBMI">>;
 
+def experimental_modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">, 
+  Group<f_Group>, Visibility<[ClangOption, CC1Option]>, Alias<modules_reduced_bmi>;
+
 def fmodules_embed_all_files : Joined<["-"], "fmodules-embed-all-files">,
   Visibility<[ClangOption, CC1Option, CLOption]>,
   HelpText<"Embed the contents of all files read by this compilation into "
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 4c6f508f1f24a6..b67993d7b84d42 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4255,7 +4255,7 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
   if (Args.hasArg(options::OPT_modules_reduced_bmi) &&
       (Input.getType() == driver::types::TY_CXXModule ||
        Input.getType() == driver::types::TY_PP_CXXModule)) {
-    CmdArgs.push_back("-fexperimental-modules-reduced-bmi");
+    CmdArgs.push_back("-fmodules-reduced-bmi");
 
     if (Args.hasArg(options::OPT_fmodule_output_EQ))
       Args.AddLastArg(CmdArgs, options::OPT_fmodule_output_EQ);
@@ -4265,7 +4265,15 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
           getCXX20NamedModuleOutputPath(Args, Input.getBaseInput())));
   }
 
-  // Noop if we see '-fexperimental-modules-reduced-bmi' with other translation
+  if ((Input.getType() == driver::types::TY_CXXModule ||
+       Input.getType() == driver::types::TY_PP_CXXModule) &&
+      (Args.hasArg(options::OPT_fmodule_output) ||
+       Args.hasArg(options::OPT_fmodule_output_EQ) ||
+       Args.hasArg(options::OPT__precompile)) &&
+      !Args.hasArg(options::OPT_modules_reduced_bmi))
+    D.Diag(diag::warn_drv_module_reduced_bmi_not_enabled);
+
+  // Noop if we see '-fmodules-reduced-bmi' with other translation
   // units than module units. This is more user friendly to allow end uers to
   // enable this feature without asking for help from build systems.
   Args.ClaimAllArgs(options::OPT_modules_reduced_bmi);
diff --git a/clang/test/Driver/module-fgen-reduced-bmi.cppm b/clang/test/Driver/module-fgen-reduced-bmi.cppm
index 1223189fb49b72..af98684b5f95f9 100644
--- a/clang/test/Driver/module-fgen-reduced-bmi.cppm
+++ b/clang/test/Driver/module-fgen-reduced-bmi.cppm
@@ -29,13 +29,35 @@
 //
 // RUN: %clang -std=c++20 Hello.cc -fexperimental-modules-reduced-bmi -Wall -Werror \
 // RUN:     -c -o Hello.o -### 2>&1 | FileCheck Hello.cc
+//
+// RUN: %clang -std=c++20 Hello.cppm -fmodule-output=Hello.pcm \
+// RUN:     -fmodules-reduced-bmi -c -o Hello.o -### 2>&1 | FileCheck Hello.cppm
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -o Hello.o -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-UNSPECIFIED
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-NO-O
+//
+// RUN: %clang -std=c++20 Hello.cppm \
+// RUN:     -fmodules-reduced-bmi -c -o AnotherName.o -### 2>&1 | \
+// RUN:         FileCheck Hello.cppm --check-prefix=CHECK-ANOTHER-NAME
+//
+// RUN: %clang -std=c++20 Hello.cppm --precompile -fmodules-reduced-bmi \
+// RUN:     -o Hello.full.pcm -### 2>&1 | FileCheck Hello.cppm \
+// RUN:     --check-prefix=CHECK-EMIT-MODULE-INTERFACE
+//
+// RUN: %clang -std=c++20 Hello.cc -fmodules-reduced-bmi -Wall -Werror \
+// RUN:     -c -o Hello.o -### 2>&1 | FileCheck Hello.cc
 
 //--- Hello.cppm
 export module Hello;
 
 // Test that we won't generate the emit-module-interface as 2 phase compilation model.
 // CHECK-NOT: -emit-module-interface
-// CHECK: "-fexperimental-modules-reduced-bmi"
+// CHECK: "-fmodules-reduced-bmi"
 
 // CHECK-UNSPECIFIED: -fmodule-output=Hello.pcm
 
@@ -48,4 +70,4 @@ export module Hello;
 
 //--- Hello.cc
 
-// CHECK-NOT: "-fexperimental-modules-reduced-bmi"
+// CHECK-NOT: "-fmodules-reduced-bmi"

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! We should definitely have a release note so users are more aware of the changes.

Comment on lines 560 to 564
"reduced BMI is expected to be enabled by default in Clang 21. It is encouraged to "
"enable it ahead of time to avoid potential breaking change. You can enable it "
"by offering '-fmodules-reduced-bmi' option in one phase compilation model (e.g., CMake). "
"Or if your build system support two phase compilation model, please contact the "
"build system authors to support reduced BMI and turn off the warning temporarily">,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"reduced BMI is expected to be enabled by default in Clang 21. It is encouraged to "
"enable it ahead of time to avoid potential breaking change. You can enable it "
"by offering '-fmodules-reduced-bmi' option in one phase compilation model (e.g., CMake). "
"Or if your build system support two phase compilation model, please contact the "
"build system authors to support reduced BMI and turn off the warning temporarily">,
"reduced BMI is expected to be enabled by default in Clang 21; it can be enabled explicitly "
"by passing '-fmodules-reduced-bmi' (in one-phase compilation models, like CMake) "
"or by disabling the diagnostic and requesting the build system vendor support "
"reduced BMIs (in two-phase compilation models)">,

Oof, this is a bit tough -- diagnostics need to be terse and should not contain full sentences. I took a stab at rewording for our usual diagnostic style.

Probably need to re-flow this for 80 col limits

This diagnostic is missing test coverage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I feel the new wording is pretty cool and fine enough : )

@@ -691,7 +691,7 @@ ensure it is reachable, e.g. ``using N::g;``.
Support for Reduced BMIs is still experimental, but it may become the default
in the future. The expected roadmap for Reduced BMIs as of Clang 19.x is:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
in the future. The expected roadmap for Reduced BMIs as of Clang 19.x is:
in the future. The expected roadmap for Reduced BMIs as of Clang 20.x is:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked up the dictionary and it says "as of" means "from". So I feel the wording is good since it was added in 19.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's confusing to update/clarify the plan during the 20.x cycle and then not update the "as of 19.x" (even though the overall shape of the plan didn't change). If you want to keep the "as of 19.x", then we should specify which steps have been done already (see below), though IMO any updates here should also update the "as of".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done as suggested.

@@ -691,7 +691,7 @@ ensure it is reachable, e.g. ``using N::g;``.
Support for Reduced BMIs is still experimental, but it may become the default
in the future. The expected roadmap for Reduced BMIs as of Clang 19.x is:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's confusing to update/clarify the plan during the 20.x cycle and then not update the "as of 19.x" (even though the overall shape of the plan didn't change). If you want to keep the "as of 19.x", then we should specify which steps have been done already (see below), though IMO any updates here should also update the "as of".

Comment on lines 694 to 697
1. ``-fmodules-reduced-bmi`` is opt-in for 1~2 releases. The period depends
on user feedback and may be extended.
2. Announce that Reduced BMIs are no longer experimental and introduce
``-fmodules-reduced-bmi`` as a new option, and recommend use of the new
Copy link
Contributor

@h-vetinari h-vetinari Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

w.r.t. keeping the "as of 19.x": since removing experimental- prefix was part of the plan since v19, the first point should either be marked as done (or simply removed), but not just renamed (which would be inconsistent both with the "as of 19.x", as well as the following item that already covers the same point). For example:

Suggested change
1. ``-fmodules-reduced-bmi`` is opt-in for 1~2 releases. The period depends
on user feedback and may be extended.
2. Announce that Reduced BMIs are no longer experimental and introduce
``-fmodules-reduced-bmi`` as a new option, and recommend use of the new
1. ``-fexperimental-modules-reduced-bmi`` was introduced in v19.x
2. For v20.x, ``-fmodules-reduced-bmi`` is introduced as an equivalent non-experimental
option. It is expected to stay opt-in for 1~2 releases, though the period depends
on user feedback and may be extended.
3. Finally, [...]

Copy link
Contributor

@mizvekov mizvekov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the overall plan is fine, thanks for working on it!

I think the idea of issuing a warning in order to get users to opt-in to a flag is somewhat novel. Have I missed discussion about this approach, versus simply changing the default?

Regardless, for any new warning we add here, we will probably need to add a targeted way to disable that warning, as otherwise users of Werror are likely to complain.

@ChuanqiXu9
Copy link
Member Author

I think the overall plan is fine, thanks for working on it!

I think the idea of issuing a warning in order to get users to opt-in to a flag is somewhat novel. Have I missed discussion about this approach, versus simply changing the default?

I originally mentioned it in #85050 and then in the document. And it looks like no one had a question about it.

Regardless, for any new warning we add here, we will probably need to add a targeted way to disable that warning, as otherwise users of Werror are likely to complain.

We can disable it with the same way with other warnings, e.g., -Wno-missing-reduced-bmi

@ChuanqiXu9
Copy link
Member Author

I'd like to land this in the next week if no objection comes in.

@ChuanqiXu9
Copy link
Member Author

If there are more opinions, let's continue it in post commit review.

@ChuanqiXu9 ChuanqiXu9 requested a review from a team as a code owner November 27, 2024 02:30
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 27, 2024
@ChuanqiXu9
Copy link
Member Author

I forgot this affects modules support in libc++. Let's wait for the review from libc++ folks.

@ChuanqiXu9
Copy link
Member Author

It looks the CI will compile libc++ with "old" clang19 and the just built clang. So it might not be able to recognize the new flag. Tried to use -fexperimental-modules-reduced-bmi for libc++. We can replace it with -fmodules-reduced-bmi someday after we don't compile it with clang19 and previous version.

@ChuanqiXu9
Copy link
Member Author

Oh, I didn't notice libcxx will test with clang17 and clang18 too. Where we didn't introduce reduced bmi. We can't use -Wno-missing-reduced-bmi since it won't be recognized by clang17 and clang18.

@ldionne @mordante do you think how can we proceed on this?

@ChuanqiXu9
Copy link
Member Author

@ldionne @mordante ping~

@mizvekov
Copy link
Contributor

mizvekov commented Dec 6, 2024

I still find the current approach wrt the warning to be odd, we would basically force every modules user to change their command line, either by adding the new -fmodules-reduced-bmi, or add -Wno-wathever to suppress the warning, or just learn to live with the warning I guess.

I can't recall prior art on this.

If we think this has such a high chance of causing breakages, perhaps we shouldn't warn at all. Otherwise, we should just change the default.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't have time to gather much context around this patch, sorry, I just noticed your question on Discord.

@@ -172,6 +172,7 @@ def parseScript(test, preamble):
f"{compileFlags} "
"-Wno-reserved-module-identifier -Wno-reserved-user-defined-literal "
"-fmodule-file=std=%T/std.pcm " # The std.compat module imports std.
"-fexperimental-modules-reduced-bmi "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the intent to have people using a -fexperimental-foo flag whenever they compile with C++20 modules enabled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@@ -188,6 +189,7 @@ def parseScript(test, preamble):
"%dbg(MODULE std) %{cxx} %{flags} "
f"{compileFlags} "
"-Wno-reserved-module-identifier -Wno-reserved-user-defined-literal "
"-fexperimental-modules-reduced-bmi "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is the "new way" of building with C++20 modules, then I think it would make sense to mark the offending libc++ tests as UNSUPPORTED: clang-18, clang-19. But that does seem like a big user-facing change too, doesn't it?

Copy link
Member Author

@ChuanqiXu9 ChuanqiXu9 Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so I'd like to accept @mizvekov 's suggestion to not emit the warning to avoid these failures in users code.

@ChuanqiXu9
Copy link
Member Author

ChuanqiXu9 commented Dec 10, 2024

I still find the current approach wrt the warning to be odd, we would basically force every modules user to change their command line, either by adding the new -fmodules-reduced-bmi, or add -Wno-wathever to suppress the warning, or just learn to live with the warning I guess.

I can't recall prior art on this.

If we think this has such a high chance of causing breakages, perhaps we shouldn't warn at all. Otherwise, we should just change the default.

Yeah, my intention is to get more users get involved during the cycle to make the final output more stable. But it is not traditional indeed. Given it makes some problems already, let's follow the traditional step in the beginning.

@ChuanqiXu9 ChuanqiXu9 merged commit 411196b into llvm:main Dec 10, 2024
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants