Skip to content

[fixit] Provide a fixit when c-style loop is counting down. #3094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 21, 2016
Merged

[fixit] Provide a fixit when c-style loop is counting down. #3094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7beb451
[fixit] Provide a fixit when c-style loop is counting down.
nkcsgexi Jun 21, 2016
286b18f
Address Jordan's code review comments.
nkcsgexi Jun 21, 2016
c994a3b
Specify the underlying type of enum class.
nkcsgexi Jun 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 84 additions & 19 deletions lib/Sema/MiscDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2508,7 +2508,15 @@ void swift::performAbstractFuncDeclDiagnostics(TypeChecker &TC,

/// Diagnose C style for loops.

static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarDecl *loopVar) {
namespace {
enum class OperatorKind : char {
Greater,
Smaller,
NotEqual,
};

static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS,
VarDecl *loopVar, OperatorKind &OpKind) {
auto *Cond = FS->getCond().getPtrOrNull();
if (!Cond)
return nullptr;
Expand All @@ -2527,8 +2535,15 @@ static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarD

// Verify that the condition is a simple != or < comparison to the loop variable.
auto comparisonOpName = binaryFuncExpr->getDecl()->getNameStr();
if (comparisonOpName != "!=" && comparisonOpName != "<")
if (comparisonOpName == "!=")
OpKind = OperatorKind::NotEqual;
else if (comparisonOpName == "<")
OpKind = OperatorKind::Smaller;
else if (comparisonOpName == ">")
OpKind = OperatorKind::Greater;
else
return nullptr;

auto args = binaryExpr->getArg()->getElements();
auto loadExpr = dyn_cast<LoadExpr>(args[0]);
if (!loadExpr)
Expand All @@ -2541,7 +2556,9 @@ static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS, VarD
return args[1];
}

static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS, VarDecl *loopVar) {
static bool unaryOperatorCheckForConvertingCStyleForLoop(const ForStmt *FS,
VarDecl *loopVar,
StringRef OpName) {
auto *Increment = FS->getIncrement().getPtrOrNull();
if (!Increment)
return false;
Expand All @@ -2552,19 +2569,33 @@ static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS, VarDecl
return false;
auto inoutExpr = dyn_cast<InOutExpr>(unaryExpr->getArg());
if (!inoutExpr)
return false;
return false;
auto incrementDeclRefExpr = dyn_cast<DeclRefExpr>(inoutExpr->getSubExpr());
if (!incrementDeclRefExpr)
return false;
auto unaryFuncExpr = dyn_cast<DeclRefExpr>(unaryExpr->getFn());
if (!unaryFuncExpr)
return false;
if (unaryFuncExpr->getDecl()->getNameStr() != "++")
if (unaryFuncExpr->getDecl()->getNameStr() != OpName)
return false;
return incrementDeclRefExpr->getDecl() == loopVar;
return incrementDeclRefExpr->getDecl() == loopVar;
}

static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, const ForStmt *FS, VarDecl *loopVar) {

static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS,
VarDecl *loopVar) {
return unaryOperatorCheckForConvertingCStyleForLoop(FS, loopVar, "++");
}

static bool unaryDecrementForConvertingCStyleForLoop(const ForStmt *FS,
VarDecl *loopVar) {
return unaryOperatorCheckForConvertingCStyleForLoop(FS, loopVar, "--");
}

static bool binaryOperatorCheckForConvertingCStyleForLoop(TypeChecker &TC,
const ForStmt *FS,
VarDecl *loopVar,
StringRef OpName) {
auto *Increment = FS->getIncrement().getPtrOrNull();
if (!Increment)
return false;
Expand All @@ -2574,7 +2605,7 @@ static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, con
auto binaryFuncExpr = dyn_cast<DeclRefExpr>(binaryExpr->getFn());
if (!binaryFuncExpr)
return false;
if (binaryFuncExpr->getDecl()->getNameStr() != "+=")
if (binaryFuncExpr->getDecl()->getNameStr() != OpName)
return false;
auto argTupleExpr = dyn_cast<TupleExpr>(binaryExpr->getArg());
if (!argTupleExpr)
Expand All @@ -2595,6 +2626,19 @@ static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC, con
if (!declRefExpr)
return false;
return declRefExpr->getDecl() == loopVar;

}

static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC,
const ForStmt *FS,
VarDecl *loopVar) {
return binaryOperatorCheckForConvertingCStyleForLoop(TC, FS, loopVar, "+=");
}

static bool minusEqualOneDecrementForConvertingCStyleForLoop(TypeChecker &TC,
const ForStmt *FS,
VarDecl *loopVar) {
return binaryOperatorCheckForConvertingCStyleForLoop(TC, FS, loopVar, "-=");
}

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

VarDecl *loopVar = dyn_cast<VarDecl>(initializers[1]);
Expr *startValue = loopVarDecl->getInit(0);
Expr *endValue = endConditionValueForConvertingCStyleForLoop(FS, loopVar);
OperatorKind OpKind;
Expr *endValue = endConditionValueForConvertingCStyleForLoop(FS, loopVar, OpKind);
bool strideByOne = unaryIncrementForConvertingCStyleForLoop(FS, loopVar) ||
plusEqualOneIncrementForConvertingCStyleForLoop(TC, FS, loopVar);
bool strideBackByOne = unaryDecrementForConvertingCStyleForLoop(FS, loopVar) ||
minusEqualOneDecrementForConvertingCStyleForLoop(TC, FS, loopVar);

if (!loopVar || !startValue || !endValue || !strideByOne)
if (!loopVar || !startValue || !endValue || (!strideByOne && !strideBackByOne))
return;


assert(strideBackByOne != strideByOne && "cannot be both increment and decrement.");

// Verify that the loop variable is invariant inside the body.
VarDeclUsageChecker checker(TC, loopVar);
checker.suppressDiagnostics();
Expand All @@ -2639,15 +2688,31 @@ static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
SourceLoc endOfIncrementLoc =
Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
FS->getIncrement().getPtrOrNull()->getEndLoc());

diagnostic
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
.fixItReplaceChars(FS->getFirstSemicolonLoc(), endValue->getStartLoc(),
" ..< ")
.fixItRemoveChars(FS->getSecondSemicolonLoc(), endOfIncrementLoc);
}

if (strideByOne && OpKind != OperatorKind::Greater) {
diagnostic
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
.fixItReplaceChars(FS->getFirstSemicolonLoc(), endValue->getStartLoc(),
" ..< ")
.fixItRemoveChars(FS->getSecondSemicolonLoc(), endOfIncrementLoc);
return;
} else if (strideBackByOne && OpKind != OperatorKind::Smaller) {
SourceLoc startValueEnd = Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
startValue->getEndLoc());

StringRef endValueStr = CharSourceRange(TC.Context.SourceMgr, endValue->getStartLoc(),
Lexer::getLocForEndOfToken(TC.Context.SourceMgr, endValue->getEndLoc())).str();

diagnostic
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
.fixItInsert(startValue->getStartLoc(), (llvm::Twine("((") + endValueStr + " + 1)...").str())
.fixItInsert(startValueEnd, ").reversed()")
.fixItRemoveChars(FS->getFirstSemicolonLoc(), endOfIncrementLoc);
}
}
}// Anonymous namespace end.

// Perform MiscDiagnostics on Switch Statements.
static void checkSwitch(TypeChecker &TC, const SwitchStmt *stmt) {
Expand Down
6 changes: 6 additions & 0 deletions test/Sema/diag_c_style_for.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ for var e = 3; e > 4; e++ { // expected-error {{C-style for statement has been r
for var f = 3; f < 4; f-- { // expected-error {{C-style for statement has been removed in Swift 3}} {{none}}
}

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=}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a reverse test that uses !=, since you're testing for it!

}

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=}}
}

let start = Int8(4)
let count = Int8(10)
var other = Int8(2)
Expand Down