Skip to content

Commit

Permalink
Warn when using 'var' bindings in function parameters
Browse files Browse the repository at this point in the history
These will no longer be allowed in a future Swift release.

rdar://problem/23172698
  • Loading branch information
bitjammer committed Nov 4, 2015
1 parent 9f6994b commit 93b6962
Show file tree
Hide file tree
Showing 43 changed files with 185 additions and 99 deletions.
3 changes: 3 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4151,6 +4151,9 @@ class VarDecl : public AbstractStorageDecl {
/// that it can be rewritten to a 'var'. This is used in situations where the
/// compiler detects obvious attempts to mutate a constant.
void emitLetToVarNoteIfSimple(DeclContext *UseDC) const;

/// Returns true if the name is the self identifier and is implicit.
bool isImplicitSelf() const;

// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) {
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ ERROR(untyped_pattern_ellipsis,pattern_parsing,none,
ERROR(non_func_decl_pattern_init,pattern_parsing,none,
"default argument is only permitted for a non-curried function parameter",())
WARNING(var_not_allowed_in_pattern,pattern_parsing, none,
"Use of 'var' binding here is deprecated and will be removed in a future version of Swift", ())
"Use of '%select{var|let}0' binding here is deprecated and will be removed in a future version of Swift", (unsigned))
ERROR(var_pattern_in_var,pattern_parsing,none,
"'%select{var|let}0' cannot appear nested inside another 'var' or "
"'let' pattern", (unsigned))
Expand Down
8 changes: 7 additions & 1 deletion lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3154,6 +3154,9 @@ Pattern *VarDecl::getParentPattern() const {
return nullptr;
}

bool VarDecl::isImplicitSelf() const {
return isImplicit() && getName() == getASTContext().Id_self;
}

/// Return true if this stored property needs to be accessed with getters and
/// setters for Objective-C.
Expand Down Expand Up @@ -3232,9 +3235,12 @@ void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const {
// If it isn't a 'let', don't touch it.
if (!isLet()) return;

// Don't suggest mutability for explicit function parameters
if (isa<ParamDecl>(this) && !isImplicitSelf()) return;

// If this is the 'self' argument of a non-mutating method in a value type,
// suggest adding 'mutating' to the method.
if (getName().str() == "self" && UseDC) {
if (isImplicitSelf() && UseDC) {
// If the problematic decl is 'self', then we might be trying to mutate
// a property in a non-mutating method.
auto FD = dyn_cast<FuncDecl>(UseDC->getInnermostMethodContext());
Expand Down
8 changes: 6 additions & 2 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,13 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
param.LetVarInOutLoc = consumeToken();
param.SpecifierKind = ParsedParameter::InOut;
} else if (Tok.is(tok::kw_let)) {
diagnose(Tok.getLoc(), diag::var_not_allowed_in_pattern,
Tok.is(tok::kw_let)).fixItRemove(Tok.getLoc());
param.LetVarInOutLoc = consumeToken();
param.SpecifierKind = ParsedParameter::Let;
} else if (Tok.is(tok::kw_var)) {
diagnose(Tok.getLoc(), diag::var_not_allowed_in_pattern,
Tok.is(tok::kw_let)).fixItRemove(Tok.getLoc());
param.LetVarInOutLoc = consumeToken();
param.SpecifierKind = ParsedParameter::Var;
}
Expand Down Expand Up @@ -851,7 +855,7 @@ ParserResult<Pattern> Parser::parsePattern() {
} else {
// In an always immutable context, `var` is not allowed.
if (alwaysImmutable)
diagnose(varLoc, diag::var_not_allowed_in_pattern)
diagnose(varLoc, diag::var_not_allowed_in_pattern, isLetKeyword)
.fixItRemove(varLoc);
}

Expand Down Expand Up @@ -1063,7 +1067,7 @@ ParserResult<Pattern> Parser::parseMatchingPatternAsLetOrVar(bool isLet,
diagnose(varLoc, diag::let_pattern_in_immutable_context);

if (!isLet && InVarOrLetPattern == IVOLP_AlwaysImmutable)
diagnose(varLoc, diag::var_not_allowed_in_pattern)
diagnose(varLoc, diag::var_not_allowed_in_pattern, isLet)
.fixItReplace(varLoc, "let");

// In our recursive parse, remember that we're in a var/let pattern.
Expand Down
12 changes: 8 additions & 4 deletions test/1_stdlib/CollectionDiagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,13 @@ func sortResultIgnored<
S.Generator.Element : Comparable,
MC.Generator.Element : Comparable
>(
var sequence: S, // expected-warning {{parameter 'sequence' was never mutated; consider changing to 'let' constant}} {{3-7=}}
var mutableCollection: MC, // expected-warning {{parameter 'mutableCollection' was never mutated; consider changing to 'let' constant}} {{3-7=}}
var array: [Int] // expected-warning {{parameter 'array' was never mutated; consider changing to 'let' constant}} {{3-7=}}
sequence: S,
mutableCollection: MC,
array: [Int]
) {
var sequence = sequence // expected-warning {{was never mutated; consider changing to 'let' constant}}
var mutableCollection = mutableCollection // expected-warning {{was never mutated; consider changing to 'let' constant}}
var array = array // expected-warning {{was never mutated; consider changing to 'let' constant}}

sequence.sort() // expected-warning {{result of call to 'sort()' is unused}}
sequence.sort { $0 < $1 } // expected-warning {{result of call to 'sort' is unused}}
Expand Down Expand Up @@ -359,7 +362,8 @@ struct MyCollection : Sliceable {} // expected-error {{'Sliceable' has been rena
protocol MyProtocol : Sliceable {} // expected-error {{'Sliceable' has been renamed to 'CollectionType'}} {{23-32=CollectionType}}
func processCollection<E : Sliceable>(e: E) {} // expected-error {{'Sliceable' has been renamed to 'CollectionType'}} {{28-37=CollectionType}}

func renamedRangeReplaceableCollectionTypeMethods(var c: DefaultedForwardRangeReplaceableCollection<Int>) {
func renamedRangeReplaceableCollectionTypeMethods(c: DefaultedForwardRangeReplaceableCollection<Int>) {
var c = c
c.extend([ 10 ]) // expected-error {{'extend' has been renamed to 'appendContentsOf'}} {{5-11=appendContentsOf}}
c.splice([ 10 ], atIndex: c.startIndex) // expected-error {{'splice(_:atIndex:)' has been renamed to 'insertContentsOf'}} {{5-11=insertContentsOf}}
}
Expand Down
6 changes: 4 additions & 2 deletions test/ClangModules/ctypes_parse_union.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

import ctypes

func useStructWithUnion(var vec: GLKVector4) -> GLKVector4 {
func useStructWithUnion(vec: GLKVector4) -> GLKVector4 {
var vec = vec
_ = vec.v.0
_ = vec.v.1
_ = vec.v.2
Expand Down Expand Up @@ -57,7 +58,8 @@ func useStructWithAnonymousUnion(u: AnonUnion) -> AnonUnion {
return u
}

func useStructWithUnnamedUnion(var u: UnnamedUnion) -> UnnamedUnion {
func useStructWithUnnamedUnion(u: UnnamedUnion) -> UnnamedUnion {
var u = u
u.u.i = 100
u.u.f = 1.0
}
2 changes: 1 addition & 1 deletion test/ClangModules/objc_parse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func dynamicLookupMethod(b: AnyObject) {
}

// Properties
func properties(b: B) { // expected-note {{mark parameter with 'var' to make it mutable}} {{17-17=var }}
func properties(b: B) {
var i = b.counter
b.counter = i + 1
i = i + b.readCounter
Expand Down
3 changes: 2 additions & 1 deletion test/Constraints/bridging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ func rdar19831698() {
}

// <rdar://problem/19836341> Incorrect fixit for NSString? to String? conversions
func rdar19836341(ns: NSString?, var vns: NSString?) {
func rdar19836341(ns: NSString?, vns: NSString?) {
var vns = vns
let _: String? = ns // expected-error{{cannot convert value of type 'NSString?' to specified type 'String?'}}
var _: String? = ns // expected-error{{cannot convert value of type 'NSString?' to specified type 'String?'}}
// FIXME: there should be a fixit appending "as String?" to the line; for now
Expand Down
4 changes: 3 additions & 1 deletion test/Constraints/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ struct X3<T> {
init(_: (T)->()) {}
}

func testX3(var x: Int) {
func testX3(x: Int) {
var x = x
_ = X3({ x = $0 })
_ = x
}

// <rdar://problem/13811882>
Expand Down
4 changes: 3 additions & 1 deletion test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,9 @@ zip(numbers, numbers).filter { $0.2 > 1 } // expected-error {{value of tuple ty

// <rdar://problem/20868864> QoI: Cannot invoke 'function' with an argument list of type 'type'
func foo20868864(callback: ([String]) -> ()) { }
func rdar20868864(var s: String) {

func rdar20868864(s: String) {
var s = s
foo20868864 { (strings: [String]) in
s = strings // expected-error {{cannot assign value of type '[String]' to type 'String'}}
}
Expand Down
6 changes: 5 additions & 1 deletion test/Constraints/keyword_arguments.swift
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,11 @@ trailingclosure1(x: 1, { return 5 }) // expected-error{{missing argument label '
func trailingclosure2(x x: Int, f: (() -> Int)!...) {}
trailingclosure2(x: 5) { return 5 }

func trailingclosure3(x x: Int, var f: (() -> Int)!) { f = nil}
func trailingclosure3(x x: Int, f: (() -> Int)!) {
var f = f
_ = f
f = nil
}
trailingclosure3(x: 5) { return 5 }

func trailingclosure4(f f: () -> Int) {}
Expand Down
3 changes: 2 additions & 1 deletion test/Constraints/lvalues.swift
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ takeArrayRef(["asdf", "1234"]) // expected-error{{contextual type 'inout Array<S
// <rdar://problem/19835413> Reference to value from array changed
func rdar19835413() {
func f1(p: UnsafeMutablePointer<Void>) {}
func f2(var a: [Int], i: Int, pi: UnsafeMutablePointer<Int>) {
func f2(a: [Int], i: Int, pi: UnsafeMutablePointer<Int>) {
var a = a
f1(&a)
f1(&a[i])
f1(&a[0])
Expand Down
6 changes: 4 additions & 2 deletions test/Constraints/members.swift
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ protocol ClassP : class {
func bas(x: Int)
}

func generic<T: P>(var t: T) {
func generic<T: P>(t: T) {
var t = t
// Instance member of archetype
let _: Int -> () = id(t.bar)
let _: () = id(t.bar(0))
Expand Down Expand Up @@ -250,7 +251,8 @@ func genericClassP<T: ClassP>(t: T) {
// Members of existentials
////

func existential(var p: P) {
func existential(p: P) {
var p = p
// Fully applied mutating method
p.mut(1)
_ = p.mut // expected-error{{partial application of 'mutating' method is not allowed}}
Expand Down
7 changes: 4 additions & 3 deletions test/Constraints/subscript.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,12 @@ let _ = 1["1"] // expected-error {{ambiguous use of 'subscript'}}


// rdar://17687826 - QoI: error message when reducing to an untyped dictionary isn't helpful
let squares = [ 1, 2, 3 ].reduce([:]) { (var dict, n) in // expected-error {{cannot invoke 'reduce' with an argument list of type '([_ : _], @noescape (_, Int) throws -> _)'}}
let squares = [ 1, 2, 3 ].reduce([:]) { (dict, n) in // expected-error {{cannot invoke 'reduce' with an argument list of type '([_ : _], @noescape (_, Int) throws -> _)'}}
// expected-note @-1 {{expected an argument list of type '(T, combine: @noescape (T, Self.Generator.Element) throws -> T)'}}
var dict = dict // expected-error {{type of expression is ambiguous without more context}}

dict[n] = n * n // expected-error {{type of expression is ambiguous without more context}}
return dict // expected-error {{type of expression is ambiguous without more context}}
dict[n] = n * n
return dict
}


5 changes: 3 additions & 2 deletions test/Constraints/tuple.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ class C {
func f(_: C) {}
}

func testLValue(var c: C) {
func testLValue(c: C) {
var c = c
c.f(c)

let x = c
Expand All @@ -100,7 +101,7 @@ func testLValue(var c: C) {


// <rdar://problem/21444509> Crash in TypeChecker::coercePatternToType
func invalidPatternCrash(let k : Int) {
func invalidPatternCrash(k : Int) {
switch k {
case (k, cph_: k) as UInt8: // expected-error {{tuple pattern cannot match values of the non-tuple type 'UInt8'}} expected-warning {{cast from 'Int' to unrelated type 'UInt8' always fails}}
break
Expand Down
6 changes: 4 additions & 2 deletions test/Constraints/unchecked_optional.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ struct B {
var x : Int
}

func test2(var b : B!) {
func test2(b : B!) {
var b = b
let x = b.x
b.x = x
b = nil
Expand Down Expand Up @@ -85,7 +86,8 @@ func test10(x : Int?) -> Int! { return test10_helper(x) }
protocol P11 { }
extension Int : P11 { }
func test11_helper<T : P11>(t: T) { }
func test11(i: Int!, var j: Int!) {
func test11(i: Int!, j: Int!) {
var j = j
test11_helper(i)
test11_helper(j)
j = nil
Expand Down
3 changes: 2 additions & 1 deletion test/DebugInfo/autoclosure.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ func &&&&&(lhs: BooleanType, @autoclosure rhs: ()->BooleanType) -> Bool {
return lhs.boolValue ? rhs().boolValue : false
}

func call_me(var input: Int64) -> Void {
func call_me(input: Int64) -> Void {
var input = input
// rdar://problem/14627460
// An autoclosure should have a line number in the debug info and a scope line of 0.
// CHECK-DAG: !DISubprogram({{.*}}linkageName: "_TFF11autoclosure7call_meFVs5Int64T_u_KT_Ps11BooleanType_",{{.*}} line: [[@LINE+3]],{{.*}} isLocal: false, isDefinition: true
Expand Down
25 changes: 18 additions & 7 deletions test/Generics/algorithms.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ func countIf<

func equal<R1 : GeneratorType, R2 : GeneratorType where R1.Element : Eq,
R1.Element == R2.Element>
(var range1 : R1, var range2 : R2) -> Bool {
(range1 : R1, range2 : R2) -> Bool {

var range1 = range1
var range2 = range2
var e1 = range1.next()
var e2 = range2.next()

Expand All @@ -77,8 +79,10 @@ func equal<R1 : GeneratorType, R2 : GeneratorType where R1.Element : Eq,
}

func equalIf<R1 : GeneratorType, R2 : GeneratorType>
(var range1 : R1, var range2 : R2,
(range1 : R1, range2 : R2,
predicate : (R1.Element, R2.Element)-> Bool) -> Bool {
var range1 = range1
var range2 = range2
var e1 = range1.next()
var e2 = range2.next()

Expand All @@ -94,7 +98,9 @@ func equalIf<R1 : GeneratorType, R2 : GeneratorType>

func mismatch<R1 : GeneratorType, R2 : GeneratorType where R1.Element : Eq,
R1.Element == R2.Element>
(var range1 : R1, var range2 : R2) -> (R1, R2) {
(range1 : R1, range2 : R2) -> (R1, R2) {
var range1 = range1
var range2 = range2
var prev1 = range1, prev2 = range2

while true {
Expand All @@ -109,8 +115,10 @@ func mismatch<R1 : GeneratorType, R2 : GeneratorType where R1.Element : Eq,
}

func mismatchIf<R1 : GeneratorType, R2 : GeneratorType>
(var range1 : R1, var range2 : R2,
(range1 : R1, range2 : R2,
predicate : (R1.Element, R2.Element) -> Bool) -> (R1, R2) {
var range1 = range1
var range2 = range2
var prev1 = range1, prev2 = range2

while true {
Expand All @@ -124,8 +132,9 @@ func mismatchIf<R1 : GeneratorType, R2 : GeneratorType>
return (prev1, prev2)
}

func minElement<R : GeneratorType where R.Element : Comparable>(var range: R)
func minElement<R : GeneratorType where R.Element : Comparable>(range: R)
-> R.Element {
var range = range
var result = range.next()!
for next in GeneratorSequence(range) {
if next < result {
Expand All @@ -135,8 +144,9 @@ func minElement<R : GeneratorType where R.Element : Comparable>(var range: R)
return result
}

func maxElement<R : GeneratorType where R.Element : Comparable>(var range: R)
func maxElement<R : GeneratorType where R.Element : Comparable>(range: R)
-> R.Element {
var range = range
var result = range.next()!
for next in GeneratorSequence(range) {
if next > result {
Expand All @@ -146,8 +156,9 @@ func maxElement<R : GeneratorType where R.Element : Comparable>(var range: R)
return result
}

func minMaxElement<R : GeneratorType where R.Element : Comparable>(var range: R)
func minMaxElement<R : GeneratorType where R.Element : Comparable>(range: R)
-> (R.Element, R.Element) {
var range = range
var min = range.next()!, max = min
for next in GeneratorSequence(range) {
if next < min { min = next }
Expand Down
1 change: 0 additions & 1 deletion test/Generics/function_defs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ protocol IntSubscriptable {
subscript (index : Int) -> ElementType { get }
}

// expected-note @+1 {{mark parameter with 'var' to make it mutable}} {{66-66=var }}
func subscripting<T : protocol<Subscriptable, IntSubscriptable>>(t: T) {
var index = t.getIndex()
var value = t.getValue()
Expand Down
3 changes: 2 additions & 1 deletion test/Generics/generic_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ struct GenericReq<
T : GeneratorType, U : GeneratorType where T.Element == U.Element
> {}

func getFirst<R : GeneratorType>(var r: R) -> R.Element {
func getFirst<R : GeneratorType>(r: R) -> R.Element {
var r = r
return r.next()!
}

Expand Down
Loading

0 comments on commit 93b6962

Please sign in to comment.