Skip to content

[stdlib] Refactor Dictionary.subscript._modify for better layering #20918

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

Merged
merged 1 commit into from
Dec 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 1 addition & 38 deletions stdlib/public/core/Dictionary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -794,44 +794,7 @@ extension Dictionary {
}
}
_modify {
// FIXME: This code should be moved to _variant, with Dictionary.subscript
// yielding `&_variant[key]`.

let (bucket, found) = _variant.mutatingFind(key)

// FIXME: Mark this entry as being modified in hash table metadata
// so that lldb can recognize it's not valid.

// Move the old value (if any) out of storage, wrapping it into an
// optional before yielding it.
let native = _variant.asNative
if found {
var value: Value? = (native._values + bucket.offset).move()
yield &value
if let value = value {
// **Mutation**
//
// Initialize storage to new value.
(native._values + bucket.offset).initialize(to: value)
} else {
// **Removal**
//
// We've already deinitialized the value; deinitialize the key too and
// register the removal.
(native._keys + bucket.offset).deinitialize(count: 1)
native._delete(at: bucket)
}
} else {
var value: Value? = nil
yield &value
if let value = value {
// **Insertion**
//
// Insert the new entry at the correct place. Note that
// `mutatingFind` already ensured that we have enough capacity.
native._insert(at: bucket, key: key, value: value)
}
}
yield &_variant[key]
_fixLifetime(self)
}
}
Expand Down
25 changes: 25 additions & 0 deletions stdlib/public/core/DictionaryVariant.swift
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,31 @@ extension Dictionary._Variant: _DictionaryBuffer {
}
}

extension Dictionary._Variant {
@inlinable
internal subscript(key: Key) -> Value? {
@inline(__always)
get {
return lookup(key)
}
@inline(__always)
_modify {
#if _runtime(_ObjC)
guard isNative else {
let cocoa = asCocoa
var native = _NativeDictionary<Key, Value>(
cocoa, capacity: cocoa.count + 1)
self = .init(native: native)
yield &native[key, isUnique: true]
return
}
#endif
let isUnique = isUniquelyReferenced()
yield &asNative[key, isUnique: isUnique]
}
}
}

extension Dictionary._Variant {
/// Same as find(_:), except assume a corresponding key/value pair will be
/// inserted if it doesn't already exist, and mutated if it does exist. When
Expand Down
38 changes: 38 additions & 0 deletions stdlib/public/core/NativeDictionary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,44 @@ extension _NativeDictionary: _DictionaryBuffer {
}
}

extension _NativeDictionary {
@inlinable
subscript(key: Key, isUnique isUnique: Bool) -> Value? {
@inline(__always)
get {
// Dummy definition; don't use.
return lookup(key)
}
@inline(__always)
_modify {
let (bucket, found) = mutatingFind(key, isUnique: isUnique)
if found {
// Move the old value out of storage, wrapping it into an optional
// before yielding it.
var value: Value? = (_values + bucket.offset).move()
yield &value
if let value = value {
// **Mutation.** Initialize storage to new value.
(_values + bucket.offset).initialize(to: value)
} else {
// **Removal.** We've already deinitialized the value; deinitialize
// the key too and register the removal.
(_keys + bucket.offset).deinitialize(count: 1)
_delete(at: bucket)
}
} else {
var value: Value? = nil
yield &value
if let value = value {
// **Insertion.** Insert the new entry at the correct place. Note
// that `mutatingFind` already ensured that we have enough capacity.
_insert(at: bucket, key: key, value: value)
}
}
}
}
}

// This function has a highly visible name to make it stand out in stack traces.
@usableFromInline
@inline(never)
Expand Down