Skip to content

Commit

Permalink
Make function parameters and refutable patterns always immutable
Browse files Browse the repository at this point in the history
All refutable patterns and function parameters marked with 'var'
is now an error.

- Using explicit 'let' keyword on function parameters causes a warning.
- Don't suggest making function parameters mutable
- Remove uses in the standard library
- Update tests

rdar://problem/23378003
  • Loading branch information
bitjammer committed Nov 10, 2015
1 parent 22ba61b commit 8f2fbdc
Show file tree
Hide file tree
Showing 99 changed files with 729 additions and 579 deletions.
6 changes: 4 additions & 2 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -641,8 +641,10 @@ ERROR(untyped_pattern_ellipsis,pattern_parsing,none,
"'...' cannot be applied to a subpattern which is not explicitly typed", ())
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 '%select{var|let}0' binding here is deprecated and will be removed in a future version of Swift", (unsigned))
ERROR(var_not_allowed_in_pattern,pattern_parsing, none,
"Use of 'var' binding here is not allowed", ())
WARNING(let_on_param_is_redundant,pattern_parsing, none,
"'let' keyword is unnecessary; function parameters are immutable by default", (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
20 changes: 0 additions & 20 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3268,26 +3268,6 @@ void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const {
.fixItReplace(PBD->getLoc(), "var");
return;
}

// If this is an argument pattern, suggest adding 'var' to the pattern.
if (auto *PD = dyn_cast<ParamDecl>(this)) {
if (auto *P = PD->getParamParentPattern()) {
if (P->getStartLoc().isValid() && !PD->isImplicit()) {
// The pattern is: (pattern_let (pattern_typed (pattern_named)))
// where the outside let pattern may be missing. If present, this is a
// change, otherwise this is an addition of "var"
auto &d = getASTContext().Diags;
if (auto *VP = dyn_cast<VarPattern>(P)) {
d.diagnose(VP->getLoc(), diag::change_let_to_var_param)
.fixItReplace(VP->getLoc(), "var");
} else {
d.diagnose(P->getStartLoc(), diag::mark_param_var)
.fixItInsert(P->getStartLoc(), "var ");
}
return;
}
}
}
}


Expand Down
14 changes: 7 additions & 7 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,15 @@ 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,
diagnose(Tok.getLoc(), diag::let_on_param_is_redundant,
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());
diagnose(Tok.getLoc(), diag::var_not_allowed_in_pattern)
.fixItRemove(Tok.getLoc());
param.LetVarInOutLoc = consumeToken();
param.SpecifierKind = ParsedParameter::Var;
param.SpecifierKind = ParsedParameter::Let;
}

// Redundant specifiers are fairly common, recognize, reject, and recover
Expand Down Expand Up @@ -855,8 +855,8 @@ ParserResult<Pattern> Parser::parsePattern() {
} else {
// In an always immutable context, `var` is not allowed.
if (alwaysImmutable)
diagnose(varLoc, diag::var_not_allowed_in_pattern, isLetKeyword)
.fixItRemove(varLoc);
diagnose(varLoc, diag::var_not_allowed_in_pattern)
.fixItRemove(varLoc);
}

// In our recursive parse, remember that we're in a var/let pattern.
Expand Down Expand Up @@ -1067,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, isLet)
diagnose(varLoc, diag::var_not_allowed_in_pattern)
.fixItReplace(varLoc, "let");

// In our recursive parse, remember that we're in a var/let pattern.
Expand Down
13 changes: 10 additions & 3 deletions stdlib/private/StdlibUnittest/CheckCollectionType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ extension TestSuite {
Collection.SubSequence.SubSequence == Collection.SubSequence,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -302,6 +302,8 @@ extension TestSuite {
outOfBoundsSubscriptOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down Expand Up @@ -786,7 +788,7 @@ self.test("\(testNamePrefix).removeFirst(n: Int)/slice/removeTooMany/semantics")
CollectionWithEquatableElement.Index : BidirectionalIndexType,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -800,6 +802,9 @@ self.test("\(testNamePrefix).removeFirst(n: Int)/slice/removeTooMany/semantics")
outOfBoundsIndexOffset: Int = 1,
outOfBoundsSubscriptOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down Expand Up @@ -1088,7 +1093,7 @@ self.test("\(testNamePrefix).suffix/semantics") {
CollectionWithEquatableElement.Index : RandomAccessIndexType,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -1103,6 +1108,8 @@ self.test("\(testNamePrefix).suffix/semantics") {
outOfBoundsSubscriptOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ extension TestSuite {
CollectionWithEquatableElement.Generator.Element : Equatable,
CollectionWithComparableElement.Generator.Element : Comparable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -99,6 +99,8 @@ extension TestSuite {
withUnsafeMutableBufferPointerIsSupported: Bool
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down Expand Up @@ -383,7 +385,7 @@ self.test("\(testNamePrefix).sort/${'Predicate' if predicate else 'WhereElementI
CollectionWithComparableElement.Index : BidirectionalIndexType,
CollectionWithComparableElement.Generator.Element : Comparable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -403,6 +405,8 @@ self.test("\(testNamePrefix).sort/${'Predicate' if predicate else 'WhereElementI
withUnsafeMutableBufferPointerIsSupported: Bool
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down Expand Up @@ -500,7 +504,7 @@ if resiliencyChecks.subscriptOnOutOfBoundsIndicesBehavior != .None {
CollectionWithComparableElement.Index : RandomAccessIndexType,
CollectionWithComparableElement.Generator.Element : Comparable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -520,6 +524,8 @@ if resiliencyChecks.subscriptOnOutOfBoundsIndicesBehavior != .None {
withUnsafeMutableBufferPointerIsSupported: Bool
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ extension TestSuite {
Collection.SubSequence.SubSequence == Collection.SubSequence,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -362,6 +362,8 @@ extension TestSuite {
outOfBoundsIndexOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down Expand Up @@ -1108,7 +1110,7 @@ self.test("\(testNamePrefix).OperatorPlus") {
CollectionWithEquatableElement.Index : BidirectionalIndexType,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -1122,6 +1124,8 @@ self.test("\(testNamePrefix).OperatorPlus") {
outOfBoundsIndexOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down Expand Up @@ -1233,7 +1237,7 @@ self.test("\(testNamePrefix).removeLast(n: Int)/whereIndexIsBidirectional/remove
CollectionWithEquatableElement.Index : RandomAccessIndexType,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -1247,6 +1251,8 @@ self.test("\(testNamePrefix).removeLast(n: Int)/whereIndexIsBidirectional/remove
outOfBoundsIndexOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ extension TestSuite {
Collection.SubSequence == Collection,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -34,6 +34,8 @@ extension TestSuite {
outOfBoundsIndexOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

// Don't run the same tests twice.
if checksAdded.value.contains(__FUNCTION__) {
return
Expand Down Expand Up @@ -150,7 +152,7 @@ extension TestSuite {
CollectionWithEquatableElement.SubSequence == CollectionWithEquatableElement,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -164,6 +166,8 @@ extension TestSuite {
outOfBoundsIndexOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

// Don't run the same tests twice.
if checksAdded.value.contains(__FUNCTION__) {
return
Expand Down Expand Up @@ -294,7 +298,7 @@ extension TestSuite {
CollectionWithEquatableElement.SubSequence == CollectionWithEquatableElement,
CollectionWithEquatableElement.Generator.Element : Equatable
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeCollection: ([Collection.Generator.Element]) -> Collection,
wrapValue: (OpaqueValue<Int>) -> Collection.Generator.Element,
extractValue: (Collection.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -308,6 +312,8 @@ extension TestSuite {
outOfBoundsIndexOffset: Int = 1
) {

var testNamePrefix = testNamePrefix

// Don't run the same tests twice.
if checksAdded.value.contains(__FUNCTION__) {
return
Expand Down
4 changes: 3 additions & 1 deletion stdlib/private/StdlibUnittest/CheckSequenceType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ extension TestSuite {
Sequence.SubSequence.Generator.Element == Sequence.Generator.Element,
Sequence.SubSequence.SubSequence == Sequence.SubSequence
>(
var testNamePrefix: String = "",
testNamePrefix: String = "",
makeSequence: ([Sequence.Generator.Element]) -> Sequence,
wrapValue: (OpaqueValue<Int>) -> Sequence.Generator.Element,
extractValue: (Sequence.Generator.Element) -> OpaqueValue<Int>,
Expand All @@ -1452,6 +1452,8 @@ extension TestSuite {
resiliencyChecks: CollectionMisuseResiliencyChecks = .all
) {

var testNamePrefix = testNamePrefix

if checksAdded.value.contains(__FUNCTION__) {
return
}
Expand Down
3 changes: 2 additions & 1 deletion stdlib/private/StdlibUnittest/OpaqueIdentityFunctions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ func _blackHolePtr<T>(x: UnsafePointer<T>) {
_stdlib_getPointer(COpaquePointer(x))
}

public func _blackHole<T>(var x: T) {
public func _blackHole<T>(x: T) {
var x = x
_blackHolePtr(&x)
}

Expand Down
3 changes: 2 additions & 1 deletion stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -2008,7 +2008,8 @@ public func check${traversal}Collection<
x + offset0 >= 0 && x + offset1 <= count
}

func nextN(n: C.Index.Distance, var _ i: C.Index) -> C.Index {
func nextN(n: C.Index.Distance, _ i: C.Index) -> C.Index {
var i = i
if n < 0 {
for _ in 0 ..< -(n.toIntMax()) {
--i
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
public struct MsgPackSerializer {
public static func serialize<
T : SerializableFixedDictionaryType
>(var value: T) -> [UInt8] {
>(value: T) -> [UInt8] {
var value = value
// FIXME(performance): we could serialize directly into a byte stream
// without creating an intermediate data structure if our MsgPack
// serializer could accept arrays and dictionaries of unknown length and
Expand Down Expand Up @@ -76,7 +77,8 @@ internal func _toMsgPackVariant<
}
% end
return .Array(MsgPackVariantArray(value.map {
(var element) -> MsgPackVariant in
element -> MsgPackVariant in
var element = element
var serializerImpl = _MsgPackSerializerImpl()
element.serializeDeserialize(&serializerImpl)
return serializerImpl._value!
Expand Down
3 changes: 2 additions & 1 deletion stdlib/public/SDK/AssetsLibrary/AssetsLibrary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
//===----------------------------------------------------------------------===//
extension ALAssetsLibrary {
@nonobjc
public func enumerateGroupsWithTypes(var types: UInt32,
public func enumerateGroupsWithTypes(types: UInt32,
usingBlock enumerationBlock: ALAssetsLibraryGroupsEnumerationResultsBlock!,
failureBlock: ALAssetsLibraryAccessFailureBlock!) {
var types = types
if (types == ALAssetsGroupAll) {
types = ALAssetsGroupLibrary | ALAssetsGroupAlbum | ALAssetsGroupEvent |
ALAssetsGroupFaces | ALAssetsGroupSavedPhotos |
Expand Down
3 changes: 2 additions & 1 deletion stdlib/public/core/Arrays.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,9 @@ public func _allocateUninitializedArray<Element>(_count: Builtin.Word)
@warn_unused_result
@_semantics("array.dealloc_uninitialized")
public func _deallocateUninitialized${Self}<Element>(
var array: ${Self}<Element>
array: ${Self}<Element>
) {
var array = array
array._deallocateUninitialized()
}
%end
Expand Down
3 changes: 2 additions & 1 deletion stdlib/public/core/CollectionAlgorithms.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ ${partitionDocComment}
${orderingRequirementForPredicate}
public mutating func partition(
range: Range<Index>,
var isOrderedBefore: (${GElement}, ${GElement}) -> Bool
isOrderedBefore: (${GElement}, ${GElement}) -> Bool
) -> Index {
var isOrderedBefore = isOrderedBefore

% else:

Expand Down
Loading

0 comments on commit 8f2fbdc

Please sign in to comment.