-
Notifications
You must be signed in to change notification settings - Fork 13k
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
[DO NOT MERGE] crater run on the tail expression drop order lint #129604
base: master
Are you sure you want to change the base?
[DO NOT MERGE] crater run on the tail expression drop order lint #129604
Conversation
The job Click to see the possible cause of the failure (guessed by this bot)
|
r? jieyouxu |
The tests may have to be temporarily ignored since you're changing lint's behavior. |
@jieyouxu Let's put this on ice briefly. @traviscross and I have come up with a plan to enable both the lint and the feature gate over in #129607. This will help us narrow down to a smaller set of crates to test for the lint and/or the feature separately. |
That seems fine with me. Feel free to ping me if you are happy with a set of changes that you would like a crater run on. |
@jieyouxu @traviscross I need your advice here. Given that simulating the shorter tail expression temporary lifetime on Edition 2021 will stop If this makes sense, I would propose to do checking only with WDYT? |
Sounds OK . You might want to see if this can be included in the crater rollup which is scheduled to run next: |
@bors try |
…-order-crater-run, r=<try> [DO NOT MERGE] crater run on the tail expression drop order lint This PR is intended for a crater run *for the lint tail-expr-drop-order*. This need another patch on `src/tools/cargo` which is still in progress.
[CRATER] Crater Rollup This is a " crater rollup" of: * rust-lang#126452 * rust-lang#128784 * rust-lang#129392 * rust-lang#129422 * rust-lang#129543 * rust-lang#129604 **What is a crater rollup?** It's simply a crater job that is run on all of the containing PRs *together*, and then we can set the crates list for each of these jobs to just the failures after it's done. It should cut out on the bulk of "normal" crates that do nothing and simply just take time to build. r? `@ghost`
[CRATER] Crater Rollup This is a " crater rollup" of: * rust-lang#126452 * rust-lang#128784 * rust-lang#129392 * rust-lang#129422 * rust-lang#129543 * rust-lang#129604 **What is a crater rollup?** It's simply a crater job that is run on all of the containing PRs *together*, and then we can set the crates list for each of these jobs to just the failures after it's done. It should cut out on the bulk of "normal" crates that do nothing and simply just take time to build. r? `@ghost`
☀️ Try build successful - checks-actions |
🚨 Error: experiment 'pr-129604' not found 🆘 If you have any trouble with Crater please ping |
oops! crater never was started on this one anyways. @craterbot mode=check-only start=master#ab869e094a907cc5d19b4080f22eccaf347f1f95 end=try#4d98531622e1718ae0bc3c7a1ab9cd8938428452+rustflags=-Dtail_expr_drop_order crates=https://gist.githubusercontent.com/compiler-errors/4a09d64cd15dc3dca50edeea26cc9938/raw/b4181c225709e120a11d91cce69d0d4da3e652d0/regressed.txt p=1 |
🚨 Error: experiment 'pr-129604' not found 🆘 If you have any trouble with Crater please ping |
@craterbot name=pr-129604-1 mode=check-only start=master#ab869e094a907cc5d19b4080f22eccaf347f1f95 end=try#4d98531622e1718ae0bc3c7a1ab9cd8938428452+rustflags=-Dtail_expr_drop_order crates=https://gist.githubusercontent.com/compiler-errors/4a09d64cd15dc3dca50edeea26cc9938/raw/b4181c225709e120a11d91cce69d0d4da3e652d0/regressed.txt p=1 |
🚨 Error: experiment 'pr-129604-1' not found 🆘 If you have any trouble with Crater please ping |
oops i needed a @craterbot run mode=check-only start=master#ab869e094a907cc5d19b4080f22eccaf347f1f95 end=try#4d98531622e1718ae0bc3c7a1ab9cd8938428452+rustflags=-Dtail_expr_drop_order crates=https://gist.githubusercontent.com/compiler-errors/4a09d64cd15dc3dca50edeea26cc9938/raw/b4181c225709e120a11d91cce69d0d4da3e652d0/regressed.txt p=1 |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
@jieyouxu @traviscross The lint seems to be working fine from the report, but as I have feared it has a lot of false positives. For instance, the lint fires when a value with significant drop is moved/consumed at a tail expression, even though in fact it will not be dropped. struct Droppy;
impl Droppy {
fn consume(self) -> u8 {
0 // dropping `self` here
}
}
impl Drop for Droppy { .. }
fn droppy_is_consumed() -> u8 {
let droppy = Droppy;
droppy.consume()
} There is a solution, which is to use The question now is, should we leave the lint as EDIT: I checked the |
Another solution is to take a subset of visitor rules from |
@dingxiangfei2009 I'm not super sure about this in terms of what the correct approach is. I would suggest that we discuss this in a new #t-compiler/help zulip thread. |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🎉 Experiment
|
This PR is intended for a crater run for the lint tail-expr-drop-order.
This need another patch on
src/tools/cargo
which is still in progress.