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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

<details>
<summary>Changes</summary>

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.

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


3 Files Affected:

- (modified) clang/lib/Sema/SemaExpr.cpp (+6) 
- (modified) clang/lib/Sema/SemaOverload.cpp (+8-5) 
- (modified) clang/test/SemaCXX/explicit.cpp (+15) 


``````````diff
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>();
+  }
+}

``````````

</details>


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