add unqualified_local_imports lint

[RalfJung]

This lint helps deal with rust-lang/rustfmt#4709 by having the compiler detect imports of local items that are not syntactically distinguishable from imports from other cates. Making them syntactically distinguishable ensures rustfmt can consistently apply the desired import grouping.

[rustbot]

r? @estebank

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

The Miri subtree was changed

cc @rust-lang/miri

[RalfJung]

I have enabled the lint in Miri and fixed all its occurrences, so one can see what that looks like.

[petrochenkov]

I have enabled the lint in Miri and fixed all its occurrences, so one can see what that looks like.

Ah yes, import like it's 2015 😄

There is a number of practical cases where a name is in current scope and can be imported, but it is not in self (and not in extern prelude).

// Modularize a macro
macro_use! m() {}
pub(crate) use m;

// Local enum
fn f() {
    enum E { A, B, C, D }
    use E::*;
}

// Maybe something else

These cases will not affect rustfmt behavior because the imports like these are typically separated from the main import group at the module top by some other items, but will apparently be reported by this lint.

[RalfJung]

Ah, hm... I already wondered if the lint should maybe stay silent inside functions. Those are usually pretty unambiguous and they generally won't have large amounts of imports that need grouping. (In fact I might want all imports to be a single group there even if on the file level a different style is used...)

[RalfJung]

It seems like these re-exports of local macros cannot be disambiguated with either self nor crate, so the only way to fix the warning is to allow it...

[lcnr]

could you instead disable the lint for macro definitions?

[RalfJung]

Yeah that's probably the best option. Though I don't think I quite understand where these macros even live then if they are neither in self nor in crate nor in ::... Cc @petrochenkov

[petrochenkov]

Ideally this lint would work in rustc_resolve in roughly the same way as the redundant import lint (#123813).
Then we'd check that self::path resolves to the same thing as path in all namespaces and it would work correctly in all cases.

[petrochenkov]

where these macros even live then if they are neither in self nor in crate nor in ::

There are many places where names can live, see enum Scope in rustc_resolve.
self and crate modules are Scope::Module and :: is Scope::ExternPrelude.

[RalfJung]

Ideally this lint would work in rustc_resolve in roughly the same way as the redundant import lint (#123813).

Hm... I am entirely unfamiliar with that part of the codebase. I'm happy for someone else to take over this PR, but it may be a while until I have the time to dig into this.

[RalfJung]

I now made the lint just skip over macro imports.

[RalfJung]

@rustbot author

[bors]

☔ The latest upstream changes (presumably #126891) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #130709) made this pull request unmergeable. Please resolve the merge conflicts.

[nnethercote]

It still seems odd to me to enable this lint only in Miri. It think it would have been better to enable it in a single rustc crate (rustc_const_eval?) to (a) demonstrate how it works on non-test code that uses StdExternalCrate, but (b) not cause too much difficulty with bootstrapping. But I'll be optimistic and assume that miri will start using StdExternalCrate and/or other rustc crates will use this lint in the future.

@bors r+

[rustbot]

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

[RalfJung]

It still seems odd to me to enable this lint only in Miri. It think it would have been better to enable it in a single rustc crate (rustc_const_eval?) to (a) demonstrate how it works on non-test code that uses StdExternalCrate, but (b) not cause too much difficulty with bootstrapping.

That's a fair point. I have done this for rustc_const_eval now, so you can take a look.

[nnethercote]

@bors r+

[bors]

📌 Commit 1eb51e7 has been approved by nnethercote

It is now in the queue for this repository.

[bors]

⌛ Testing commit 1eb51e7 with merge 7042c26...

[bors]

☀️ Test successful - checks-actions
Approved by: nnethercote
Pushing 7042c26 to master...

[rust-timer]

Finished benchmarking commit (7042c26): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.5%	[0.4%, 2.0%]	11
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.4%, -0.2%]	6
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -0.1%, 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.8%	[0.5%, 1.0%]	2
Regressions ❌ (secondary)	3.0%	[3.0%, 3.0%]	1
Improvements ✅ (primary)	-1.8%	[-1.8%, -1.8%]	1
Improvements ✅ (secondary)	-4.3%	[-5.2%, -3.1%]	4
All ❌✅ (primary)	-0.1%	[-1.8%, 1.0%]	3

Cycles

Results (secondary 2.4%)

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)	2.4%	[2.4%, 2.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 767.967s -> 767.3s (-0.09%)
Artifact size: 341.51 MiB -> 341.60 MiB (0.03%)

[RalfJung]

Hm, seems like the lint is run even when it is allowed? Is there some established way to avoid work for allowed lints?

[Kobzol]

I think that (some?) lints are currently running independently of their allowed status, as it was non-trivial to do it otherwise. @blyxyas might know more.

[oli-obk]

cc #59024

[jieyouxu]

Hm, seems like the lint is run even when it is allowed? Is there some established way to avoid work for allowed lints?

There is work by @blyxyas for not running allow-as-would-be-finalized-to-user in #125116, but it's very tricky because some load-bearing lints that need to run for correctness are intermingled in the code paths for emitting would-be-allow lints.

[pnkfelix]

Visiting for weekly rustc-perf triage:

• There was some surprise here for the PR author because this was an allow-by-default lint and so they didn't expect it to have any actual perf impact,
  because they assumed that allowed-linted could be skipped (which is not actually the way they work today, but there are PR's in process that try
  to get that effect).
• In any case, this PR was flagged solely because of the number of secondary benchmarks that were affected.
• Marking as triaged.

@rustbot label: +perf-regression-triaged

[camsteffen]

Should this have a tracking issue?

[jieyouxu]

Should this have a tracking issue?

Yeah, it should.

EDIT: I thought this was allow-by-default but this is warn-but-feature-gated.