Skip to content

Commit 5e7a42f

Browse files
committed
Disallow getting a SyntaxArena from a SyntaxArenaRef
1 parent 389f0f4 commit 5e7a42f

File tree

1 file changed

+24
-14
lines changed

1 file changed

+24
-14
lines changed

Sources/SwiftSyntax/SyntaxArena.swift

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ public class SyntaxArena {
4949
#if DEBUG || SWIFTSYNTAX_ENABLE_ASSERTIONS
5050
/// Whether or not this arena has been added to other arenas as a child.
5151
/// Used to make sure we don’t introduce retain cycles between arenas.
52-
private var hasParent: Bool
52+
///
53+
/// - Important: This is only intended to be used for assertions to catch
54+
/// retain cycles in syntax arenas.
55+
var hasParent: Bool
5356
#endif
5457

5558
/// Construct a new ``SyntaxArena`` in which syntax nodes can be allocated.
@@ -146,20 +149,11 @@ public class SyntaxArena {
146149
if childRefs.insert(otherRef).inserted {
147150
otherRef.retain()
148151
#if DEBUG || SWIFTSYNTAX_ENABLE_ASSERTIONS
149-
// FIXME: This may trigger a data race warning in Thread Sanitizer.
150-
// Can we use atomic bool here?
151-
otherRef.value.hasParent = true
152+
otherRef.setHasParent(true)
152153
#endif
153154
}
154155
}
155156

156-
/// Recursively checks if this arena contains given `arenaRef` as a descendant.
157-
func contains(arenaRef: SyntaxArenaRef) -> Bool {
158-
childRefs.contains { childRef in
159-
childRef == arenaRef || childRef.value.contains(arenaRef: arenaRef)
160-
}
161-
}
162-
163157
/// Checks if the given syntax text is managed by this arena.
164158
///
165159
/// "managed" means it's empty, a part of "source buffer", or in the memory
@@ -264,15 +258,15 @@ struct SyntaxArenaRef: Hashable, @unchecked Sendable {
264258
}
265259

266260
/// Returns the ``SyntaxArena``
267-
var value: SyntaxArena {
261+
private var value: SyntaxArena {
268262
get { self._value.takeUnretainedValue() }
269263
}
270264

271265
/// Assuming that this references a `ParsingSyntaxArena`,
272266
func parseTrivia(source: SyntaxText, position: TriviaPosition) -> [RawTriviaPiece] {
273267
// It is safe to access `_value` here because `parseTrivia` only accesses
274268
// `parseTriviaFunction`, which is a constant.
275-
(_value.takeUnretainedValue() as! ParsingSyntaxArena).parseTrivia(source: source, position: position)
269+
(value as! ParsingSyntaxArena).parseTrivia(source: source, position: position)
276270
}
277271

278272
func retain() {
@@ -285,9 +279,25 @@ struct SyntaxArenaRef: Hashable, @unchecked Sendable {
285279

286280
/// Get an opaque wrapper that keeps the syntax arena alive.
287281
var retained: RetainedSyntaxArena {
288-
return RetainedSyntaxArena(_value.takeUnretainedValue())
282+
return RetainedSyntaxArena(value)
283+
}
284+
285+
#if DEBUG || SWIFTSYNTAX_ENABLE_ASSERTIONS
286+
/// Accessor for ther underlying's `SyntaxArena.hasParent`
287+
var hasParent: Bool {
288+
// FIXME: This accesses mutable state across actor boundaries and is thus not concurrency-safe.
289+
// We should use an atomic for `hasParent` to make it concurrency safe.
290+
value.hasParent
289291
}
290292

293+
/// Sets the `SyntaxArena.hasParent` on the referenced arena.
294+
func setHasParent(_ newValue: Bool) {
295+
// FIXME: This modifies mutable state across actor boundaries and is thus not concurrency-safe.
296+
// We should use an atomic for `hasParent` to make it concurrency safe.
297+
value.hasParent = newValue
298+
}
299+
#endif
300+
291301
func hash(into hasher: inout Hasher) {
292302
hasher.combine(_value.toOpaque())
293303
}

0 commit comments

Comments
 (0)