-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Avoid non-local definitions in functions #3373
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
Changes from 5 commits
09a0e24
632d8c9
5d5304e
85c7849
c29d41c
22ac798
66ef463
218949d
d1e9965
01045be
4d28f57
e2ae261
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
- Feature Name: N/A | ||
- Start Date: 2022-01-19 | ||
- RFC PR: [rust-lang/rfcs#3373](https://github.com/rust-lang/rfcs/pull/3373) | ||
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
Starting in Rust 2024, stop allowing items inside functions to implement | ||
methods or traits that are visible outside the function. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
Currently, tools cross-referencing uses and definitions (such as IDEs) must | ||
either search inside all function bodies to find potential definitions | ||
corresponding to uses within a function, or not cross-reference those | ||
definitions at all. | ||
|
||
Humans cross-referencing such uses and definitions may find themselves | ||
similarly baffled. | ||
|
||
With this change, both humans and tools can limit the scope of their search and | ||
avoid looking for definitions inside other functions, without missing any | ||
relevant definitions. | ||
|
||
# Explanation | ||
[explanation]: #explanation | ||
|
||
Starting in the Rust 2024 edition: | ||
- An item nested inside a function or closure (through any level of nesting) | ||
may not define an `impl Type` block unless the `Type` is also nested inside | ||
the same function or closure. | ||
- An item nested inside a function or closure (through any level of nesting) | ||
may not define an `impl Trait for Type` unless either the `Trait` or the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If If I think it boils down to: "looking at some code, can we resolve the methods and names in it without parsing the bodies of the other functions (statics, consts etc.), with the exception of the ancestors of the given code?". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant "Inner" == "local", and "Outer" == "non-local". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If only parameters of the trait/type are non-local I think it should be fine? Those impls still can't be observed from the outside since you'd need to be able to mention the trait/type still if I am not mistaken There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessarily, you can actually leak local types to the outside like this: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1921433dd4209e89c7c96949a23e99b8 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neither the trait nor the type in that impl are local to the function though, so that should be rejected by these restrictions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And potentially also cases like in https://internals.rust-lang.org/t/overly-strict-coherence-for-constrained-blanket-implementations/18204/8. I think there's like a whole coherence aspect to this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can still leak non-opaque types while following the proposed rules. If that seems far fetched, consider |
||
`Type` is also nested inside the same function or closure. | ||
joshtriplett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Rust 2015, 2018, and 2021 continue to permit this, but will produce a | ||
joshtriplett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
warn-by-default lint. | ||
|
||
No other language features provide a means of defining a name inside a function | ||
and referencing that name outside the function. | ||
joshtriplett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
Some existing code makes use of this pattern, and would need to migrate to a | ||
different pattern. In particular, this pattern may occur in macro-generated | ||
code, or in code generated by tools like rustdoc. Making this change would | ||
require such code and tools to restructure to meet this requirement. | ||
|
||
joshtriplett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Unresolved questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
We'll need a crater run to look at how widespread this pattern is in existing | ||
code. | ||
|
||
# Future possibilities | ||
[future-possibilities]: #future-possibilities | ||
|
||
If in the future Rust provides a "standalone `derive`" mechanism (e.g. `derive | ||
Trait for Type` as a standalone definition separate from `Type`), the `impl` | ||
produced by that mechanism would be subject to the same requirements. |
Uh oh!
There was an error while loading. Please reload this page.