[DO NOT MERGE] crater run on the tail expression drop order lint

[dingxiangfei2009]

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.

[rustbot]

r? @fmease

rustbot has assigned @fmease. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rust-log-analyzer]

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

------
 > importing cache manifest from ghcr.io/rust-lang/rust-ci-cache:20d3b4d4a2629cbf7865cdbf92fe47512a7c96658c24253a045ff38e8075cd7fb37ca6fcadfa6e6d093333943ad24f6fc4f163ec5b74fd940de9d5bb03eb4d3b:
------
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
---- [ui] tests/ui/drop/lint-tail-expr-drop-order-gated.rs#neither stdout ----

error in revision `neither`: test compilation failed although it shouldn't!
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/drop/lint-tail-expr-drop-order-gated.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--cfg" "neither" "--check-cfg" "cfg(FALSE,neither,no_feature_gate,edition_less_than_2024)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/drop/lint-tail-expr-drop-order-gated.neither" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/drop/lint-tail-expr-drop-order-gated.neither/auxiliary" "--edition=2021"
--- stderr -------------------------------
--- stderr -------------------------------
error: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
   |
LL |     let x = LoudDropper;
LL |     let x = LoudDropper;
   |         - these values have significant drop implementation and will observe changes in drop order under Edition 2024
LL |     x.get() + LoudDropper.get()
   |
   = warning: this changes meaning in Rust 2024
   = note: for more information, see issue #123739 <https://github.com/rust-lang/rust/issues/123739>
note: the lint level is defined here
---


---- [ui] tests/ui/drop/lint-tail-expr-drop-order-gated.rs#edition_less_than_2024 stdout ----

error in revision `edition_less_than_2024`: test compilation failed although it shouldn't!
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/drop/lint-tail-expr-drop-order-gated.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--cfg" "edition_less_than_2024" "--check-cfg" "cfg(FALSE,neither,no_feature_gate,edition_less_than_2024)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/drop/lint-tail-expr-drop-order-gated.edition_less_than_2024" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/drop/lint-tail-expr-drop-order-gated.edition_less_than_2024/auxiliary" "--edition=2021"
--- stderr -------------------------------
--- stderr -------------------------------
error: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
   |
LL |     let x = LoudDropper;
LL |     let x = LoudDropper;
   |         - these values have significant drop implementation and will observe changes in drop order under Edition 2024
LL |     x.get() + LoudDropper.get()
   |
   = warning: this changes meaning in Rust 2024
   = note: for more information, see issue #123739 <https://github.com/rust-lang/rust/issues/123739>
note: the lint level is defined here
---
---- [ui] tests/ui/drop/lint-tail-expr-drop-order-gated.rs#no_feature_gate stdout ----

error in revision `no_feature_gate`: test compilation failed although it shouldn't!
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/drop/lint-tail-expr-drop-order-gated.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--cfg" "no_feature_gate" "--check-cfg" "cfg(FALSE,neither,no_feature_gate,edition_less_than_2024)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/drop/lint-tail-expr-drop-order-gated.no_feature_gate" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/drop/lint-tail-expr-drop-order-gated.no_feature_gate/auxiliary" "-Z" "unstable-options" "--edition=2024"
--- stderr -------------------------------
--- stderr -------------------------------
error: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
   |
LL |     let x = LoudDropper;
LL |     let x = LoudDropper;
   |         - these values have significant drop implementation and will observe changes in drop order under Edition 2024
LL |     x.get() + LoudDropper.get()
   |
   = warning: this changes meaning in Rust 2024
   = note: for more information, see issue #123739 <https://github.com/rust-lang/rust/issues/123739>
note: the lint level is defined here

[jieyouxu]

r? jieyouxu

[jieyouxu]

The tests may have to be temporarily ignored since you're changing lint's behavior.

[dingxiangfei2009]

@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.

[jieyouxu]

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.

[dingxiangfei2009]

@jieyouxu @traviscross I need your advice here. Given that simulating the shorter tail expression temporary lifetime on Edition 2021 will stop rustc from bootstrapping itself beyond stage1, shall we move ahead and test the lint first? Just to get a feel of its verbosity and accuracy.

If this makes sense, I would propose to do checking only with +rustflags=-Dtail-expr-drop-order. We don't need to patch cargo in this PR since we are only running cargo check.

WDYT?

[traviscross]

Sounds OK . You might want to see if this can be included in the crater rollup which is scheduled to run next:

• https://github.com/rust-lang/rust/pull/129660

[compiler-errors]

@bors try

[bors]

:hourglass: Trying commit e694863a5d4214e06c5f7692d9859e699af482d5 with merge 4d98531622e1718ae0bc3c7a1ab9cd8938428452...

[bors]

:sunny: Try build successful - checks-actions Build commit: 4d98531622e1718ae0bc3c7a1ab9cd8938428452 (4d98531622e1718ae0bc3c7a1ab9cd8938428452)

[compiler-errors]

@craterbot crates=https://gist.githubusercontent.com/compiler-errors/4a09d64cd15dc3dca50edeea26cc9938/raw/b4181c225709e120a11d91cce69d0d4da3e652d0/regressed.txt

[craterbot]

:rotating_light: Error: experiment 'pr-129604' not found

:sos: If you have any trouble with Crater please ping @rust-lang/infra! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[compiler-errors]

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

[craterbot]

:rotating_light: Error: experiment 'pr-129604' not found

:sos: If you have any trouble with Crater please ping @rust-lang/infra! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[compiler-errors]

@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

[craterbot]

:rotating_light: Error: experiment 'pr-129604-1' not found

:sos: If you have any trouble with Crater please ping @rust-lang/infra! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[compiler-errors]

oops i needed a run

@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

[craterbot]

:ok_hand: Experiment pr-129604 created and queued. :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[dingxiangfei2009]

@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 rustc_hir_typeck::expr_use_visitor::ExprUseVisitor to mark nodes that are consumed to be ignored. The problem is, rust_hir_typeck crate already depends on rustc_lint. :sweat:

The question now is, should we leave the lint as Allow as it is now, or should we continue to polish it? If we are to polish it, for which I would recommend ExprUseVisitor aka. EUV, can we build a case for moving it into rustc_middle or somewhere else?

EDIT: I checked the ExprUseVisitor out. It depends on trait selection and FnCtxt which makes it a bit of challenge to move it.

[dingxiangfei2009]

Another solution is to take a subset of visitor rules from ExprUseVisitor, tracking only a limit set of cases where consume happens. Also to avoid future divergence from the match ergonomics work, I will not consider consume cases in the match arms. This would allow us to prune away sufficient false positives. What would y'all think about this?

[jieyouxu]

@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.

[craterbot]

:construction: Experiment pr-129604 is now running

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:tada: Experiment pr-129604 is completed! :bar_chart: 70450 regressed and 0 fixed (98236 total) :newspaper: Open the full report.

:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[dingxiangfei2009]

Our job here is done. Thank you all.