rust-analyzer / rowan

Apache License 2.0
677 stars 57 forks source link

track mutability at the type level #118

Closed oberblastmeister closed 2 years ago

oberblastmeister commented 2 years ago

This pr adds a type level token for api::SyntaxNode so mutability information is available at compile time. This means that calling splice_children on an immutable tree will fail at compile time. Calling close_for_update on a mutable tree will also fail. I think this fits the rust philosophy as it makes mutability explicit and is tracked at compile time. Let me know if you think this makes sense for rowan.

CAD97 commented 2 years ago

@matklad this one falls to you; I don't know enough about how Rowan is used in r-a to address this PR.

oberblastmeister commented 2 years ago

The downside is that you have to parameterize stuff which can lead to more declarations. I'm not sure if that's worth it.

matklad commented 2 years ago

Oups, forgot to submit the comment. But yeah, the fact that type parameters inflct the call sites is a big deal I think

Yeah, this is something we deliberately didn't do. The problem with making mutability a type is that now all tree-traversal algorithms like https://github.com/rust-analyzer/rust-analyzer/blob/59c758d0cb86122ecc926f2cb89cff72fbc71749/crates/syntax/src/algo.rs#L58 needs to be made generic over mutability. With rust compilation model, the end-result turns out to be weird -- at runtime, there's absolutely no difference between mutable and immutable trees, they have the same representation. But, at compile time, we have to duplicate the function for processing them twice. Generics also add a fair amount of syntactic boilerplate at the call-site. Curiously, this is the case where having subtyping or Java-style erased generics would probably tip the scale towards having this.

CAD97 commented 2 years ago

But, at compile time, we have to duplicate the function for processing them twice.

(I know you're aware of this, but to reiterate) some chunk of the monomorphized code duplication can be avoided, by having the generic version dispatch to a non-generic version.

This can help for the code implementing raw tree operations, but unfortunately it's not a super lot of help for tree traversal algorithms. Doing this decreases binary code bloat, but increases the syntactic cost of providing this genericism in helpers (as said helpers now need to both thread the generics through and do the demonomorphization dance).