You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I've seen some crates making clever use of invariant lifetimes (e.g. via a PhantomData which is an fn(&'a ()) -> &'a ()). I haven't worked out the details yet, but I think in principle this technique should be exactly what we need to have the borrow checker enforce that the the descendant node passed to AstNodeRef::new is actually from the tree of the root node given. If they have exactly the same lifetime (not just lifetimes where a meet can be found), that gives the guarantee we need.
So not high priority, but at some point we could try that out and see if it allows us to better encapsulate the unsafe.
The text was updated successfully, but these errors were encountered:
carljm
changed the title
use an invariant lifetime to avoid exposing unsafe API for AstNodeRef
[red-knot] use an invariant lifetime to avoid exposing unsafe API for AstNodeRef
Jun 26, 2024
Do you still have the crates around? It would be helpful to better understand what you're describing here.
I think we discussed that the signature fn new<'a>(root: &'a ParsedModule, child: &'a T) -> Self isn't enough because that only constraints that root and child have a common lifetime, but that would also permit:
let a = parse_module("a.py");let b = parse_module("b.py");AstNodeRef::new(&a, a.syntax());// Works as expectedAstNodeRef::new(&a, b.syntax());// should not work but works too
Calling new with a as root but with b as a childworks just fine because bothaandb` life as long as the enclosing scope.
I think what would work is the following signature:
But my tentative conclusion from skimming this article is that this technique is probably more hacky and painful than it's worth, at least for our case. The current approach with unsafe is fine in practice. So I think we should just close this.
I've seen some crates making clever use of invariant lifetimes (e.g. via a PhantomData which is an
fn(&'a ()) -> &'a ()
). I haven't worked out the details yet, but I think in principle this technique should be exactly what we need to have the borrow checker enforce that the the descendant node passed toAstNodeRef::new
is actually from the tree of the root node given. If they have exactly the same lifetime (not just lifetimes where a meet can be found), that gives the guarantee we need.So not high priority, but at some point we could try that out and see if it allows us to better encapsulate the
unsafe
.The text was updated successfully, but these errors were encountered: