Skip to content

Commit 7beb451

Browse files
committed
[fixit] Provide a fixit when c-style loop is counting down.
1 parent 4190fe5 commit 7beb451

File tree

2 files changed

+85
-18
lines changed

2 files changed

+85
-18
lines changed

lib/Sema/MiscDiagnostics.cpp

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

25092509
/// Diagnose C style for loops.
25102510

2511-
static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarDecl *loopVar) {
2511+
enum OperatorKind {
2512+
Greater,
2513+
Smaller,
2514+
NEqual,
2515+
};
2516+
2517+
static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS,
2518+
VarDecl *loopVar, OperatorKind &OpKind) {
25122519
auto *Cond = FS->getCond().getPtrOrNull();
25132520
if (!Cond)
25142521
return nullptr;
@@ -2527,8 +2534,15 @@ static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarD
25272534

25282535
// Verify that the condition is a simple != or < comparison to the loop variable.
25292536
auto comparisonOpName = binaryFuncExpr->getDecl()->getNameStr();
2530-
if (comparisonOpName != "!=" && comparisonOpName != "<")
2537+
if (comparisonOpName == "!=")
2538+
OpKind = OperatorKind::NEqual;
2539+
else if (comparisonOpName == "<")
2540+
OpKind = OperatorKind::Smaller;
2541+
else if (comparisonOpName == ">")
2542+
OpKind = OperatorKind::Greater;
2543+
else
25312544
return nullptr;
2545+
25322546
auto args = binaryExpr->getArg()->getElements();
25332547
auto loadExpr = dyn_cast<LoadExpr>(args[0]);
25342548
if (!loadExpr)
@@ -2541,7 +2555,9 @@ static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarD
25412555
return args[1];
25422556
}
25432557

2544-
static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS, VarDecl *loopVar) {
2558+
static bool unaryOperatorCheckForConvertingCStyleForLoop(const ForStmt *FS,
2559+
VarDecl *loopVar,
2560+
StringRef OpContent) {
25452561
auto *Increment = FS->getIncrement().getPtrOrNull();
25462562
if (!Increment)
25472563
return false;
@@ -2552,19 +2568,33 @@ static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS, VarDecl
25522568
return false;
25532569
auto inoutExpr = dyn_cast<InOutExpr>(unaryExpr->getArg());
25542570
if (!inoutExpr)
2555-
return false;
2571+
return false;
25562572
auto incrementDeclRefExpr = dyn_cast<DeclRefExpr>(inoutExpr->getSubExpr());
25572573
if (!incrementDeclRefExpr)
25582574
return false;
25592575
auto unaryFuncExpr = dyn_cast<DeclRefExpr>(unaryExpr->getFn());
25602576
if (!unaryFuncExpr)
25612577
return false;
2562-
if (unaryFuncExpr->getDecl()->getNameStr() != "++")
2578+
if (unaryFuncExpr->getDecl()->getNameStr() != OpContent)
25632579
return false;
2564-
return incrementDeclRefExpr->getDecl() == loopVar;
2580+
return incrementDeclRefExpr->getDecl() == loopVar;
2581+
}
2582+
2583+
2584+
static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS,
2585+
VarDecl *loopVar) {
2586+
return unaryOperatorCheckForConvertingCStyleForLoop(FS, loopVar, "++");
2587+
}
2588+
2589+
static bool unaryDecrementForConvertingCStyleForLoop(const ForStmt *FS,
2590+
VarDecl *loopVar) {
2591+
return unaryOperatorCheckForConvertingCStyleForLoop(FS, loopVar, "--");
25652592
}
25662593

2567-
static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, const ForStmt *FS, VarDecl *loopVar) {
2594+
static bool binaryOperatorCheckForConvertingCStyleForLoop(TypeChecker &TC,
2595+
const ForStmt *FS,
2596+
VarDecl *loopVar,
2597+
StringRef OpContent) {
25682598
auto *Increment = FS->getIncrement().getPtrOrNull();
25692599
if (!Increment)
25702600
return false;
@@ -2574,7 +2604,7 @@ static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, con
25742604
auto binaryFuncExpr = dyn_cast<DeclRefExpr>(binaryExpr->getFn());
25752605
if (!binaryFuncExpr)
25762606
return false;
2577-
if (binaryFuncExpr->getDecl()->getNameStr() != "+=")
2607+
if (binaryFuncExpr->getDecl()->getNameStr() != OpContent)
25782608
return false;
25792609
auto argTupleExpr = dyn_cast<TupleExpr>(binaryExpr->getArg());
25802610
if (!argTupleExpr)
@@ -2595,6 +2625,19 @@ static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, con
25952625
if (!declRefExpr)
25962626
return false;
25972627
return declRefExpr->getDecl() == loopVar;
2628+
2629+
}
2630+
2631+
static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC,
2632+
const ForStmt *FS,
2633+
VarDecl *loopVar) {
2634+
return binaryOperatorCheckForConvertingCStyleForLoop(TC, FS, loopVar, "+=");
2635+
}
2636+
2637+
static bool minusEqualOneDecrementForConvertingCStyleForLoop(TypeChecker &TC,
2638+
const ForStmt *FS,
2639+
VarDecl *loopVar) {
2640+
return binaryOperatorCheckForConvertingCStyleForLoop(TC, FS, loopVar, "-=");
25982641
}
25992642

26002643
static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
@@ -2616,13 +2659,18 @@ static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
26162659

26172660
VarDecl *loopVar = dyn_cast<VarDecl>(initializers[1]);
26182661
Expr *startValue = loopVarDecl->getInit(0);
2619-
Expr *endValue = endConditionValueForConvertingCStyleForLoop(FS, loopVar);
2662+
OperatorKind OpKind;
2663+
Expr *endValue = endConditionValueForConvertingCStyleForLoop(FS, loopVar, OpKind);
26202664
bool strideByOne = unaryIncrementForConvertingCStyleForLoop(FS, loopVar) ||
26212665
plusEqualOneIncrementForConvertingCStyleForLoop(TC, FS, loopVar);
2666+
bool strideBackByOne = unaryDecrementForConvertingCStyleForLoop(FS, loopVar) ||
2667+
minusEqualOneDecrementForConvertingCStyleForLoop(TC, FS, loopVar);
26222668

2623-
if (!loopVar || !startValue || !endValue || !strideByOne)
2669+
if (!loopVar || !startValue || !endValue || (!strideByOne && !strideBackByOne))
26242670
return;
2625-
2671+
2672+
assert(strideBackByOne != strideByOne && "cannot be both increment and decrement.");
2673+
26262674
// Verify that the loop variable is invariant inside the body.
26272675
VarDeclUsageChecker checker(TC, loopVar);
26282676
checker.suppressDiagnostics();
@@ -2639,13 +2687,29 @@ static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
26392687
SourceLoc endOfIncrementLoc =
26402688
Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
26412689
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);
2690+
2691+
if (strideByOne && OpKind != OperatorKind::Greater) {
2692+
diagnostic
2693+
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
2694+
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
2695+
.fixItReplaceChars(FS->getFirstSemicolonLoc(), endValue->getStartLoc(),
2696+
" ..< ")
2697+
.fixItRemoveChars(FS->getSecondSemicolonLoc(), endOfIncrementLoc);
2698+
return;
2699+
} else if (strideBackByOne && OpKind != OperatorKind::Smaller) {
2700+
SourceLoc startValueEnd = Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
2701+
startValue->getEndLoc());
2702+
2703+
StringRef endValueStr = CharSourceRange(TC.Context.SourceMgr, endValue->getStartLoc(),
2704+
Lexer::getLocForEndOfToken(TC.Context.SourceMgr, endValue->getEndLoc())).str();
2705+
2706+
diagnostic
2707+
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
2708+
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
2709+
.fixItInsert(startValue->getStartLoc(), (llvm::Twine("((") + endValueStr + " + 1)...").str())
2710+
.fixItInsert(startValueEnd, ").reversed()")
2711+
.fixItRemoveChars(FS->getFirstSemicolonLoc(), endOfIncrementLoc);
2712+
}
26492713
}
26502714

26512715

test/Sema/diag_c_style_for.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ 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+
2831
let start = Int8(4)
2932
let count = Int8(10)
3033
var other = Int8(2)

0 commit comments

Comments
 (0)