Skip to content

Get rid of mutable syntax node usage #15710

Open
@Veykril

Description

Somewhat meant as a means of discussion (though ultimately just expressing my opinion):

After having used and reviewed a bunch of PRs using the mutable syntax node API I am personally not too happy about it. It can be awkward to use at times and has some pitfalls that one has to be aware of. A major problem with that API is that it introduces iterator invalidation (you can iterate while mutating the tree), giving way to easily end up panicking if one does not delay the edits after iteration via collecting into a vec or similar. Likewise it makes it harder to replace our version of rowan to one that uses proper slots which is something I am in favor of, as the slot API is unlikely to have the same feature (unless we implement it back into rome's fork of rowan which would be the current slot based rowan solution).

The current benefit of it is that we have the ted api that allows automagically fixing up whitespace on edits, but that's really not necessary in the end game when we have our own syntax formatter which would allow us to format edited nodes on the fly. It is also a rather powerful api in that you can patch up syntax trees on the fly.

The main question then arises, how do we do edits if not via mutable syntax nodes? The current approach was using the handwritten make module to stitch nodes together (which is still somewhat used). But that approach is really annoying to use, inefficient and looks horrid as well. One nice looking approach would be having a quasi quote api as described in #2227, that could also do some static checks ideally (like checking well typedness of the tree at comp time maybe). With quoting one could just reconstruct the patched tree / construct a the biggest tree encompassing the changes of interested and then ask the assist building api to replace the node pointed to by a specific node ptr with the constructed tree. And alternative to mutation is #9649 though I haven't looked too much into it.

In general, assists are all over the place currently, some do text based editing, some do make api + stringified text edits and finally we have some that use the mutable node api. Obviously not ideal if we mix and match however we like to.

(Issue spawned of me really not enjoying reviewing assist PRs nowadays anymore, as they tend to be messy in general, though also because I really want us to switch to the trailing trivia model)

Metadata

Assignees

Labels

A-assistsBroken WindowBugs / technical debt to be addressed immediatelyC-ArchitectureBig architectural things which we need to figure up-front (or suggestions for rewrites :0) )E-hard

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions