Skip to content

Commit bafcb2b

Browse files
swift-ciswift-ci
authored
Merge pull request #11033 from rjmccall/fix-c-style-for-loops
2 parents ff07df8 + dcc65ca commit bafcb2b

File tree

2 files changed

+83
-37
lines changed

2 files changed

+83
-37
lines changed

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2616,10 +2616,38 @@ void swift::performAbstractFuncDeclDiagnostics(TypeChecker &TC,
26162616
namespace {
26172617
enum class OperatorKind : char {
26182618
Greater,
2619+
GreaterEqual,
26192620
Smaller,
2621+
SmallerEqual,
26202622
NotEqual,
26212623
};
26222624

2625+
static ValueDecl *getReferencedDecl(Expr *expr) {
2626+
if (auto inoutExpr = dyn_cast<InOutExpr>(expr))
2627+
expr = inoutExpr->getSubExpr();
2628+
if (auto declRefExpr = dyn_cast<DeclRefExpr>(expr))
2629+
return declRefExpr->getDecl();
2630+
return nullptr;
2631+
}
2632+
2633+
static DeclBaseName getOperatorName(ApplyExpr *expr) {
2634+
auto fnExpr = expr->getFn();
2635+
if (auto dotSyntaxExpr = dyn_cast<DotSyntaxCallExpr>(fnExpr)) {
2636+
if (!isa<TypeExpr>(dotSyntaxExpr->getBase()))
2637+
return DeclBaseName();
2638+
fnExpr = dotSyntaxExpr->getFn();
2639+
}
2640+
if (auto fnRefExpr = dyn_cast<DeclRefExpr>(fnExpr)) {
2641+
return fnRefExpr->getDecl()->getBaseName();
2642+
}
2643+
if (auto fnRefExpr = dyn_cast<OverloadedDeclRefExpr>(fnExpr)) {
2644+
auto decls = fnRefExpr->getDecls();
2645+
if (decls.empty()) return DeclBaseName();
2646+
return decls.front()->getBaseName();
2647+
}
2648+
return DeclBaseName();
2649+
}
2650+
26232651
static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS,
26242652
VarDecl *loopVar, OperatorKind &OpKind) {
26252653
auto *Cond = FS->getCond().getPtrOrNull();
@@ -2634,36 +2662,34 @@ static Expr *endConditionValueForConvertingCStyleForLoop(const ForStmt *FS,
26342662
auto binaryExpr = dyn_cast<BinaryExpr>(dotSyntaxExpr->getBase());
26352663
if (!binaryExpr)
26362664
return nullptr;
2637-
auto binaryFuncExpr = dyn_cast<DeclRefExpr>(binaryExpr->getFn());
2638-
if (!binaryFuncExpr)
2639-
return nullptr;
26402665

26412666
// Verify that the condition is a simple != or < comparison to the loop variable.
2642-
auto comparisonOpName = binaryFuncExpr->getDecl()->getBaseName();
2667+
auto comparisonOpName = getOperatorName(binaryExpr);
26432668
if (comparisonOpName == "!=")
26442669
OpKind = OperatorKind::NotEqual;
26452670
else if (comparisonOpName == "<")
26462671
OpKind = OperatorKind::Smaller;
2672+
else if (comparisonOpName == "<=")
2673+
OpKind = OperatorKind::SmallerEqual;
26472674
else if (comparisonOpName == ">")
26482675
OpKind = OperatorKind::Greater;
2676+
else if (comparisonOpName == ">=")
2677+
OpKind = OperatorKind::GreaterEqual;
26492678
else
26502679
return nullptr;
26512680

26522681
auto args = binaryExpr->getArg()->getElements();
26532682
auto loadExpr = dyn_cast<LoadExpr>(args[0]);
26542683
if (!loadExpr)
26552684
return nullptr;
2656-
auto declRefExpr = dyn_cast<DeclRefExpr>(loadExpr->getSubExpr());
2657-
if (!declRefExpr)
2658-
return nullptr;
2659-
if (declRefExpr->getDecl() != loopVar)
2685+
if (getReferencedDecl(loadExpr->getSubExpr()) != loopVar)
26602686
return nullptr;
26612687
return args[1];
26622688
}
26632689

26642690
static bool unaryOperatorCheckForConvertingCStyleForLoop(const ForStmt *FS,
26652691
VarDecl *loopVar,
2666-
StringRef OpName) {
2692+
StringRef opName) {
26672693
auto *Increment = FS->getIncrement().getPtrOrNull();
26682694
if (!Increment)
26692695
return false;
@@ -2672,21 +2698,11 @@ static bool unaryOperatorCheckForConvertingCStyleForLoop(const ForStmt *FS,
26722698
unaryExpr = dyn_cast<PostfixUnaryExpr>(Increment);
26732699
if (!unaryExpr)
26742700
return false;
2675-
auto inoutExpr = dyn_cast<InOutExpr>(unaryExpr->getArg());
2676-
if (!inoutExpr)
2677-
return false;
2678-
auto incrementDeclRefExpr = dyn_cast<DeclRefExpr>(inoutExpr->getSubExpr());
2679-
if (!incrementDeclRefExpr)
2680-
return false;
2681-
auto unaryFuncExpr = dyn_cast<DeclRefExpr>(unaryExpr->getFn());
2682-
if (!unaryFuncExpr)
2701+
if (getOperatorName(unaryExpr) != opName)
26832702
return false;
2684-
if (unaryFuncExpr->getDecl()->getBaseName() != OpName)
2685-
return false;
2686-
return incrementDeclRefExpr->getDecl() == loopVar;
2703+
return getReferencedDecl(unaryExpr->getArg()) == loopVar;
26872704
}
26882705

2689-
26902706
static bool unaryIncrementForConvertingCStyleForLoop(const ForStmt *FS,
26912707
VarDecl *loopVar) {
26922708
return unaryOperatorCheckForConvertingCStyleForLoop(FS, loopVar, "++");
@@ -2707,10 +2723,7 @@ static bool binaryOperatorCheckForConvertingCStyleForLoop(TypeChecker &TC,
27072723
ApplyExpr *binaryExpr = dyn_cast<BinaryExpr>(Increment);
27082724
if (!binaryExpr)
27092725
return false;
2710-
auto binaryFuncExpr = dyn_cast<DeclRefExpr>(binaryExpr->getFn());
2711-
if (!binaryFuncExpr)
2712-
return false;
2713-
if (binaryFuncExpr->getDecl()->getBaseName() != OpName)
2726+
if (getOperatorName(binaryExpr) != OpName)
27142727
return false;
27152728
auto argTupleExpr = dyn_cast<TupleExpr>(binaryExpr->getArg());
27162729
if (!argTupleExpr)
@@ -2724,14 +2737,7 @@ static bool binaryOperatorCheckForConvertingCStyleForLoop(TypeChecker &TC,
27242737
if (range.str() != "1")
27252738
return false;
27262739

2727-
auto inoutExpr = dyn_cast<InOutExpr>(argTupleExpr->getElement(0));
2728-
if (!inoutExpr)
2729-
return false;
2730-
auto declRefExpr = dyn_cast<DeclRefExpr>(inoutExpr->getSubExpr());
2731-
if (!declRefExpr)
2732-
return false;
2733-
return declRefExpr->getDecl() == loopVar;
2734-
2740+
return getReferencedDecl(argTupleExpr->getElement(0)) == loopVar;
27352741
}
27362742

27372743
static bool plusEqualOneIncrementForConvertingCStyleForLoop(TypeChecker &TC,
@@ -2794,15 +2800,19 @@ static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
27942800
Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
27952801
FS->getIncrement().getPtrOrNull()->getEndLoc());
27962802

2797-
if (strideByOne && OpKind != OperatorKind::Greater) {
2803+
if (strideByOne &&
2804+
(OpKind == OperatorKind::Smaller ||
2805+
OpKind == OperatorKind::SmallerEqual)) {
27982806
diagnostic
27992807
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
28002808
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
28012809
.fixItReplaceChars(FS->getFirstSemicolonLoc(), endValue->getStartLoc(),
2802-
" ..< ")
2810+
OpKind == OperatorKind::Smaller ? " ..< " : " ... ")
28032811
.fixItRemoveChars(FS->getSecondSemicolonLoc(), endOfIncrementLoc);
28042812
return;
2805-
} else if (strideBackByOne && OpKind != OperatorKind::Smaller) {
2813+
} else if (strideBackByOne &&
2814+
(OpKind == OperatorKind::Greater ||
2815+
OpKind == OperatorKind::GreaterEqual)) {
28062816
SourceLoc startValueEnd = Lexer::getLocForEndOfToken(TC.Context.SourceMgr,
28072817
startValue->getEndLoc());
28082818

@@ -2812,7 +2822,10 @@ static void checkCStyleForLoop(TypeChecker &TC, const ForStmt *FS) {
28122822
diagnostic
28132823
.fixItRemoveChars(loopVarDecl->getLoc(), loopVar->getLoc())
28142824
.fixItReplaceChars(loopPatternEnd, startValue->getStartLoc(), " in ")
2815-
.fixItInsert(startValue->getStartLoc(), (llvm::Twine("((") + endValueStr + " + 1)...").str())
2825+
.fixItInsert(startValue->getStartLoc(),
2826+
OpKind == OperatorKind::Greater
2827+
? (llvm::Twine("((") + endValueStr + " + 1)...").str()
2828+
: (llvm::Twine("(") + endValueStr + "...").str())
28162829
.fixItInsert(startValueEnd, ").reversed()")
28172830
.fixItRemoveChars(FS->getFirstSemicolonLoc(), endOfIncrementLoc);
28182831
}

‎test/stmt/for_fixit.swift

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: %target-typecheck-verify-swift -typecheck %s
2+
3+
for var i = 0; i < 10; i++ {}
4+
// expected-error @-1 {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{14-20= ..< }} {{22-27=}}
5+
// expected-error @-2 {{unary operator '++' cannot be applied}}
6+
// expected-note @-3 {{overloads for '++' exist}}
7+
8+
for var i = 0; i < 10; i += 1 {}
9+
// expected-error @-1 {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{14-20= ..< }} {{22-30=}}
10+
11+
for var i = 0; i <= 10; i++ {}
12+
// expected-error @-1 {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{14-21= ... }} {{23-28=}}
13+
// expected-error @-2 {{unary operator '++' cannot be applied}}
14+
// expected-note @-3 {{overloads for '++' exist}}
15+
16+
for var i = 0; i <= 10; i += 1 {}
17+
// expected-error @-1 {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{14-21= ... }} {{23-31=}}
18+
19+
for var i = 10; i > 0; i-- {}
20+
// expected-error @-1 {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{13-13=((0 + 1)...}} {{15-15=).reversed()}} {{15-27=}}
21+
// expected-error @-2 {{unary operator '--' cannot be applied}}
22+
// expected-note @-3 {{overloads for '--' exist}}
23+
24+
for var i = 10; i > 0; i -= 1 {}
25+
// expected-error @-1 {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{13-13=((0 + 1)...}} {{15-15=).reversed()}} {{15-30=}}
26+
27+
for var i = 10; i >= 0; i-- {}
28+
// expected-error @-1 {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{13-13=(0...}} {{15-15=).reversed()}} {{15-28=}}
29+
// expected-error @-2 {{unary operator '--' cannot be applied}}
30+
// expected-note @-3 {{overloads for '--' exist}}
31+
32+
for var i = 10; i >= 0; i -= 1 {}
33+
// expected-error @-1 {{C-style for statement has been removed in Swift 3}} {{5-9=}} {{10-13= in }} {{13-13=(0...}} {{15-15=).reversed()}} {{15-31=}}

0 commit comments

Comments
 (0)