PR11778: Fix the rejects-valid half of this bug. We still produce the same
poorly-worded warning for a case value that is not a possible value of the
switched-on expression.
llvm-svn: 214678
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 1ddb369..6bb71eb 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -481,47 +481,6 @@
thenStmt, ElseLoc, elseStmt);
}
-/// ConvertIntegerToTypeWarnOnOverflow - Convert the specified APInt to have
-/// the specified width and sign. If an overflow occurs, detect it and emit
-/// the specified diagnostic.
-void Sema::ConvertIntegerToTypeWarnOnOverflow(llvm::APSInt &Val,
- unsigned NewWidth, bool NewSign,
- SourceLocation Loc,
- unsigned DiagID) {
- // Perform a conversion to the promoted condition type if needed.
- if (NewWidth > Val.getBitWidth()) {
- // If this is an extension, just do it.
- Val = Val.extend(NewWidth);
- Val.setIsSigned(NewSign);
-
- // If the input was signed and negative and the output is
- // unsigned, don't bother to warn: this is implementation-defined
- // behavior.
- // FIXME: Introduce a second, default-ignored warning for this case?
- } else if (NewWidth < Val.getBitWidth()) {
- // If this is a truncation, check for overflow.
- llvm::APSInt ConvVal(Val);
- ConvVal = ConvVal.trunc(NewWidth);
- ConvVal.setIsSigned(NewSign);
- ConvVal = ConvVal.extend(Val.getBitWidth());
- ConvVal.setIsSigned(Val.isSigned());
- if (ConvVal != Val)
- Diag(Loc, DiagID) << Val.toString(10) << ConvVal.toString(10);
-
- // Regardless of whether a diagnostic was emitted, really do the
- // truncation.
- Val = Val.trunc(NewWidth);
- Val.setIsSigned(NewSign);
- } else if (NewSign != Val.isSigned()) {
- // Convert the sign to match the sign of the condition. This can cause
- // overflow as well: unsigned(INTMIN)
- // We don't diagnose this overflow, because it is implementation-defined
- // behavior.
- // FIXME: Introduce a second, default-ignored warning for this case?
- Val.setIsSigned(NewSign);
- }
-}
-
namespace {
struct CaseCompareFunctor {
bool operator()(const std::pair<llvm::APSInt, CaseStmt*> &LHS,
@@ -671,13 +630,30 @@
}
static void AdjustAPSInt(llvm::APSInt &Val, unsigned BitWidth, bool IsSigned) {
- if (Val.getBitWidth() < BitWidth)
- Val = Val.extend(BitWidth);
- else if (Val.getBitWidth() > BitWidth)
- Val = Val.trunc(BitWidth);
+ Val = Val.extOrTrunc(BitWidth);
Val.setIsSigned(IsSigned);
}
+/// Check the specified case value is in range for the given unpromoted switch
+/// type.
+static void checkCaseValue(Sema &S, SourceLocation Loc, const llvm::APSInt &Val,
+ unsigned UnpromotedWidth, bool UnpromotedSign) {
+ // If the case value was signed and negative and the switch expression is
+ // unsigned, don't bother to warn: this is implementation-defined behavior.
+ // FIXME: Introduce a second, default-ignored warning for this case?
+ if (UnpromotedWidth < Val.getBitWidth()) {
+ llvm::APSInt ConvVal(Val);
+ AdjustAPSInt(ConvVal, UnpromotedWidth, UnpromotedSign);
+ AdjustAPSInt(ConvVal, Val.getBitWidth(), Val.isSigned());
+ // FIXME: Use different diagnostics for overflow in conversion to promoted
+ // type versus "switch expression cannot have this value". Use proper
+ // IntRange checking rather than just looking at the unpromoted type here.
+ if (ConvVal != Val)
+ S.Diag(Loc, diag::warn_case_value_overflow) << Val.toString(10)
+ << ConvVal.toString(10);
+ }
+}
+
/// Returns true if we should emit a diagnostic about this case expression not
/// being a part of the enum used in the switch controlling expression.
static bool ShouldDiagnoseSwitchCaseNotInEnum(const ASTContext &Ctx,
@@ -744,13 +720,20 @@
}
}
- // Get the bitwidth of the switched-on value before promotions. We must
+ // Get the bitwidth of the switched-on value after promotions. We must
// convert the integer case values to this width before comparison.
bool HasDependentValue
= CondExpr->isTypeDependent() || CondExpr->isValueDependent();
- unsigned CondWidth
+ unsigned CondWidth = HasDependentValue ? 0 : Context.getIntWidth(CondType);
+ bool CondIsSigned = CondType->isSignedIntegerOrEnumerationType();
+
+ // Get the width and signedness that the condition might actually have, for
+ // warning purposes.
+ // FIXME: Grab an IntRange for the condition rather than using the unpromoted
+ // type.
+ unsigned CondWidthBeforePromotion
= HasDependentValue ? 0 : Context.getIntWidth(CondTypeBeforePromotion);
- bool CondIsSigned
+ bool CondIsSignedBeforePromotion
= CondTypeBeforePromotion->isSignedIntegerOrEnumerationType();
// Accumulate all of the case values in a vector so that we can sort them
@@ -816,15 +799,13 @@
Lo = ImpCastExprToType(Lo, CondType, CK_IntegralCast).get();
}
- // Convert the value to the same width/sign as the condition had prior to
- // integral promotions.
- //
- // FIXME: This causes us to reject valid code:
- // switch ((char)c) { case 256: case 0: return 0; }
- // Here we claim there is a duplicated condition value, but there is not.
- ConvertIntegerToTypeWarnOnOverflow(LoVal, CondWidth, CondIsSigned,
- Lo->getLocStart(),
- diag::warn_case_value_overflow);
+ // Check the unconverted value is within the range of possible values of
+ // the switch expression.
+ checkCaseValue(*this, Lo->getLocStart(), LoVal,
+ CondWidthBeforePromotion, CondIsSignedBeforePromotion);
+
+ // Convert the value to the same width/sign as the condition.
+ AdjustAPSInt(LoVal, CondWidth, CondIsSigned);
CS->setLHS(Lo);
@@ -847,9 +828,8 @@
llvm::APSInt ConstantCondValue;
bool HasConstantCond = false;
if (!HasDependentValue && !TheDefaultStmt) {
- HasConstantCond
- = CondExprBeforePromotion->EvaluateAsInt(ConstantCondValue, Context,
- Expr::SE_AllowSideEffects);
+ HasConstantCond = CondExpr->EvaluateAsInt(ConstantCondValue, Context,
+ Expr::SE_AllowSideEffects);
assert(!HasConstantCond ||
(ConstantCondValue.getBitWidth() == CondWidth &&
ConstantCondValue.isSigned() == CondIsSigned));
@@ -935,10 +915,13 @@
Hi = ImpCastExprToType(Hi, CondType, CK_IntegralCast).get();
}
+ // Check the unconverted value is within the range of possible values of
+ // the switch expression.
+ checkCaseValue(*this, Hi->getLocStart(), HiVal,
+ CondWidthBeforePromotion, CondIsSignedBeforePromotion);
+
// Convert the value to the same width/sign as the condition.
- ConvertIntegerToTypeWarnOnOverflow(HiVal, CondWidth, CondIsSigned,
- Hi->getLocStart(),
- diag::warn_case_value_overflow);
+ AdjustAPSInt(HiVal, CondWidth, CondIsSigned);
CR->setRHS(Hi);