Add a new lint that warns for pointers to stack memory #134218
Conversation
|
rustbot has assigned @compiler-errors. Use |
|
I am aware that this may warn on existing code. I wonder what is a good strategy here? Should it be an |
|
Is there a reason why this was implemented as a rust lint rather than a clippy suspicious lint or something? I guess let's find the fallout of this lint @bors try |
Add a new lint that warns for pointers to stack memory
This adds a new lint with level `Warn` to check for code like:
```rust
fn foo() -> *const i32 {
let x = 42;
&x
}
```
and produce a warning like:
```text
error: returning a pointer to stack memory associated with a local variable
--> <source>:12:5
|
LL| &x
| ^^
```
This fixes rust-lang#134215.
This was completely my judgment, I am happy to change this if requested. My reasoning was that this is a pretty fundamental lint, since the code it lints should probably never exist? And looking at the list of rustc lints I thought it might be a good fit. |
This comment has been minimized.
This comment has been minimized.
40eb5d7 to
e162e89
Compare
Oops - I don't fully understand that error. The failure looks like in the .stderr file, but it still has unexpected errors that it doesn't match on... This even happens when passing |
This comment has been minimized.
This comment has been minimized.
|
@craterbot check |
|
🚨 Error: missing start toolchain 🆘 If you have any trouble with Crater please ping |
|
@craterbot run mode=check-only start=master#8e37e151835d96d6a7415e93e6876561485a3354 end=try#6db1a6a7ebb2ad243051f3ff949f987dd70cde2b |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
bless will generate a .stderr file for you, but it will not update the original .rs file. compiletest is complaining because it expects each warning to be annotated with |
This comment has been minimized.
This comment has been minimized.
3c39f0f to
f56a313
Compare
|
The Miri subtree was changed cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
f56a313 to
80a2f21
Compare
This comment has been minimized.
This comment has been minimized.
80a2f21 to
4e7732c
Compare
|
@craterbot end=try#6db1a6a7ebb2ad243051f3ff949f987dd70cde2b+rustflags=-Dreturn-local-variable-ptr |
|
📝 Configuration of the ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
|
Looks like this just hit one crate (out of 553468!). There was an ICE happening as well, which looks unrelated. The other 119 errors look like spurious failures (something wrong with the docker invocation). Can we conclude that this lint will not break a great amount of existing users? |
|
The only one that seems related to the lint is triggering at and it looks legit. Separately, I am slightly concerned about the ICEs at https://crater-reports.s3.amazonaws.com/pr-134218/try%236db1a6a7ebb2ad243051f3ff949f987dd70cde2b%2Brustflags=-Dreturn-local-variable-ptr/gh/fmhall.chess-program/log.txt which seem to point at an issue elsewhere that was recently introduced. |
|
r? estebank |
|
☔ The latest upstream changes (presumably #136024) made this pull request unmergeable. Please resolve the merge conflicts. |
352f384 to
eb706f9
Compare
|
Hey all, friendly ping on this PR, do you have overall feedback on this change :) I made a small change to the lint to also include nested pointers of arbitrary depth. This was funnily enough discovered by having some production code I saw that ran into this issue, which is I think a good reason to have a lint like that? |
This comment has been minimized.
This comment has been minimized.
d56da6f to
5576c0a
Compare
|
☔ The latest upstream changes (presumably #136332) made this pull request unmergeable. Please resolve the merge conflicts. |
This adds a new lint with level `Warn` to check for code like:
```rust
fn foo() -> *const i32 {
let x = 42;
&x
}
```
and produce a warning like:
```text
error: returning a pointer to stack memory associated with a local variable
--> <source>:12:5
|
LL| &x
| ^^
```
5576c0a to
e38f2c8
Compare
|
Friendly ping on this one :) |
|
Friendly ping on this again :) It would be great to get some feedback! |
|
Friendly ping again! |
|
☔ The latest upstream changes (presumably #138058) made this pull request unmergeable. Please resolve the merge conflicts. |
This adds a new lint with level
Warnto check for code like:and produce a warning like:
This fixes #134215.