-
Notifications
You must be signed in to change notification settings - Fork 72
feat(linter): Implement immediately_bind_scoped rule
#862
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(linter): Implement immediately_bind_scoped rule
#862
Conversation
|
praise: Ooh this is really cool! <3 |
9cb1134 to
1eeee0c
Compare
1eeee0c to
0fd6b5a
Compare
|
Majorly unsure if all of the edge-cases are actually correct now that I'm looking back at it... Would be nice if you could write a few tests/examples to test the hardest and most obscure but still "correct" uses. Also need to figure out how to handle the remaining 8 failures which all seem to follow the pattern: let px = value.unbind();
let gc = gc.into_nogc();
let py = py.get(agent);
(px.bind(gc), py.bind(gc)) |
I'll try to get on this... sometime around here? :)
Hmm... It does seem to me that these are effectively all more or less wrong and the lint is correct. I'll try fixing them up. |
…is either as a binding or as a parameter to a method call
e160dbd to
eafee71
Compare
aapoalas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, and it's a great and important step towards using GC lint safety <3
| } | ||
|
|
||
| /// Check if a use of an unbound value is valid (binding or function argument) | ||
| fn is_valid_use_of_unbound_value(cx: &LateContext<'_>, expr: &Expr, hir_id: HirId) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: Return position would also be valid: return Ok(value.get(agent)); is perfectly safe.
|
Updated the lint to check the |
Needs #790 to be merged first.
Still a bit work in progress with some edge-cases which need ironing out, but it seems to mostly work now which is pretty cool.