Skip to content

Commit 559b0b4

Browse files
nkcsgexinkcsgexi
authored
Merge pull request #3094 from nkcsgexi/master
[fixit] Provide a fixit when c-style loop is counting down.
2 parents 9b9513b + c994a3b commit 559b0b4

File tree

2 files changed

+90
-19
lines changed

2 files changed

+90
-19
lines changed

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 84 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2508,7 +2508,15 @@ void swift::performAbstractFuncDeclDiagnostics(TypeChecker &TC,
25082508

25092509
/// Diagnose C style for loops.
25102510

2511-
static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarDecl *loopVar) {
2511+
namespace {
2512+
enum class OperatorKind : char {
2513+
Greater,
2514+
Smaller,
2515+
NotEqual,
2516+
};
2517+
2518+
static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS,
2519+
VarDecl *loopVar, OperatorKind &OpKind) {
25122520
auto *Cond = FS->getCond().getPtrOrNull();
25132521
if (!Cond)
25142522
return nullptr;
@@ -2527,8 +2535,15 @@ static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarD
25272535

25282536
// Verify that the condition is a simple != or < comparison to the loop variable.
25292537
auto comparisonOpName = binaryFuncExpr->getDecl()->getNameStr();
2530-
if (comparisonOpName != "!=" && comparisonOpName != "<")
2538+
if (comparisonOpName == "!=")
2539+
OpKind = OperatorKind::NotEqual;
2540+
else if (comparisonOpName == "<")
2541+
OpKind = OperatorKind::Smaller;
2542+
else if (comparisonOpName == ">")
2543+
OpKind = OperatorKind::Greater;
2544+
else
25312545
return nullptr;
2546+
25322547
auto args = binaryExpr->getArg()->getElements();
25332548
auto loadExpr = dyn_cast<LoadExpr>(args[0]);
25342549
if (!loadExpr)
@@ -2541,7 +2556,9 @@ static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarD
25412556
return args[1];
25422557
}
25432558

2544-
static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS, VarDecl *loopVar) {
2559+
static bool unaryOperatorCheckForConvertingCStyleForLoop(const ForStmt *FS,
2560+
VarDecl *loopVar,
2561+
StringRef OpName) {
25452562
auto *Increment = FS->getIncrement().getPtrOrNull();
25462563
if (!Increment)
25472564
return false;
@@ -2552,19 +2569,33 @@ static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS, VarDecl
25522569
return false;
25532570
auto inoutExpr = dyn_cast<InOutExpr>(unaryExpr->getArg());
25542571
if (!inoutExpr)
2555-
return false;
2572+
return false;
25562573
auto incrementDeclRefExpr = dyn_cast<DeclRefExpr>(inoutExpr->getSubExpr());
25572574
if (!incrementDeclRefExpr)
25582575
return false;
25592576
auto unaryFuncExpr = dyn_cast<DeclRefExpr>(unaryExpr->getFn());
25602577
if (!unaryFuncExpr)
25612578
return false;
2562-
if (unaryFuncExpr->getDecl()->getNameStr() != "++")
2579+
if (unaryFuncExpr->getDecl()->getNameStr() != OpName)
25632580
return false;
2564-
return incrementDeclRefExpr->getDecl() == loopVar;
2581+
return incrementDeclRefExpr->getDecl() == loopVar;
25652582
}
25662583

2567-
static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, const ForStmt *FS, VarDecl *loopVar) {
2584+
2585+
static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS,
2586+
VarDecl *loopVar) {
2587+
return unaryOperatorCheckForConvertingCStyleForLoop(FS, loopVar, "++");
2588+
}
2589+
2590+
static bool unaryDecrementForConvertingCStyleForLoop(const ForStmt *FS,
2591+
VarDecl *loopVar) {
2592+
return unaryOperatorCheckForConvertingCStyleForLoop(FS, loopVar, "--");
2593+
}
2594+
2595+
static bool binaryOperatorCheckForConvertingCStyleForLoop(TypeChecker &TC,
2596+
const ForStmt *FS,
2597+
VarDecl *loopVar,
2598+
StringRef OpName) {
25682599
auto *Increment = FS->getIncrement().getPtrOrNull();
25692600
if (!Increment)
25702601
return false;
@@ -2574,7 +2605,7 @@ static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, con
25742605
auto binaryFuncExpr = dyn_cast<DeclRefExpr>(binaryExpr->getFn());
25752606
if (!binaryFuncExpr)
25762607
return false;
2577-
if (binaryFuncExpr->getDecl()->getNameStr() != "+=")
2608+
if (binaryFuncExpr->getDecl()->getNameStr() != OpName)
25782609
return false;
25792610
auto argTupleExpr = dyn_cast<TupleExpr>(binaryExpr->getArg());
25802611
if (!argTupleExpr)
@@ -2595,6 +2626,19 @@ static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, con
25952626
if (!declRefExpr)
25962627
return false;
25972628
return declRefExpr->getDecl() == loopVar;
2629+
2630+
}
2631+
2632+
static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC,
2633+
const ForStmt *FS,
2634+
VarDecl *loopVar) {
2635+
return binaryOperatorCheckForConvertingCStyleForLoop(TC, FS, loopVar, "+=");
2636+
}
2637+
2638+
static bool minusEqualOneDecrementForConvertingCStyleForLoop(TypeChecker &TC,
2639+
const ForStmt *FS,
2640+
VarDecl *loopVar) {
2641+
return binaryOperatorCheckForConvertingCStyleForLoop(TC, FS, loopVar, "-=");
25982642
}
25992643

26002644
static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
@@ -2616,13 +2660,18 @@ static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
26162660

26172661
VarDecl *loopVar = dyn_cast<VarDecl>(initializers[1]);
26182662
Expr *startValue = loopVarDecl->getInit(0);
2619-
Expr *endValue = endConditionValueForConvertingCStyleForLoop(FS, loopVar);
2663+
OperatorKind OpKind;
2664+
Expr *endValue = endConditionValueForConvertingCStyleForLoop(FS, loopVar, OpKind);
26202665
bool strideByOne = unaryIncrementForConvertingCStyleForLoop(FS, loopVar) ||
26212666
plusEqualOneIncrementForConvertingCStyleForLoop(TC, FS, loopVar);
2667+
bool strideBackByOne = unaryDecrementForConvertingCStyleForLoop(FS, loopVar) ||
2668+
minusEqualOneDecrementForConvertingCStyleForLoop(TC, FS, loopVar);
26222669

2623-
if (!loopVar || !startValue || !endValue || !strideByOne)
2670+
if (!loopVar || !startValue || !endValue || (!strideByOne && !strideBackByOne))
26242671
return;
2625-
2672+
2673+
assert(strideBackByOne != strideByOne && "cannot be both increment and decrement.");
2674+
26262675
// Verify that the loop variable is invariant inside the body.
26272676
VarDeclUsageChecker checker(TC, loopVar);
26282677
checker.suppressDiagnostics();
@@ -2639,15 +2688,31 @@ static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
26392688
SourceLoc endOfIncrementLoc =
26402689
Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
26412690
FS->getIncrement().getPtrOrNull()->getEndLoc());
2642-
2643-
diagnostic
2644-
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
2645-
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
2646-
.fixItReplaceChars(FS->getFirstSemicolonLoc(), endValue->getStartLoc(),
2647-
" ..< ")
2648-
.fixItRemoveChars(FS->getSecondSemicolonLoc(), endOfIncrementLoc);
2649-
}
26502691

2692+
if (strideByOne && OpKind != OperatorKind::Greater) {
2693+
diagnostic
2694+
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
2695+
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
2696+
.fixItReplaceChars(FS->getFirstSemicolonLoc(), endValue->getStartLoc(),
2697+
" ..< ")
2698+
.fixItRemoveChars(FS->getSecondSemicolonLoc(), endOfIncrementLoc);
2699+
return;
2700+
} else if (strideBackByOne && OpKind != OperatorKind::Smaller) {
2701+
SourceLoc startValueEnd = Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
2702+
startValue->getEndLoc());
2703+
2704+
StringRef endValueStr = CharSourceRange(TC.Context.SourceMgr, endValue->getStartLoc(),
2705+
Lexer::getLocForEndOfToken(TC.Context.SourceMgr, endValue->getEndLoc())).str();
2706+
2707+
diagnostic
2708+
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
2709+
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
2710+
.fixItInsert(startValue->getStartLoc(), (llvm::Twine("((") + endValueStr + " + 1)...").str())
2711+
.fixItInsert(startValueEnd, ").reversed()")
2712+
.fixItRemoveChars(FS->getFirstSemicolonLoc(), endOfIncrementLoc);
2713+
}
2714+
}
2715+
}// Anonymous namespace end.
26512716

26522717
// Perform MiscDiagnostics on Switch Statements.
26532718
static void checkSwitch(TypeChecker &TC, const SwitchStmt *stmt) {

‎test/Sema/diag_c_style_for.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ for var e = 3; e > 4; e++ { // expected-error {{C-style for statement has been r
2525
for var f = 3; f < 4; f-- { // expected-error {{C-style for statement has been removed in Swift 3}} {{none}}
2626
}
2727

28+
for var i = 6; i > 0; i-=1 { // expected-error {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{13-13=((0 + 1)...}} {{14-14=).reversed()}} {{14-27=}}
29+
}
30+
31+
for var i = 100; i != 0; i-=1 { // expected-error {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{13-13=((0 + 1)...}} {{16-16=).reversed()}} {{16-30=}}
32+
}
33+
2834
let start = Int8(4)
2935
let count = Int8(10)
3036
var other = Int8(2)

0 commit comments

Comments
 (0)