[clang] Revert default behavior change of P0522R0 implementation #91811

mizvekov · 2024-05-10T21:11:38Z

This partially reverts b86e099.

Just the default is changed back, on the Driver side.
No Frontend changes.
The positive spelling of the flag is undeprecated.

No documentation changes or changelog entries because we plan to revert this revert as soon as #62529 is fixed.

This partially reverts b86e099 Just the default is changed back, on the Driver side. No Frontend changes. The positive spelling of the flag is undeprecated. No documentation changes or changelog entries because we plan to revert this revert as soon as llvm#62529 is fixed.

llvmbot · 2024-05-10T21:12:13Z

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This partially reverts b86e099.

Just the default is changed back, on the Driver side.
No Frontend changes.
The positive spelling of the flag is undeprecated.

No documentation changes or changelog entries because we plan to revert this revert as soon as #62529 is fixed.

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

2 Files Affected:

(modified) clang/lib/Driver/ToolChains/Clang.cpp (+7-7)
(modified) clang/test/Driver/frelaxed-template-template-args.cpp (+4-2)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 42feb1650574e..f0cc018b6668f 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7249,15 +7249,15 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   Args.addOptOutFlag(CmdArgs, options::OPT_fassume_unique_vtables,
                      options::OPT_fno_assume_unique_vtables);
 
-  // -frelaxed-template-template-args is deprecated.
-  if (Arg *A =
-          Args.getLastArg(options::OPT_frelaxed_template_template_args,
-                          options::OPT_fno_relaxed_template_template_args)) {
+  // -fno-relaxed-template-template-args is deprecated.
+  if (Arg *A = Args.getLastArg(options::OPT_frelaxed_template_template_args,
+                               options::OPT_fno_relaxed_template_template_args);
+      A &&
+      A->getOption().matches(options::OPT_fno_relaxed_template_template_args))
     D.Diag(diag::warn_drv_deprecated_arg)
         << A->getAsString(Args) << /*hasReplacement=*/false;
-    if (A->getOption().matches(options::OPT_fno_relaxed_template_template_args))
-      CmdArgs.push_back("-fno-relaxed-template-template-args");
-  }
+  else
+    CmdArgs.push_back("-fno-relaxed-template-template-args");
 
   // -fsized-deallocation is off by default, as it is an ABI-breaking change for
   // most platforms.
diff --git a/clang/test/Driver/frelaxed-template-template-args.cpp b/clang/test/Driver/frelaxed-template-template-args.cpp
index dd6265ba8375e..136c360276a15 100644
--- a/clang/test/Driver/frelaxed-template-template-args.cpp
+++ b/clang/test/Driver/frelaxed-template-template-args.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang -fsyntax-only -frelaxed-template-template-args %s 2>&1 | FileCheck --check-prefix=CHECK-ON %s
+// RUN: %clang -fsyntax-only -### %s 2>&1 | FileCheck --check-prefix=CHECK-DEF %s
+// RUN: %clang -fsyntax-only -frelaxed-template-template-args %s 2>&1 | FileCheck --check-prefix=CHECK-ON --allow-empty %s
 // RUN: %clang -fsyntax-only -fno-relaxed-template-template-args %s 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
 
-// CHECK-ON:  warning: argument '-frelaxed-template-template-args' is deprecated [-Wdeprecated]
+// CHECK-DEF: "-cc1"{{.*}} "-fno-relaxed-template-template-args"
+// CHECK-ON-NOT: warning: argument '-frelaxed-template-template-args' is deprecated [-Wdeprecated]
 // CHECK-OFF: warning: argument '-fno-relaxed-template-template-args' is deprecated [-Wdeprecated]

llvmbot · 2024-05-10T21:12:13Z

@llvm/pr-subscribers-clang-driver

Author: Matheus Izvekov (mizvekov)

Changes

This partially reverts b86e099.

Just the default is changed back, on the Driver side.
No Frontend changes.
The positive spelling of the flag is undeprecated.

No documentation changes or changelog entries because we plan to revert this revert as soon as #62529 is fixed.

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

2 Files Affected:

(modified) clang/lib/Driver/ToolChains/Clang.cpp (+7-7)
(modified) clang/test/Driver/frelaxed-template-template-args.cpp (+4-2)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 42feb1650574e..f0cc018b6668f 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7249,15 +7249,15 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   Args.addOptOutFlag(CmdArgs, options::OPT_fassume_unique_vtables,
                      options::OPT_fno_assume_unique_vtables);
 
-  // -frelaxed-template-template-args is deprecated.
-  if (Arg *A =
-          Args.getLastArg(options::OPT_frelaxed_template_template_args,
-                          options::OPT_fno_relaxed_template_template_args)) {
+  // -fno-relaxed-template-template-args is deprecated.
+  if (Arg *A = Args.getLastArg(options::OPT_frelaxed_template_template_args,
+                               options::OPT_fno_relaxed_template_template_args);
+      A &&
+      A->getOption().matches(options::OPT_fno_relaxed_template_template_args))
     D.Diag(diag::warn_drv_deprecated_arg)
         << A->getAsString(Args) << /*hasReplacement=*/false;
-    if (A->getOption().matches(options::OPT_fno_relaxed_template_template_args))
-      CmdArgs.push_back("-fno-relaxed-template-template-args");
-  }
+  else
+    CmdArgs.push_back("-fno-relaxed-template-template-args");
 
   // -fsized-deallocation is off by default, as it is an ABI-breaking change for
   // most platforms.
diff --git a/clang/test/Driver/frelaxed-template-template-args.cpp b/clang/test/Driver/frelaxed-template-template-args.cpp
index dd6265ba8375e..136c360276a15 100644
--- a/clang/test/Driver/frelaxed-template-template-args.cpp
+++ b/clang/test/Driver/frelaxed-template-template-args.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang -fsyntax-only -frelaxed-template-template-args %s 2>&1 | FileCheck --check-prefix=CHECK-ON %s
+// RUN: %clang -fsyntax-only -### %s 2>&1 | FileCheck --check-prefix=CHECK-DEF %s
+// RUN: %clang -fsyntax-only -frelaxed-template-template-args %s 2>&1 | FileCheck --check-prefix=CHECK-ON --allow-empty %s
 // RUN: %clang -fsyntax-only -fno-relaxed-template-template-args %s 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
 
-// CHECK-ON:  warning: argument '-frelaxed-template-template-args' is deprecated [-Wdeprecated]
+// CHECK-DEF: "-cc1"{{.*}} "-fno-relaxed-template-template-args"
+// CHECK-ON-NOT: warning: argument '-frelaxed-template-template-args' is deprecated [-Wdeprecated]
 // CHECK-OFF: warning: argument '-fno-relaxed-template-template-args' is deprecated [-Wdeprecated]

erichkeane

This should probably also change the release note that still says we've changed this to on by default.

erichkeane · 2024-05-10T21:20:02Z

clang/lib/Driver/ToolChains/Clang.cpp

-  if (Arg *A =
-          Args.getLastArg(options::OPT_frelaxed_template_template_args,
-                          options::OPT_fno_relaxed_template_template_args)) {
+  // -fno-relaxed-template-template-args is deprecated.


I'm not sure I get the case for marking this deprecated?

cor3ntin · 2024-05-10T21:32:24Z

@erichkeane The goal is to provide a temporary working default, and a proper fix next fix, so that folks who are impacted by the break (everyone working on stdexec) isn't impacted longer than necessary. So I'll go and merge that and we can discuss a longer-term solution monday

A few tests were checking for all driver flags. We should revert this commit when we revert llvm#91811

A few tests were checking for all driver flags. We should revert this commit when we revert #91811

…ion (#91811)" With blocking issues fixed, re-enable relaxed template template argument matching by reverting these commits. This reverts commit 4198aeb. This reverts commit 2d5634a.

…ion (#91811)" (#91837) With blocking issues fixed, re-enable relaxed template template argument matching by reverting these commits. This reverts commit 4198aeb. This reverts commit 2d5634a.

mizvekov requested a review from cor3ntin May 10, 2024 21:11

mizvekov self-assigned this May 10, 2024

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels May 10, 2024

cor3ntin approved these changes May 10, 2024

View reviewed changes

erichkeane reviewed May 10, 2024

View reviewed changes

cor3ntin merged commit 2d5634a into llvm:main May 10, 2024
6 of 7 checks passed

This was referenced May 10, 2024

[clang] Enable C++17 relaxed template template argument matching by default #89807

Merged

[clang][regression][rejects-valid] clang trunk rejecting valid pack expansion #91787

Closed

cor3ntin added a commit to cor3ntin/llvm-project that referenced this pull request May 10, 2024

[Clang] Fix tests broken by llvm#91811

f2acd71

A few tests were checking for all driver flags. We should revert this commit when we revert llvm#91811

cor3ntin mentioned this pull request May 10, 2024

[Clang] Fix tests broken by #91811 #91822

Merged

cor3ntin added a commit that referenced this pull request May 10, 2024

[Clang] Fix tests broken by #91811 (#91822)

4198aeb

A few tests were checking for all driver flags. We should revert this commit when we revert #91811

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang] Revert default behavior change of P0522R0 implementation #91811

[clang] Revert default behavior change of P0522R0 implementation #91811

mizvekov commented May 10, 2024

llvmbot commented May 10, 2024

llvmbot commented May 10, 2024

erichkeane left a comment

erichkeane May 10, 2024

cor3ntin commented May 10, 2024

[clang] Revert default behavior change of P0522R0 implementation #91811

[clang] Revert default behavior change of P0522R0 implementation #91811

Conversation

mizvekov commented May 10, 2024

llvmbot commented May 10, 2024

llvmbot commented May 10, 2024

erichkeane left a comment

Choose a reason for hiding this comment

erichkeane May 10, 2024

Choose a reason for hiding this comment

cor3ntin commented May 10, 2024