Remove Types type familiy

[matklad]

Currently, the tree is parametrized over kind and root data, but this is not really necessary. Kind is basically an integer tag, and we can mark it as such.

Root data indeed needs to be custom, but the tree itself does not use it all all, it only stores the data. So, we can just use Any here. Given that the users are expected to wrap API anyway, loss of type-safety does not seem like a big problem.

I am not 100% sure that this is a good idea, but it seems so!

[matklad]

@jD91mZM2 curious, what would you think about this?

In rust-analyzer, we wrap the whole API of rowan into our own type anyway, so loss of type-safety is not a problem at all.

[jD91mZM2]

I think this may be the first PR which I don't agree with. The function to map the enum to an integer and back is basically just boilerplate, and removing generics removes abilities to use types either smaller or larger than u16. In my experience you can't remove generic parameters for long anyway, since you might find some other reason to keep them. Not to mention using integers will encourage lazy peeps like me to just use constants instead of creating an actual, typed, enum.

Using Any for root data also adds an unnecessary allocation, especially annoying since you in most cases allocate something small like a Vec<SomeType>, which now with the system allocator as default is slow.

That said, I honestly think root data can be dropped completely. I personally see no reason not to wrap the AST in a custom struct anyway, since then you can also provide some of your own commands, see rnix's source code.

[matklad]

The function to map the enum to an integer and back is basically just boilerplate,

That is true, yeah.

removing generics removes abilities to use types either smaller or larger than u16

Note that using u8 does not affect the size of any nodes, and u16 seems to be enough for any reasonable grammar?

Not to mention using integers will encourage lazy peeps like me to just use constants instead of creating an actual, typed, enum.

I think that is not necessary bad! The enum would be basically non-exhaustive anyway, so there's little difference between an enum and a module with constants. In rust-analyzer, we even use SCREAMING_SNAKE_CASE to name the enum's variants, because they are used like constants. I guess we should rework the example to just list the constants instead of inventing an enum.

Using Any for root data also adds an unnecessary allocation,

Yeah... OTOH, being generic over root data does not make sense at all: we never look at it anyway, so there's zero reasons to monomorphise code. I also agree that RootData is probably an anti-pattern: in ra, we store errors there, and I want to change that to just returning a pair (syntax tree, Vec) from the parser. That's why root data is an option: you can just set it to None and don't care.

That said, ability to have RootData seems fundamental: if we don't have a slot for it, it would be impossible to attach it from outside. Even if you pass piars of (Tree, Data) everywhere, you can't bind the Data to the children in the tree.

I also wouldn't worry about allocation to much here: we are allocating a SyntaxRoot and a GreenNode anyway, so it would be the third allocation in the best case.

[matklad]

We've discussed this in Zulip Discord and the conclusion is, while this API is indeed surprising with loose types, it is indeed the case where we know what we are doing.

[matklad]

I'll merge this, migrate rust-analyzer to it and, unless anyhing blows up, publish a new major version

[matklad]

https://github.com/rust-analyzer/rust-analyzer/pull/1123