Skip to content

Fix #12438 std_instead_of_core regression #12447

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
Mar 9, 2024

Conversation

MarcusGrass
Copy link
Contributor

Fixes #12438.

Boy-scouting removed two paths that checks for duplication since I thought they were unused. However, that's just because I didn't spot it in the diff.

I installed difftastic and ran it on the old one:

image

And the new one (fixed):

image

New one (stderr):
image

Good teachings for the future when inspecting diffs with a lot of line changes, should've thought of that before, sorry for the trouble!

changelog: [std_instead_of_core] Fix false positive for crates that are in std but not core

@rustbot
Copy link
Collaborator

rustbot commented Mar 9, 2024

r? @y21

rustbot has assigned @y21.
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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 9, 2024
@y21
Copy link
Member

y21 commented Mar 9, 2024

Is the order in which check_path is called for different resolutions in a use guaranteed? Otherwise it seems a bit fragile, since I think this would break if check_path is called with the macro resolution first, because it hasn't seen the mod res at that point.
Though fixing that likely requires reworking the lint a bit to be able to buffer the lint emissions until all parts of a use statement are checked.

Either way, that's how it was before (for a long time) and it seems this hasn't been an issue, so this LGTM. Thank you for fixing this!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 9, 2024

📌 Commit b44ab66 has been approved by y21

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 9, 2024

⌛ Testing commit b44ab66 with merge 7ee75f8...

@bors
Copy link
Contributor

bors commented Mar 9, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: y21
Pushing 7ee75f8 to master...

@bors bors merged commit 7ee75f8 into rust-lang:master Mar 9, 2024
@MarcusGrass
Copy link
Contributor Author

Is the order in which check_path is called for different resolutions in a use guaranteed? Otherwise it seems a bit fragile, since I think this would break if check_path is called with the macro resolution first, because it hasn't seen the mod res at that point. Though fixing that likely requires reworking the lint a bit to be able to buffer the lint emissions until all parts of a use statement are checked.

Either way, that's how it was before (for a long time) and it seems this hasn't been an issue, so this LGTM. Thank you for fixing this!

@bors r+

Yeah it is a bit problematic, added an issue for it here #12468

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std_instead_of_core: false positive with std::env since nightly-2024-03-08
4 participants