Skip to content

Suggest adding semicolon in user code rather than macro impl details #142543

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

Merged
merged 1 commit into from
Jun 16, 2025

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Jun 15, 2025

This PR tries to find the right span (by peeling expansion) so that the suggestion for adding a semicolon is suggested in user code rather than in the expanded code (in the example a macro impl).

Fixes #139049
r? @fmease

@rustbot
Copy link
Collaborator

rustbot commented Jun 15, 2025

fmease is not on the review rotation at the moment.
They may take a while to respond.

@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 Jun 15, 2025
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Thanks! r=me with two nits addressed in one way or another

@fmease fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 15, 2025
@fmease

This comment was marked as outdated.

@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the span-borrowck-semicolon branch from ac875a1 to 6ff3713 Compare June 15, 2025 18:07
@Urgau Urgau added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 15, 2025
@Urgau
Copy link
Member Author

Urgau commented Jun 15, 2025

@bors r=fmease

@bors
Copy link
Collaborator

bors commented Jun 15, 2025

📌 Commit 6ff3713 has been approved by fmease

It is now in the queue for this repository.

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 15, 2025
bors added a commit that referenced this pull request Jun 16, 2025
Rollup of 10 pull requests

Successful merges:

 - #133952 (Remove wasm legacy abi)
 - #134661 (Reduce precedence of expressions that have an outer attr)
 - #141769 (Move metadata object generation for dylibs to the linker code )
 - #141937 (Report never type lints in dependencies)
 - #142347 (Async drop - fix for StorageLive/StorageDead codegen for pinned future)
 - #142389 (Apply ABI attributes on return types in `rustc_codegen_cranelift`)
 - #142470 (Add some missing mailmap entries)
 - #142481 (Add `f16` inline asm support for LoongArch)
 - #142499 (Remove check run bootstrap)
 - #142543 (Suggest adding semicolon in user code rather than macro impl details)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0704864 into rust-lang:master Jun 16, 2025
10 checks passed
rust-timer added a commit that referenced this pull request Jun 16, 2025
Rollup merge of #142543 - Urgau:span-borrowck-semicolon, r=fmease

Suggest adding semicolon in user code rather than macro impl details

This PR tries to find the right span (by peeling expansion) so that the suggestion for adding a semicolon is suggested in user code rather than in the expanded code (in the example a macro impl).

Fixes #139049
r? `@fmease`
@rustbot rustbot added this to the 1.89.0 milestone Jun 16, 2025
Comment on lines +37 to +42
let _write = {
let mutex = Mutex;
write!(Out, "{}", mutex.lock())
//~^ ERROR `mutex` does not live long enough
//~| SUGGESTION ;
};
Copy link
Member

@m-ou-se m-ou-se Jun 16, 2025

Choose a reason for hiding this comment

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

@Urgau #140748 makes this just work, so this new test blocks me from merging that PR. Should I remove it? Is there a different way to test what you want to test?

No wait, the error still appears. Only the suggestion doesn't appear. Investigating..

Copy link
Member

@m-ou-se m-ou-se Jun 16, 2025

Choose a reason for hiding this comment

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

I just noticed that this test passes in Rust 2024, due to the new rules for temporaries in tail expressions.

Copy link
Member

@fmease fmease Jun 16, 2025

Choose a reason for hiding this comment

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

I just noticed that this test passes in Rust 2024

Sure, but we still want to provide non-butchered / good diagnostics in older editions, they have "long-term support".

Copy link
Member

@fmease fmease Jun 16, 2025

Choose a reason for hiding this comment

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

We could replace that test with the following one:

// Make sure the generated suggestion suggest editing the user code instead of
// the macro implementation (which might come from an external crate).
// issue: <https://github.com/rust-lang/rust/issues/139049>

// You could assume that this comes from an extern crate (it doesn't
// because an aux crate would be overkill for this test).
macro_rules! perform { ($e:expr) => { D(&$e).end() } }

fn main() {
    { let l = (); perform!(l) }; //~ ERROR does not live long enough
    //~^ SUGGESTION ;
    let _x = { let l = (); perform!(l) }; //~ ERROR does not live long enough
    //~| SUGGESTION let x
}

struct D<T>(T);
impl<T> Drop for D<T> { fn drop(&mut self) {} }
impl<T> D<T> { fn end(&self) -> String { String::new()} }

Copy link
Member

Choose a reason for hiding this comment

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

Or modify tests/ui/nll/issue-54556-used-vs-unused-tails.rs and add it there

Copy link
Member

Choose a reason for hiding this comment

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

@fmease That test doesn't work. Then it just suggests to add the ; inside the macro definition.

jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 17, 2025
Handle same-crate macro for borrowck semicolon suggestion

Handles rust-lang#142543 (comment)

cc `@m-ou-se`
r? `@fmease`
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 17, 2025
Handle same-crate macro for borrowck semicolon suggestion

Handles rust-lang#142543 (comment)

cc ``@m-ou-se``
r? ``@fmease``
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 17, 2025
Handle same-crate macro for borrowck semicolon suggestion

Handles rust-lang#142543 (comment)

cc ```@m-ou-se```
r? ```@fmease```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Misleading compiler suggestion: Add semicolon in rustlib due to missing ; in user code
6 participants