Run borrowck tests on BIDs and emit tail-expr-drop-order lints for violations #134523

dingxiangfei2009 · 2024-12-19T16:46:58Z

This patch enlarges the scope where the tail-expr-drop-order lint applies, so that all locals involved in tail expressions are inspected. This is necessary to run borrow-checking to capture the cases where it used to compile under Edition 2021 but is not going to pass borrow-checking from Edition 2024 onwards.

The way it works is to inspect each BID against the set of borrows that are still live. If the local involved in BID has a borrow index which happens to be live as well at the location of this BID statement, in the future this will be a borrow-checking violation. The lint will fire in this case.

compiler-errors · 2024-12-19T16:50:37Z

compiler/rustc_borrowck/src/lib.rs

@@ -994,6 +997,23 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
        }
    }

+    fn maybe_polonius_borrows_in_scope<'s>(


why does this mention polonius at all? why isn't this just called borrows_in_scope?

Yes, let's.

I am tucking the comment into this function, too.

compiler/rustc_borrowck/src/lib.rs

dingxiangfei2009 · 2024-12-19T21:29:42Z

@bors try

bors · 2024-12-19T21:30:53Z

⌛ Trying commit 0a77622 with merge 9b5e8b1...

…t-2, r=<try> Run borrowck tests on BIDs and emit tail-expr-drop-order lints for violations Fix rust-lang#132861 r? `@nikomatsakis` cc `@compiler-errors` This patch enlarges the scope where the `tail-expr-drop-order` lint applies, so that all locals involved in tail expressions are inspected. This is necessary to run borrow-checking to capture the cases where it used to compile under Edition 2021 but is not going to pass borrow-checking from Edition 2024 onwards. The way it works is to inspect each BID against the set of borrows that are still live. If the local involved in BID has a borrow index which happens to be live as well at the location of this BID statement, in the future this will be a borrow-checking violation. The lint will fire in this case.

dingxiangfei2009 · 2024-12-19T21:45:21Z

I have prepared the following craterbot message.

@craterbot run start=master#11663cd3bfefef7d34e8f0892c250bf698049392 end=try#9b5e8b16bd1c4acf4cf8e6880368314cf021a987 mode=build-only +rustflags=-Dtail-expr-drop-order

I would like to check the difference in lint effects with -Dtail-expr-drop-order before and after, to see if there is any false positives and negatives.
build-only is required, because Always run tail_expr_drop_order lint in promoted MIR query #134493 is still not on master yet. Otherwise, we could use check-only for the experiment.

Shall we double-check and give this a go afterwards? Big thanks!

craterbot · 2024-12-19T21:45:23Z

🔒 Error: you're not allowed to interact with this bot.

🔑 If you are a member of the Rust team and need access, add yourself to the whitelist.
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

…-run, r=<try> Crater run for `tail-expr-drop-order` This is experiment for rust-lang#134523

bors · 2024-12-19T23:14:09Z

☀️ Try build successful - checks-actions
Build commit: 9b5e8b1 (9b5e8b16bd1c4acf4cf8e6880368314cf021a987)

lqd · 2024-12-20T08:13:53Z

A perf run shouldn't show anything since this shouldn't be enabled by default. However, some of these previous PRs have been unexpected regressions, let's check.

@rust-timer build 9b5e8b1

rust-timer · 2024-12-20T09:58:10Z

Finished benchmarking commit (9b5e8b1): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -0.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.8%	[-0.8%, -0.8%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.8%	[-0.8%, -0.8%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 769.451s -> 768.407s (-0.14%)
Artifact size: 330.39 MiB -> 330.40 MiB (0.00%)

compiler-errors · 2024-12-31T19:02:45Z

@craterbot run start=master#11663cd3bfefef7d34e8f0892c250bf698049392+rustflags=-Dtail-expr-drop-order end=try#9b5e8b16bd1c4acf4cf8e6880368314cf021a987+rustflags=-Dtail-expr-drop-order mode=build-only

craterbot · 2024-12-31T19:02:56Z

👌 Experiment pr-134523 created and queued.
🔍 You can check out the queue and this experiment's details.

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

craterbot · 2024-12-31T19:03:03Z

🚧 Experiment pr-134523 is now running

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

craterbot · 2025-01-01T12:46:44Z

🎉 Experiment pr-134523 is completed!
📊 35438 regressed and 36 fixed (561988 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Skgland · 2025-01-01T16:27:48Z

I ran https://github.com/Skgland/Crater-Analysis to classify the build failed (unknown) regressions.

31091 contain error: internal compiler error: so are likly ICEs, from a small sample these are all in dependencies of the tested crate so likely not classified as a deps breakage due to the problem I am trying to fix in rust-lang/crater#758 .

85 failures appear to be due to running out of disk space,
either containing no space left on device or in one instance collect2: fatal error: ld terminated with signal 7 [Bus error]

Full report

Regressed: 35438
build failed(unknown): 31257
----------------------------------
Results:
E0277: 3
E0308: 3
E0422: 1
E0463: 1
E0557: 1
E0635: 28
compile_error!: 1
delimiter missmatch: 2
ice: 31091
include_bytes-missing-file: 7
include_str-missing-file: 11
linker-bus-error: 1
linker-missing-library: 18
linker-undefined-symbol: 7
missing-env-var: 1
no-space: 84
----------------------------------
sum: 31260
others: 16
----------------------------------
[
    (
        "Flying-Toast.lang.9586e323c23abf9a57dc074d09470eac07f59259",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/Flying-Toast.lang",
    ),
    (
        "JanBerktold.jute-rust.0a9a534599efa22a7c317c1b9963ac2438ac028b",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/JanBerktold.jute-rust",
    ),
    (
        "KavinduShamalka.rust_addnames.a7aeddf247ddc1e9cc4c357a8d229bd3ea69de59",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/KavinduShamalka.rust_addnames",
    ),
    (
        "Wopple.bookcase-rs.ce12f79a71946ea65b637b34773af863f1c034fd",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/Wopple.bookcase-rs",
    ),
    (
        "hardglitch.xmath.bda74b1d575c7c65e13558c361f26947597295aa",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/hardglitch.xmath",
    ),
    (
        "irreducible-io.term-rewriting.546119b6ecbd8ef8c0270bc7893e28d3286384e1",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/irreducible-io.term-rewriting",
    ),
    (
        "kirisaki.jit-compiler.5ceb89cb2a39b0f5cfe69ae1cb7944580f41eb89",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/kirisaki.jit-compiler",
    ),
    (
        "meddion.fugue.9ad7c4b881efcb66a6edde1681a01aadc4a12142",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/meddion.fugue",
    ),
    (
        "mii443.breakout-checker.62e4dabd9f7940cbf0bed01793284cf1bfe20b75",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/mii443.breakout-checker",
    ),
    (
        "raviqqe.stak.d6fa9052b0eea9e9276da15a1ac60e06b251ce1e",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/raviqqe.stak",
    ),
    (
        "scupit.gcmake-rust.50476479f7776605d30e7f1a40026c3a13e85d7b",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/scupit.gcmake-rust",
    ),
    (
        "ssav7912.rust-FreeD.6f1bbd038d6746ae06276034375dcba78453bef3",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/gh/ssav7912.rust-FreeD",
    ),
    (
        "bookcase_alloc-0.0.2",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/reg/bookcase_alloc-0.0.2",
    ),
    (
        "crypto-brainfuck-0.2.0",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/reg/crypto-brainfuck-0.2.0",
    ),
    (
        "protobuf3-2.27.2",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/reg/protobuf3-2.27.2",
    ),
    (
        "vec-with-gaps-0.7.0",
        "master%2311663cd3bfefef7d34e8f0892c250bf698049392%2Brustflags=-Dtail-expr-drop-order/reg/vec-with-gaps-0.7.0",
    ),
]

compiler-errors · 2025-01-04T02:19:32Z

@bors try

Need to re-run crater after #134575 was merged

potential violations

Co-authored-by: Rémy Rakic <[email protected]>

…checking

compiler-errors · 2025-01-08T16:11:04Z

@bors r=nikomatsakis

bors · 2025-01-08T16:11:07Z

📌 Commit c55eefe has been approved by nikomatsakis

It is now in the queue for this repository.

bors · 2025-01-08T16:22:24Z

⌛ Testing commit c55eefe with merge a580b5c...

bors · 2025-01-08T19:52:34Z

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing a580b5c to master...

rust-timer · 2025-01-08T21:20:14Z

Finished benchmarking commit (a580b5c): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.4%	[0.3%, 0.6%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -3.1%, secondary -2.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.1%	[-3.1%, -3.1%]	1
Improvements ✅ (secondary)	-2.7%	[-2.7%, -2.7%]	1
All ❌✅ (primary)	-3.1%	[-3.1%, -3.1%]	1

Cycles

Results (primary -2.2%, secondary -2.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.2%	[-3.6%, -0.8%]	11
Improvements ✅ (secondary)	-2.8%	[-3.7%, -2.0%]	4
All ❌✅ (primary)	-2.2%	[-3.6%, -0.8%]	11

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 763.722s -> 763.382s (-0.04%)
Artifact size: 325.68 MiB -> 325.74 MiB (0.02%)

apiraino · 2025-01-09T10:59:32Z

compiler/rustc_borrowck/src/lib.rs

@@ -1149,6 +1161,61 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
        error_reported
    }

+    /// Through #123739, backward incompatible drops (BIDs) are introduced.


@dingxiangfei2009 A tiny late nit. I have just learned from this comment what BID stand for. Is it a commonly used acronym? I wasn't able to find it using a search engine. Could only find it disentangled here.

(thanks @lqd for the pointing me here)

dingxiangfei2009 · 2025-01-09T13:01:24Z

Hey @apiraino. BID is a shorthand that is unfortunately not well documented yet. It refers to backward-incompatible drop statements generated exclusively when edition migration is run. It is defined in https://github.com/rust-lang/rust/blob/b6b8361bce8561fb8786ad33ca1abfdf4bc487b6/compiler/rustc_middle/src/mir/syntax.rs#L440. It does not have any semantic significance other than being a hint that lints can rely on and can be treated safely as a NOP. apiraino ***@***.***> schrieb am Do. 9. Jan. 2025 um 11:59:

…

***@***.**** commented on this pull request. ------------------------------ In compiler/rustc_borrowck/src/lib.rs <#134523 (comment)>: > @@ -1149,6 +1161,61 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { error_reported } + /// Through #123739, backward incompatible drops (BIDs) are introduced. @dingxiangfei2009 <https://github.com/dingxiangfei2009> A tiny late nit. I have just learned from this comment what BID stand for. Is it a commonly used acronym? I wasn't able to find it using a search engine. Could only find it disentangled here. (thanks @lqd <https://github.com/lqd> for the pointing me here) — Reply to this email directly, view it on GitHub <#134523 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABUQYWEBU5TDZMHSSFVD4ED2JZJCXAVCNFSM6AAAAABT5MVGIKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKMZZGYYDAMZQGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

lqd · 2025-01-09T13:56:52Z

note for t-release if the backport is approved: the backport will likely not apply cleanly due to landing after #133858. I hit similar conflicts when doing a revert, so the conflicting pieces will be present and fixed in #135263. (But if there are issues, dingxiangfei/errs/I should be able to prepare it for you.)

apiraino · 2025-01-09T15:18:50Z

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Backport labels handled by them.

@rustbot label +beta-accepted

[beta] stage0 bump and backports - bump stage0 to 1.84.0 - Run borrowck tests on BIDs and emit tail-expr-drop-order lints for violations rust-lang#134523 r? cuviper

rustbot assigned nikomatsakis Dec 19, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 19, 2024

dingxiangfei2009 force-pushed the issue-130836-attempt-2 branch from beada15 to 054769b Compare December 19, 2024 16:50

compiler-errors reviewed Dec 19, 2024

View reviewed changes

lqd reviewed Dec 19, 2024

View reviewed changes

compiler/rustc_borrowck/src/lib.rs Outdated Show resolved Hide resolved

nikomatsakis approved these changes Dec 19, 2024

View reviewed changes

dingxiangfei2009 mentioned this pull request Dec 19, 2024

Crater run for tail-expr-drop-order #134537

Draft

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 19, 2024

Auto merge of rust-lang#134537 - dingxiangfei2009:issue-132861-crater…

5b4a9ad

…-run, r=<try> Crater run for `tail-expr-drop-order` This is experiment for rust-lang#134523

traviscross added the L-tail_expr_drop_order Lint: tail_expr_drop_order label Dec 19, 2024

This comment has been minimized.

Sign in to view

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 31, 2024

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jan 1, 2025

compiler-errors force-pushed the issue-130836-attempt-2 branch from 0a77622 to 2daeb51 Compare January 4, 2025 02:16

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 8, 2025

dingxiangfei2009 and others added 6 commits January 8, 2025 15:58

run borrowck tests on BIDs and emit tail-expr-drop-order lints for

045271c

potential violations

apply suggestions on fn name

34edb21

remove an extraneous comment

13c7122

Co-authored-by: Rémy Rakic <[email protected]>

Don't create cycles by normalizing opaques defined in the body we're …

197f6d8

…checking

Don't do AccessDepth::Drop for types with no drop impl

4a099b2

Try to explain borrow for tail expr temporary drop order change in 2024

c55eefe

compiler-errors force-pushed the issue-130836-attempt-2 branch from 1c69947 to c55eefe Compare January 8, 2025 16:03

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 8, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 8, 2025

bors merged commit a580b5c into rust-lang:master Jan 8, 2025
7 checks passed

rustbot added this to the 1.86.0 milestone Jan 8, 2025

bors mentioned this pull request Jan 8, 2025

Rename BitSet to DenseBitSet #135205

Merged

lqd mentioned this pull request Jan 8, 2025

Revert 133858 due to perf regression #135263

Closed

apiraino reviewed Jan 9, 2025

View reviewed changes

rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jan 9, 2025

cuviper mentioned this pull request Jan 11, 2025

[beta] stage0 bump and backports #135351

Merged

cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 11, 2025

cuviper modified the milestones: 1.86.0, 1.85.0 Jan 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run borrowck tests on BIDs and emit tail-expr-drop-order lints for violations #134523

Run borrowck tests on BIDs and emit tail-expr-drop-order lints for violations #134523

dingxiangfei2009 commented Dec 19, 2024

compiler-errors Dec 19, 2024

dingxiangfei2009 Dec 19, 2024

dingxiangfei2009 commented Dec 19, 2024

bors commented Dec 19, 2024

dingxiangfei2009 commented Dec 19, 2024

craterbot commented Dec 19, 2024

bors commented Dec 19, 2024

lqd commented Dec 20, 2024

This comment has been minimized.

rust-timer commented Dec 20, 2024

compiler-errors commented Dec 31, 2024

craterbot commented Dec 31, 2024

craterbot commented Dec 31, 2024

craterbot commented Jan 1, 2025

Skgland commented Jan 1, 2025 •

edited

Loading

compiler-errors commented Jan 4, 2025

compiler-errors commented Jan 8, 2025

bors commented Jan 8, 2025

bors commented Jan 8, 2025

bors commented Jan 8, 2025

rust-timer commented Jan 8, 2025

apiraino Jan 9, 2025

dingxiangfei2009 commented Jan 9, 2025 via email

lqd commented Jan 9, 2025

apiraino commented Jan 9, 2025

Run borrowck tests on BIDs and emit tail-expr-drop-order lints for violations #134523

Run borrowck tests on BIDs and emit tail-expr-drop-order lints for violations #134523

Conversation

dingxiangfei2009 commented Dec 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dingxiangfei2009 commented Dec 19, 2024

bors commented Dec 19, 2024

dingxiangfei2009 commented Dec 19, 2024

craterbot commented Dec 19, 2024

bors commented Dec 19, 2024

lqd commented Dec 20, 2024

This comment has been minimized.

rust-timer commented Dec 20, 2024

Overall result: ❌ regressions - no action needed

compiler-errors commented Dec 31, 2024

craterbot commented Dec 31, 2024

craterbot commented Dec 31, 2024

craterbot commented Jan 1, 2025

Skgland commented Jan 1, 2025 • edited Loading

compiler-errors commented Jan 4, 2025

compiler-errors commented Jan 8, 2025

bors commented Jan 8, 2025

bors commented Jan 8, 2025

bors commented Jan 8, 2025

rust-timer commented Jan 8, 2025

Overall result: ❌ regressions - no action needed

Choose a reason for hiding this comment

dingxiangfei2009 commented Jan 9, 2025 via email

lqd commented Jan 9, 2025

apiraino commented Jan 9, 2025

Skgland commented Jan 1, 2025 •

edited

Loading