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@+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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
