Skip to content
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

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dingxiangfei2009
Copy link
Contributor

Fix #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.

@rustbot 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
@@ -994,6 +997,23 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
}
}

fn maybe_polonius_borrows_in_scope<'s>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's.

I am tucking the comment into this function, too.

@dingxiangfei2009
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Dec 19, 2024

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

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 19, 2024
…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
Copy link
Contributor Author

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
Copy link
Collaborator

🔒 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

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 19, 2024
…-run, r=<try>

Crater run for `tail-expr-drop-order`

This is experiment for rust-lang#134523
@traviscross traviscross added the L-tail_expr_drop_order Lint: tail_expr_drop_order label Dec 19, 2024
@bors
Copy link
Contributor

bors commented Dec 19, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L-tail_expr_drop_order Lint: tail_expr_drop_order 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible false-negative with tail-expr-drop-order
8 participants