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

[via cfe-commits]

Author: Erich Keane
Date: 2024-04-18T12:40:51-07:00
New Revision: 8ba0041e158f8678e829c10efa73397afdba6b4f

URL: https://github.com/llvm/llvm-project/commit/8ba0041e158f8678e829c10efa73397afdba6b4f
DIFF: https://github.com/llvm/llvm-project/commit/8ba0041e158f8678e829c10efa73397afdba6b4f.diff

LOG: Stop double-diagnosing explicit convert operator in switch condition (#89142)

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.

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaExpr.cpp
    clang/lib/Sema/SemaOverload.cpp
    clang/test/SemaCXX/explicit.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3fe15934323c53..205c6e573d567c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -427,6 +427,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
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

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