Skip to content

Commit a7757db

Browse files
committed
fix <rdar://problem/24530312> Swift ++fix-it produces bad code in nested expressions
The AvailabilityWalker was creating a new AvailabilityWalker instance whenever it recursed through an assignexpr or memberrefexpr, which produced a new ExprStack. This caused the fixit mechanics for migrating ++/-- to think that the ++/-- was at the top level, when it wasn't.
1 parent 5b97f8e commit a7757db

File tree

2 files changed

+21
-13
lines changed

2 files changed

+21
-13
lines changed

lib/Sema/MiscDiagnostics.cpp

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "swift/Basic/StringExtras.h"
2424
#include "swift/Parse/Lexer.h"
2525
#include "llvm/ADT/MapVector.h"
26+
#include "llvm/Support/SaveAndRestore.h"
2627
using namespace swift;
2728

2829
//===----------------------------------------------------------------------===//
@@ -915,14 +916,12 @@ class AvailabilityWalker : public ASTWalker {
915916

916917
TypeChecker &TC;
917918
DeclContext *DC;
918-
const MemberAccessContext AccessContext;
919+
MemberAccessContext AccessContext = MemberAccessContext::Getter;
919920
SmallVector<const Expr *, 16> ExprStack;
920921

921922
public:
922923
AvailabilityWalker(
923-
TypeChecker &TC, DeclContext *DC,
924-
MemberAccessContext AccessContext = MemberAccessContext::Getter)
925-
: TC(TC), DC(DC), AccessContext(AccessContext) {}
924+
TypeChecker &TC, DeclContext *DC) : TC(TC), DC(DC) {}
926925

927926
virtual std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
928927
ExprStack.push_back(E);
@@ -976,7 +975,7 @@ class AvailabilityWalker : public ASTWalker {
976975
const AvailableAttr *Attr);
977976

978977
/// Walk an assignment expression, checking for availability.
979-
void walkAssignExpr(AssignExpr *E) const {
978+
void walkAssignExpr(AssignExpr *E) {
980979
// We take over recursive walking of assignment expressions in order to
981980
// walk the destination and source expressions in different member
982981
// access contexts.
@@ -990,20 +989,20 @@ class AvailabilityWalker : public ASTWalker {
990989
// encountered walking (pre-order) is the Dest is the destination of the
991990
// write. For the moment this is fine -- but future syntax might violate
992991
// this assumption.
993-
walkInContext(Dest, MemberAccessContext::Setter);
992+
walkInContext(E, Dest, MemberAccessContext::Setter);
994993

995994
// Check RHS in getter context
996995
Expr *Source = E->getSrc();
997996
if (!Source) {
998997
return;
999998
}
1000-
walkInContext(Source, MemberAccessContext::Getter);
999+
walkInContext(E, Source, MemberAccessContext::Getter);
10011000
}
10021001

10031002
/// Walk a member reference expression, checking for availability.
10041003
void walkMemberRef(MemberRefExpr *E) {
10051004
// Walk the base in a getter context.
1006-
walkInContext(E->getBase(), MemberAccessContext::Getter);
1005+
walkInContext(E, E->getBase(), MemberAccessContext::Getter);
10071006

10081007
ValueDecl *D = E->getMember().getDecl();
10091008
// Diagnose for the member declaration itself.
@@ -1021,12 +1020,15 @@ class AvailabilityWalker : public ASTWalker {
10211020

10221021
/// Walk an inout expression, checking for availability.
10231022
void walkInOutExpr(InOutExpr *E) {
1024-
walkInContext(E->getSubExpr(), MemberAccessContext::InOut);
1023+
walkInContext(E, E->getSubExpr(), MemberAccessContext::InOut);
10251024
}
10261025

10271026
/// Walk the given expression in the member access context.
1028-
void walkInContext(Expr *E, MemberAccessContext AccessContext) const {
1029-
E->walk(AvailabilityWalker(TC, DC, AccessContext));
1027+
void walkInContext(Expr *baseExpr, Expr *E,
1028+
MemberAccessContext AccessContext) {
1029+
llvm::SaveAndRestore<MemberAccessContext>
1030+
C(this->AccessContext, AccessContext);
1031+
E->walk(*this);
10301032
}
10311033

10321034
/// Emit diagnostics, if necessary, for accesses to storage where

test/expr/expressions.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -824,14 +824,20 @@ func swift22_deprecation_increment_decrement() {
824824

825825
++f // expected-warning {{'++' is deprecated: it will be removed in Swift 3}} {{3-5=}} {{6-6= += 1}}
826826
f-- // expected-warning {{'--' is deprecated: it will be removed in Swift 3}} {{4-6= -= 1}}
827-
_ = f-- // expected-warning {{'--' is deprecated: it will be removed in Swift 3}}
827+
_ = f-- // expected-warning {{'--' is deprecated: it will be removed in Swift 3}} {{none}}
828828

829829

830830
++si // expected-warning {{'++' is deprecated: it will be removed in Swift 3}} {{3-5=}} {{7-7= = si.successor()}}
831831
--si // expected-warning {{'--' is deprecated: it will be removed in Swift 3}} {{3-5=}} {{7-7= = si.predecessor()}}
832832
si++ // expected-warning {{'++' is deprecated: it will be removed in Swift 3}} {{5-7= = si.successor()}}
833833
si-- // expected-warning {{'--' is deprecated: it will be removed in Swift 3}} {{5-7= = si.predecessor()}}
834-
_ = --si // expected-warning {{'--' is deprecated: it will be removed in Swift 3}}
834+
_ = --si // expected-warning {{'--' is deprecated: it will be removed in Swift 3}} {{none}}
835+
836+
837+
// <rdar://problem/24530312> Swift ++fix-it produces bad code in nested expressions
838+
// This should not get a fixit hint.
839+
var j = 2
840+
i = ++j // expected-warning {{'++' is deprecated: it will be removed in Swift 3}} {{none}}
835841
}
836842

837843
// SR-628 mixing lvalues and rvalues in tuple expression

0 commit comments

Comments
 (0)