[clang] Stop double-diagnosing explicit convert operator in switch condition (PR #89142)

[Erich Keane via cfe-commits]

https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/89142

>From 43f1915f919d82f055e0c7147d3c46013d56feef Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Wed, 17 Apr 2024 11:46:36 -0700
Subject: [PATCH 1/2] Stop double-diagnosing explicit convert operator in
 switch condition

Note this also likely fixes a bunch of other cases.  We were
double-diagnosting in a template because we were generating the
expression anyway, so any attempts to instantiate the function would
instantiate the expression, thus re-diagnosing it.

This patch replaces it with a RecoveryExpr.  Additionally,
VerifyIntegerConstantExpression couldn't handle the RecoveryExpr, as it
requires a non-dependent expression result (which a RecoveryExpr is
dependent).  Additionally, callers of it use the return value to decide
that VerifyIntegerConstantExpression succeeded, so it fails if it sees a
RecoveryExpr.
---
 clang/lib/Sema/SemaExpr.cpp     |  6 ++++++
 clang/lib/Sema/SemaOverload.cpp | 13 ++++++++-----
 clang/test/SemaCXX/explicit.cpp | 15 +++++++++++++++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d2c77ad61644f0..c7e1d1daafea99 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17522,6 +17522,12 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
     if (Converted.isInvalid())
       return Converted;
     E = Converted.get();
+    // The 'explicit' case causes us to get a RecoveryExpr.  Give up here so we
+    // don't try to evaluate it later. We also don't want to return the
+    // RecoveryExpr here, as it results in this call succeeding, thus callers of
+    // this function will attempt to use 'Value'.
+    if (isa<RecoveryExpr>(E))
+      return ExprError();
     if (!E->getType()->isIntegralOrUnscopedEnumerationType())
       return ExprError();
   } else if (!E->getType()->isIntegralOrUnscopedEnumerationType()) {
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index adc319e97b7625..48d6264029e9bf 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -6563,11 +6563,14 @@ diagnoseNoViableConversion(Sema &SemaRef, SourceLocation Loc, Expr *&From,
                                                        HadMultipleCandidates);
     if (Result.isInvalid())
       return true;
-    // Record usage of conversion in an implicit cast.
-    From = ImplicitCastExpr::Create(SemaRef.Context, Result.get()->getType(),
-                                    CK_UserDefinedConversion, Result.get(),
-                                    nullptr, Result.get()->getValueKind(),
-                                    SemaRef.CurFPFeatureOverrides());
+
+    // Replace the conversion with a RecoveryExpr, so we don't try to
+    // instantiate it later, but can further diagnose here.
+    Result = SemaRef.CreateRecoveryExpr(From->getBeginLoc(), From->getEndLoc(),
+                                        From, Result.get()->getType());
+    if (Result.isInvalid())
+      return true;
+    From = Result.get();
   }
   return false;
 }
diff --git a/clang/test/SemaCXX/explicit.cpp b/clang/test/SemaCXX/explicit.cpp
index ba2c36d99daf62..3bb04a4d62e6c8 100644
--- a/clang/test/SemaCXX/explicit.cpp
+++ b/clang/test/SemaCXX/explicit.cpp
@@ -266,3 +266,18 @@ namespace PR18777 {
   struct S { explicit operator bool() const; } s;
   int *p = new int(s); // expected-error {{no viable conversion}}
 }
+
+namespace DoubleDiags {
+  struct ExplicitConvert{
+    explicit operator int();//#DOUBLE_DIAG_OP_INT
+  } EC;
+  template<typename T>
+  void Template(){
+      // expected-error at +2{{switch condition type 'struct ExplicitConvert' requires explicit conversion to 'int'}}
+      // expected-note@#DOUBLE_DIAG_OP_INT{{conversion to integral type 'int'}}
+      switch(EC){}
+  };
+  void Inst() {
+    Template<int>();
+  }
+}

>From a8e1af9956ec63562a51f7be53aab3623dab51e6 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Thu, 18 Apr 2024 12:40:06 -0700
Subject: [PATCH 2/2] Update release note

---
 clang/docs/ReleaseNotes.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c19ad9fba58f37..b573ecbf3ec766 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -424,6 +424,10 @@ Bug Fixes in This Version
 
 - Fixed an assertion failure on invalid InitListExpr in C89 mode (#GH88008).
 
+- Clang will no longer diagnose an erroneous non-dependent ``switch`` condition
+  during instantiation, and instead will only diagnose it once, during checking
+  of the function template.
+
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^