Replies: 1 comment
-
Feel free to send a PR :) |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
SyntaxSlots
is not exportedpub
inbiome_rowan
. It would be nice to have this exported, because otherwise it is impossible for the downstream crate to reimplement the custom ast support functions and iterators logic. I think someone forget to export it, because I don't think anyone intended to make it private.I have a use case where I want to reimplement all the support functions, custom iterators over the syntax slots (my own
AstSeparatedListIterator
with all the package that come with it).I don't really like the Biome implementation that
panic
on purpose for anything that are unexpected - this should never happen because of the static and deterministic nature of code generation that is proved to be safe, but I'm sure you know that you can create detached node and token at runtime so there's no panic free guarantee for the downstream - and theses detached functions are not marked as unsafe because there's no unsafe function used. This can cause problem later (panic
or at worst undefined behavior if you implement lower level datastructure without knowing this), but not immediately.A prime example : it's possible to create a detached
SyntaxNode
that have a slot marked missing while the Biome spec say list should always be marked present, and this will crash at runtime once the ast navigation function is called. Another example is when you have a token instead of a node (or the reverse).I would like to try a new model where the ast allow arbitrary structure while another abstract datastructure wrap the ast and provide complete typesafe navigation with respect to the language spec (no
SyntaxResult
, noOption
for the required, nopanic
, everything is present and well-formed). This require carefull validation logic on the ast and it's childrens. Once validated, this new datastructure provide all the ast navigation function with the little addition ofunwrap_unchecked
everywhere. This can be codegenerated.For this I need to reimplement my own ast, to get precise error information returned to the caller instead of
panic
when the unexpected happen - and this requirepub
SyntaxSlots
to recreate some form of iterator that doesn'tpanic
🖌️ .For now my current workaround is to assume that all
SyntaxNode
andSyntaxToken
are always following the language spec. Otherwise,panic
will happen at runtime. This also imply that all ast node follow the language spec, so care have to be taken while using the detached API.My new model is to remove theses invariants, because
SyntaxNode::new_detached
andSyntaxToken::new_detached
can lead to runtimepanic
if misused. If an ast can represent arbitrary tree that doesn't follow the language spec (to much childrens but not a bogus, invalid slot element, text that are nonsense for a token), this eliminate the problem. It's up to the caller to react on such arbitrary ast node. In contrast, the new datastructure would ensure the ast (and the subsequent syntax node) follow the language spec and provide navigation function with 0Option
/SyntaxResult
(except for the optional element). Mutation API and a builder API can also exist at this level, but care have to be taken to not mix invalid and valid element so at this level we can have a typesafe mutation & typesafe builder, AND non-typesafe mutation that are faillible & non-typesafe builder that are faillible. That way, the 3 layers can live together.Tldr is I can't recreate all of theses primitives without forking since I'm missing
SyntaxSlots
.Anyway thanks for this fantastic crate.
Pro :
SyntaxSlots
type already leak outside of it's module boundary, sinceSyntaxNode::syntax().slots()
ispub
.SyntaxSlots
are part of Biome for a long time now, and a key evolution for Rowan. And I doubt this gonna change at any time.Beta Was this translation helpful? Give feedback.
All reactions