[libSyntax] Make RawSyntax a struct #18276
Conversation
SourcePresence and ID are already shared between the two node kinds and the node's length will soon be cached in RawSyntax as well. By making it a struct, we will be able to compute the node's length when it is being constructed in the initialiser.
|
@swift-ci Please smoke test |
|
Probably @harlanhaskins should take a look at this. |
|
LGTM, this refactoring was bound to happen. Thanks for doing it @ahoppen! |
| @@ -43,53 +53,57 @@ struct SyntaxNodeId: Hashable, Codable { | |||
| } | |||
| } | |||
|
|
|||
| /// The data that is specific to a tree or token node | |||
| fileprivate indirect enum RawSyntaxData { | |||
There was a problem hiding this comment.
Why this is declared as indirect? I know the original RawSyntax was also indirect. @harlanhaskins, what was the reason? maybe MemoryLayout<RawSyntax>.size?
There was a problem hiding this comment.
At the time of writing, it ensured both cases were heap allocated and that a copy = a retain, so we could share raw data. Pretty sure that is still the case.
There was a problem hiding this comment.
It's because .node has RawSyntax as its member which again has RawSyntaxData as one of its members. That's a cycle and thus the object size wouldn't be determined at runtime if the enum is not indirect.
Or TL;DR: It doesn't compile if the enum isn't indirect.
There was a problem hiding this comment.
It should still compile. It’s already got a layer of indirection via the array of RawSyntax?.
There was a problem hiding this comment.
@ahoppen It does compile because [RawSyntaxData] provides indirect semantics.
There was a problem hiding this comment.
At the time of writing, it ensured both cases were heap allocated and that a copy = a retain, so we could share raw data.
Makes sense. Thanks :)
There was a problem hiding this comment.
It does compile because [RawSyntaxData] provides indirect semantics.
Hmm. I was pretty sure it didn't compile at some point without indirect. But maybe that was just my imagination. @harlanhaskins's explanation makes sense. I'll leave it as is.
SourcePresence and ID are already shared between the two node kinds and the node's length will soon be cached in RawSyntax as well. By making it a struct, this shared data can be stored more easily.