maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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
This commit is contained in:
Richard Smith 2014-08-04 00:40:48 +00:00
parent 454b85606e
commit 077d083b4d
2 changed files with 57 additions and 64 deletions
clang
lib/Sema
SemaStmt.cpp
test/Sema
switch.c

109
clang/lib/Sema/SemaStmt.cpp
View File

@ -481,47 +481,6 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
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(SourceLocation SwitchLoc, Expr *Cond,
}
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(SourceLocation SwitchLoc, Stmt *Switch,
}
}
// 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(SourceLocation SwitchLoc, Stmt *Switch,
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(SourceLocation SwitchLoc, Stmt *Switch,
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(SourceLocation SwitchLoc, Stmt *Switch,
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);

12
clang/test/Sema/switch.c
View File

@ -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 ExtendedEnum1 e) {
}
}
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: ;
}