Skip to content

Commit 854fa5c

Browse files
authored
[Observation] Disable caching of KeyPaths (#78853)
* [Observation] Disable caching of KeyPaths * Remove the cached keypath annottion for the ObservationTracked peer macro
1 parent f661374 commit 854fa5c

File tree

2 files changed

+37
-92
lines changed

2 files changed

+37
-92
lines changed

lib/Macros/Sources/ObservationMacros/ObservableMacro.swift

Lines changed: 36 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,6 @@ public struct ObservableMacro {
101101
"""
102102
}
103103

104-
static func canCacheKeyPaths(_ lexicalContext: [Syntax]) -> Bool {
105-
lexicalContext.allSatisfy { $0.isNonGeneric }
106-
}
107-
108104
static var ignoredAttribute: AttributeSyntax {
109105
AttributeSyntax(
110106
leadingTrivia: .space,
@@ -357,88 +353,45 @@ public struct ObservationTrackedMacro: AccessorMacro {
357353
_\(identifier) = initialValue
358354
}
359355
"""
360-
if ObservableMacro.canCacheKeyPaths(context.lexicalContext) {
361-
let getAccessor: AccessorDeclSyntax =
362-
"""
363-
get {
364-
access(keyPath: \(container.trimmed.name)._cachedKeypath_\(identifier))
365-
return _\(identifier)
366-
}
367-
"""
368-
369-
let setAccessor: AccessorDeclSyntax =
370-
"""
371-
set {
372-
guard shouldNotifyObservers(_\(identifier), newValue) else {
373-
return
374-
}
375-
withMutation(keyPath: \(container.trimmed.name)._cachedKeypath_\(identifier)) {
376-
_\(identifier) = newValue
377-
}
378-
}
379-
"""
380-
381-
// Note: this accessor cannot test the equality since it would incur
382-
// additional CoW's on structural types. Most mutations in-place do
383-
// not leave the value equal so this is "fine"-ish.
384-
// Warning to future maintence: adding equality checks here can make
385-
// container mutation O(N) instead of O(1).
386-
// e.g. observable.array.append(element) should just emit a change
387-
// to the new array, and NOT cause a copy of each element of the
388-
// array to an entirely new array.
389-
let modifyAccessor: AccessorDeclSyntax =
390-
"""
391-
_modify {
392-
let keyPath = \(container.trimmed.name)._cachedKeypath_\(identifier)
393-
access(keyPath: keyPath)
394-
\(raw: ObservableMacro.registrarVariableName).willSet(self, keyPath: keyPath)
395-
defer { \(raw: ObservableMacro.registrarVariableName).didSet(self, keyPath: keyPath) }
396-
yield &_\(identifier)
397-
}
398-
"""
399-
400-
return [initAccessor, getAccessor, setAccessor, modifyAccessor]
401-
} else {
402-
let getAccessor: AccessorDeclSyntax =
403-
"""
404-
get {
405-
access(keyPath: \\.\(identifier))
406-
return _\(identifier)
407-
}
408-
"""
356+
let getAccessor: AccessorDeclSyntax =
357+
"""
358+
get {
359+
access(keyPath: \\.\(identifier))
360+
return _\(identifier)
361+
}
362+
"""
409363

410-
let setAccessor: AccessorDeclSyntax =
411-
"""
412-
set {
413-
guard shouldNotifyObservers(_\(identifier), newValue) else {
414-
return
415-
}
416-
withMutation(keyPath: \\.\(identifier)) {
417-
_\(identifier) = newValue
418-
}
364+
let setAccessor: AccessorDeclSyntax =
365+
"""
366+
set {
367+
guard shouldNotifyObservers(_\(identifier), newValue) else {
368+
return
419369
}
420-
"""
421-
422-
// Note: this accessor cannot test the equality since it would incur
423-
// additional CoW's on structural types. Most mutations in-place do
424-
// not leave the value equal so this is "fine"-ish.
425-
// Warning to future maintence: adding equality checks here can make
426-
// container mutation O(N) instead of O(1).
427-
// e.g. observable.array.append(element) should just emit a change
428-
// to the new array, and NOT cause a copy of each element of the
429-
// array to an entirely new array.
430-
let modifyAccessor: AccessorDeclSyntax =
431-
"""
432-
_modify {
433-
access(keyPath: \\.\(identifier))
434-
\(raw: ObservableMacro.registrarVariableName).willSet(self, keyPath: \\.\(identifier))
435-
defer { \(raw: ObservableMacro.registrarVariableName).didSet(self, keyPath: \\.\(identifier)) }
436-
yield &_\(identifier)
370+
withMutation(keyPath: \\.\(identifier)) {
371+
_\(identifier) = newValue
437372
}
438-
"""
373+
}
374+
"""
375+
376+
// Note: this accessor cannot test the equality since it would incur
377+
// additional CoW's on structural types. Most mutations in-place do
378+
// not leave the value equal so this is "fine"-ish.
379+
// Warning to future maintence: adding equality checks here can make
380+
// container mutation O(N) instead of O(1).
381+
// e.g. observable.array.append(element) should just emit a change
382+
// to the new array, and NOT cause a copy of each element of the
383+
// array to an entirely new array.
384+
let modifyAccessor: AccessorDeclSyntax =
385+
"""
386+
_modify {
387+
access(keyPath: \\.\(identifier))
388+
\(raw: ObservableMacro.registrarVariableName).willSet(self, keyPath: \\.\(identifier))
389+
defer { \(raw: ObservableMacro.registrarVariableName).didSet(self, keyPath: \\.\(identifier)) }
390+
yield &_\(identifier)
391+
}
392+
"""
439393

440-
return [initAccessor, getAccessor, setAccessor, modifyAccessor]
441-
}
394+
return [initAccessor, getAccessor, setAccessor, modifyAccessor]
442395
}
443396
}
444397

@@ -467,15 +420,7 @@ extension ObservationTrackedMacro: PeerMacro {
467420
}
468421

469422
let storage = DeclSyntax(property.privatePrefixed("_", addingAttribute: ObservableMacro.ignoredAttribute))
470-
if ObservableMacro.canCacheKeyPaths(context.lexicalContext) {
471-
let cachedKeypath: DeclSyntax =
472-
"""
473-
private static let _cachedKeypath_\(identifier) = \\\(container.name).\(identifier)
474-
"""
475-
return [storage, cachedKeypath]
476-
} else {
477-
return [storage]
478-
}
423+
return [storage]
479424
}
480425
}
481426

stdlib/public/Observation/Sources/Observation/Observable.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public macro Observable() =
5151
/// framework isn't necessary.
5252
@available(SwiftStdlib 5.9, *)
5353
@attached(accessor, names: named(init), named(get), named(set), named(_modify))
54-
@attached(peer, names: prefixed(_), prefixed(_cachedKeypath_))
54+
@attached(peer, names: prefixed(_))
5555
public macro ObservationTracked() =
5656
#externalMacro(module: "ObservationMacros", type: "ObservationTrackedMacro")
5757

0 commit comments

Comments
 (0)