[clang] [C++20] [Modules] [Driver] Don't enable -fdelayed-template-parsing by default on windows with C++20 modules (PR #69431)

via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 18 01:06:06 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

<details>
<summary>Changes</summary>

There are already 3 issues about the broken state of -fdelayed-template-parsing and C++20 modules:
- https://github.com/llvm/llvm-project/issues/61068
- https://github.com/llvm/llvm-project/issues/64810
- https://github.com/llvm/llvm-project/issues/65027

The problem is more complex than I thought. I am not sure how to fix it properly now. Given the complexities and -fdelayed-template-parsing is actually an extension to support old MS codes, I think it may make sense to not enable the -fdelayed-template-parsing option by default with C++20 modules to give more user friendly experience. Users who still want -fdelayed-template-parsing can specify it explicitly.

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


5 Files Affected:

- (modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+4) 
- (modified) clang/include/clang/Driver/Types.h (+3) 
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+17-8) 
- (modified) clang/lib/Driver/Types.cpp (+4) 
- (added) clang/test/Driver/cl-delayed-template-parsing-modules.cppm (+7) 


``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 9c00fa50bb95580..0a64f8573931f37 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -527,6 +527,10 @@ 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_delayed_template_parsing_with_module : Warning<
+  "-fdelayed-template-parsing is broken with C++20 modules">,
+  InGroup<DiagGroup<"delayed-template-parsing-in-modules">>;
+
 def err_drv_extract_api_wrong_kind : Error<
   "header file '%0' input '%1' does not match the type of prior input "
   "in api extraction; use '-x %2' to override">;
diff --git a/clang/include/clang/Driver/Types.h b/clang/include/clang/Driver/Types.h
index 121b58a6b477d9b..b6a05e1ec9d624a 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);
 
+  /// isCXXModules - Is this a standard C++ modules input.
+  bool isCXXModules(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 94c184435ae14de..21aee28a2b6ba9b 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6842,14 +6842,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                         (!IsWindowsMSVC || IsMSVC2015Compatible)))
     CmdArgs.push_back("-fno-threadsafe-statics");
 
-  // -fno-delayed-template-parsing is default, except when targeting MSVC.
-  // Many old Windows SDK versions require this to parse.
-  // FIXME: MSVC introduced /Zc:twoPhase- to disable this behavior in their
-  // compiler. We should be able to disable this by default at some point.
-  if (Args.hasFlag(options::OPT_fdelayed_template_parsing,
-                   options::OPT_fno_delayed_template_parsing, IsWindowsMSVC))
-    CmdArgs.push_back("-fdelayed-template-parsing");
-
   // -fgnu-keywords default varies depending on language; only pass if
   // specified.
   Args.AddLastArg(CmdArgs, options::OPT_fgnu_keywords,
@@ -6872,6 +6864,23 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   bool HaveModules =
       RenderModulesOptions(C, D, Args, Input, Output, Std, CmdArgs);
+  bool IsModulesInput = HaveModules && isCXXModules(Input.getType());
+  // It is possible to compile .pcm files with C++20 modules.
+  IsModulesInput |= Input.getType() == types::TY_ModuleFile;
+
+  // -fno-delayed-template-parsing is default, except when targeting MSVC.
+  // Many old Windows SDK versions require this to parse.
+  // FIXME: MSVC introduced /Zc:twoPhase- to disable this behavior in their
+  // compiler. We should be able to disable this by default at some point.
+  // FIXME: The -fdelayed-template-parsing mode is broken with C++20 modules.
+  if (Args.hasFlag(options::OPT_fdelayed_template_parsing,
+                   options::OPT_fno_delayed_template_parsing,
+                   IsWindowsMSVC && !IsModulesInput)) {
+    if (IsModulesInput)
+      D.Diag(clang::diag::warn_drv_delayed_template_parsing_with_module);
+
+    CmdArgs.push_back("-fdelayed-template-parsing");
+  }
 
   if (Args.hasFlag(options::OPT_fpch_validate_input_files_content,
                    options::OPT_fno_pch_validate_input_files_content, false))
diff --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index 08df34ade7b6530..6977b5509c26a9b 100644
--- a/clang/lib/Driver/Types.cpp
+++ b/clang/lib/Driver/Types.cpp
@@ -249,6 +249,10 @@ bool types::isCXX(ID Id) {
   }
 }
 
+bool types::isCXXModules(ID Id) {
+  return Id == TY_CXXModule || Id == TY_PP_CXXModule;
+}
+
 bool types::isLLVMIR(ID Id) {
   switch (Id) {
   default:
diff --git a/clang/test/Driver/cl-delayed-template-parsing-modules.cppm b/clang/test/Driver/cl-delayed-template-parsing-modules.cppm
new file mode 100644
index 000000000000000..fee12ae7034aed2
--- /dev/null
+++ b/clang/test/Driver/cl-delayed-template-parsing-modules.cppm
@@ -0,0 +1,7 @@
+// RUN: %clang_cl -std:c++20 -### -- %s 2>&1 | FileCheck %s
+// RUN: %clang_cl -std:c++20 -### -fdelayed-template-parsing -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-EXPLICIT
+
+// CHECK-NOT: -fdelayed-template-parsing
+
+// CHECK-EXPLICIT: -fdelayed-template-parsing
+// CHECK-EXPLICIT: warning: -fdelayed-template-parsing is broken with C++20 modules

``````````

</details>


https://github.com/llvm/llvm-project/pull/69431


More information about the cfe-commits mailing list