r214678 - PR11778: Fix the rejects-valid half of this bug. We still produce the same
Richard Smith
richard-llvm at metafoo.co.uk
Sun Aug 3 17:40:49 PDT 2014
Author: rsmith
Date: Sun Aug 3 19:40:48 2014
New Revision: 214678
URL: http://llvm.org/viewvc/llvm-project?rev=214678&view=rev
Log:
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.
Modified:
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/Sema/switch.c
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=214678&r1=214677&r2=214678&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Sun Aug 3 19:40:48 2014
@@ -481,47 +481,6 @@ Sema::ActOnIfStmt(SourceLocation IfLoc,
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 @@ Sema::ActOnStartOfSwitchStmt(SourceLocat
}
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 @@ Sema::ActOnFinishSwitchStmt(SourceLocati
}
}
- // 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 @@ Sema::ActOnFinishSwitchStmt(SourceLocati
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 @@ Sema::ActOnFinishSwitchStmt(SourceLocati
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 @@ Sema::ActOnFinishSwitchStmt(SourceLocati
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);
Modified: cfe/trunk/test/Sema/switch.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch.c?rev=214678&r1=214677&r2=214678&view=diff
==============================================================================
--- cfe/trunk/test/Sema/switch.c (original)
+++ cfe/trunk/test/Sema/switch.c Sun Aug 3 19:40:48 2014
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wswitch-enum -Wcovered-switch-default %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wswitch-enum -Wcovered-switch-default -triple x86_64-linux-gnu %s
void f (int z) {
while (z) {
default: z--; // expected-error {{statement not in switch}}
@@ -375,3 +375,13 @@ void switch_on_ExtendedEnum1(enum Extend
}
}
+void PR11778(char c, int n, long long ll) {
+ // Do not reject this; we don't have duplicate case values because we
+ // check for duplicates in the promoted type.
+ switch (c) case 1: case 257: ; // expected-warning {{overflow}}
+
+ switch (n) case 0x100000001LL: case 1: ; // expected-warning {{overflow}} expected-error {{duplicate}} expected-note {{previous}}
+ switch ((int)ll) case 0x100000001LL: case 1: ; // expected-warning {{overflow}} expected-error {{duplicate}} expected-note {{previous}}
+ switch ((long long)n) case 0x100000001LL: case 1: ;
+ switch (ll) case 0x100000001LL: case 1: ;
+}
More information about the cfe-commits
mailing list