On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items #117009

fmease · 2023-10-21T04:26:16Z

rustbot · 2023-10-21T04:26:24Z

(rustbot has picked a reviewer for you, use r? to override)

fmease · 2023-10-21T04:27:16Z

compiler/rustc_resolve/src/diagnostics.rs

-            // We have to visit module children in deterministic order to avoid
-            // instabilities in reported imports (#43552).


This comment was added in #43552 but has been outdated since #65043.

fmease · 2023-10-21T04:28:57Z

tests/ui/imports/issue-56125.stderr

The new suggestion is still correct technically speaking. I don't think it's so much a regression as an obscure test case that can't be helped ^^'.

tests/ui/unresolved/unresolved-import-avoid-suggesting-global-path.rs

fmease · 2023-10-21T12:30:07Z

pro tip: be well rested when working on a PR to get the title and comment texts right the first time ^^

b-naber

r=me with nit addressed.

b-naber · 2023-10-25T17:10:20Z

compiler/rustc_resolve/src/diagnostics.rs

                            // crate-local absolute paths start with `crate::` in edition 2018
                            // FIXME: may also be stabilized for Rust 2015 (Issues #45477, #44660)
-                            segms.insert(0, ast::PathSegment::from_ident(crate_name));
-                        }
+                            crate_path.clone()


We should be able to move this here.

You mean the whole let needs_disambiguation = self.resolutions(…).iter().any(…); … code or just the let crate_path = …; … one? If self.resolutions(…) was moved here, it would get executed several times: For each work item (while let Some(…) = worklist.pop()) and for_each_child in_module while right now it gets executed only once.

E.g., it would get executed twice if library.rs was:

pub struct SomeUsefulType; pub mod inner { pub enum SomeUsefulType {} }

🤦 sorry, should have looked more carefully.

b-naber · 2023-10-25T17:56:42Z

@bors r+ rollup

bors · 2023-10-25T17:56:45Z

📌 Commit 4aaf8e0 has been approved by b-naber

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#116401 (Return multiple object-safety violation errors and code improvements to the object-safety check) - rust-lang#116553 (Do not suggest 'Trait<Assoc=arg>' when in trait impl) - rust-lang#116931 (Improve the warning messages for the `#[diagnostic::on_unimplemented]`) - rust-lang#117008 (Uplift `Canonical` to `rustc_type_ir`) - rust-lang#117009 (On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items) - rust-lang#117175 (Rename AsyncCoroutineKind to CoroutineSource) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#117009 - fmease:diag-disambig-sugg-crate, r=b-naber On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items Fixes rust-lang#116970.

rustbot assigned b-naber Oct 21, 2023

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 Oct 21, 2023

fmease commented Oct 21, 2023

View reviewed changes

tests/ui/unresolved/unresolved-import-avoid-suggesting-global-path.rs Outdated Show resolved Hide resolved

fmease changed the title ~~On unresolved import, disambiguate suggested path if crate from extern prelude would collide with local module or type~~ On unresolved import, disambiguate suggested path if it would collide with local item Oct 21, 2023

fmease force-pushed the diag-disambig-sugg-crate branch from c21e607 to 7f55ccb Compare October 21, 2023 04:38

fmease changed the title ~~On unresolved import, disambiguate suggested path if it would collide with local item~~ On unresolved import, suggest a disambiguated path if the path would otherwise collide with local item Oct 21, 2023

fmease changed the title ~~On unresolved import, suggest a disambiguated path if the path would otherwise collide with local item~~ On unresolved import, suggest a disambiguated path if the path would otherwise collide with a local item Oct 21, 2023

This comment has been minimized.

Sign in to view

fmease force-pushed the diag-disambig-sugg-crate branch 4 times, most recently from cd6b277 to d42e8cf Compare October 21, 2023 11:57

fmease changed the title ~~On unresolved import, suggest a disambiguated path if the path would otherwise collide with a local item~~ On unresolved import, suggest a disambiguated path if necessary to avoid collision with a local item Oct 21, 2023

fmease changed the title ~~On unresolved import, suggest a disambiguated path if necessary to avoid collision with a local item~~ On unresolved import, suggest a disambiguated path if necessary to avoid collision with local items Oct 21, 2023

fmease changed the title ~~On unresolved import, suggest a disambiguated path if necessary to avoid collision with local items~~ On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items Oct 21, 2023

fmease force-pushed the diag-disambig-sugg-crate branch 2 times, most recently from 6cad9f5 to a137e46 Compare October 21, 2023 13:05

on unresolved import disambiguate suggested path if it would collide

4aaf8e0

fmease force-pushed the diag-disambig-sugg-crate branch from a137e46 to 4aaf8e0 Compare October 21, 2023 22:02

b-naber approved these changes Oct 25, 2023

View reviewed changes

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 Oct 25, 2023

matthiaskrgr mentioned this pull request Oct 25, 2023

Rollup of 6 pull requests #117193

Merged

bors merged commit 2a027fa into rust-lang:master Oct 26, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 26, 2023

fmease deleted the diag-disambig-sugg-crate branch October 26, 2023 05:30

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

fmease mentioned this pull request May 3, 2024

Rustc suggests an import that won't work if a crate and a module have the same name #90810

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items #117009

On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items #117009

fmease commented Oct 21, 2023

rustbot commented Oct 21, 2023

fmease Oct 21, 2023 •

edited

Loading

fmease Oct 21, 2023 •

edited

Loading

This comment has been minimized.

fmease commented Oct 21, 2023

b-naber left a comment

b-naber Oct 25, 2023

fmease Oct 25, 2023 •

edited

Loading

b-naber Oct 25, 2023

b-naber commented Oct 25, 2023

bors commented Oct 25, 2023

		// We have to visit module children in deterministic order to avoid
		// instabilities in reported imports (#43552).

On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items #117009

On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items #117009

Conversation

fmease commented Oct 21, 2023

rustbot commented Oct 21, 2023

fmease Oct 21, 2023 • edited Loading

Choose a reason for hiding this comment

fmease Oct 21, 2023 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

fmease commented Oct 21, 2023

b-naber left a comment

Choose a reason for hiding this comment

b-naber Oct 25, 2023

Choose a reason for hiding this comment

fmease Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

b-naber Oct 25, 2023

Choose a reason for hiding this comment

b-naber commented Oct 25, 2023

bors commented Oct 25, 2023

fmease Oct 21, 2023 •

edited

Loading

fmease Oct 21, 2023 •

edited

Loading

fmease Oct 25, 2023 •

edited

Loading