Skip to content
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.

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

[SR-5250] Establish if this is intended behavior or bug in Dictionary's new subscript(_:default:) #47825

Open
jepers opened this issue Jun 18, 2017 · 3 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella

Comments

@jepers
Copy link

jepers commented Jun 18, 2017

Previous ID SR-5250
Radar None
Original Reporter @jepers
Type Bug
Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug
Assignee None
Priority Medium

md5: d7492344859e178e647ee3da9471e91d

Issue Description:

// Swift 4, Xcode 9 beta 1, default toolchain

import Foundation

var d1 = [Int : String]()
d1[1, default: .init()].append("a")
d1[2, default: .init()].append("b")
d1[3, default: .init()].append("c")
d1[1, default: .init()].append("d")
print(d1) // [2: "b", 3: "c", 1: "ad"] as expected.

var d2 = [Int : NSMutableString]()
d2[1, default: .init()].append("a")
d2[2, default: .init()].append("b")
d2[3, default: .init()].append("c")
d2[1, default: .init()].append("d")
print(d2) // [:] but why?

I know that NSMutableString is a reference type and String is a value type and that the default argument is an @autoclosure. I also know that the newly created NSMutableString instance is just released immediately after the append call, without being stored and retained in the Dictionary's storage.

Is this the intended behavior?

@belkadan
Copy link
Contributor

Looks like a bug to me. @dabrahams, @airspeedswift?

@natecook1000
Copy link
Member

This issue originated on the swift-users list. @airspeedswift's initial response is here: https://lists.swift.org/pipermail/swift-users/Week-of-Mon-20170612/005598.html

The core issue (as far as I understand it) is that since the methods on reference types aren't mutating methods (because classes don't have those) the subscript setter is never called. So everything in this case is "working as designed", but the behavior is super confusing.

Essentially, because the compiler doesn't distinguish between the first subscript calls here, it's hard to make this operate the way we'd like—we can't make one of these write back to the dictionary without making both do so:

var d2 = [Int: NSMutableString]()
d2[1, default: .init()].appending("a")    // nonmutating
d2[2, default: .init()].append("b")       // "mutating"

I don't have a great solution for this in mind—it may be this is just the wrong way to respond to this use case. We had some other alternatives for this behavior while discussing the Keys and Values collections, including something like this method:

extension Dictionary {
  mutating func update(forKey key: Key, default: @autoclosure () -> Value, _ body: (inout Value) throws -> Void) rethrows {
    var value = self[key] ?? `default`()
    try body(&value)
    self[key] = value
  }
}

var d = ["a": 1, "b": 2, "c": 3]
d.update(forKey: "a", default: 0) { $0 += 1 }
d.update(forKey: "d", default: 0) { $0 += 1 }
// d == ["a": 2, "b": 2, "c": 3, "d": 1]

@swift-ci
Copy link
Collaborator

Comment by Mathias Quintero (JIRA)

But if NSMutableString is a class then technically the only thing being stored in the dictionary is a reference.

Append wouldn't need to write to the dictionary. It needs to write to the string. The reference should stay the same

This really seems like unintended behavior.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

4 participants