report unused_import for empty reexports even it is pub

[bvanjoi]

Fixes #116032

An easy fix. r? @petrochenkov

(Discovered this issue while reviewing #115993.)

[bvanjoi]

Perhaps this case should also be report:

mod c {
  const C: i32 = 1;
}

pub use c::*;

fn main() {}

[rustbot]

Portable SIMD is developed in its own repository. If possible, consider making this change to rust-lang/portable-simd instead.

cc @calebzulawski, @programmerjake

[rustbot]

Some changes occurred in src/tools/rust-analyzer

cc @rust-lang/rust-analyzer

[bvanjoi]

These deleted imports statements in this PR can be categorized into two types:

• Those that are neither used nor re-exported, like pub use crate::sys_common::process::CommandEnvs; in the std::sys::unix::process module.
• Those that have been re-exported by other modules, like pub use self::format::* in rustc_ast, which has been re-exported in pub use self::ast::*.

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[bvanjoi]

I realized there's an overlooked aspect related to this issue: we can't reliably determine whether the count of public items is genuinely empty in different environments. For example:

mod a {
  #[cfg(target_os="windows")]
  pub const A: i32 = 1;
}

pub use a::*;

If we follow the changes in this PR, it will throw an unused_imports for pub use a::* in Linux, which might not be the actual desired outcome.

[petrochenkov]

If we follow the changes in this PR, it will throw an unused_imports for pub use a::* in Linux

That's a perfectly valid use case for allow.
Or for placing #[cfg(target_os="windows")] on the import too.

All unused lints are affected by something like this, e.g.

struct S;

#[cfg(target_os = "windows")]
fn use_s() {
    let _s = S;
}

will also report an unused warning.

[rustbot]

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

The Miri subtree was changed

cc @rust-lang/miri

[bvanjoi]

ci is green

@rustbot ready

[petrochenkov]

Same as with library/std/src/sys/common/mod.rs.

[bvanjoi]

In this case, it's under the tests directory and none of the re-exports have been used, so it will be detected. Therefore, adding #![allow(unused_imports)] is necessary.

By the way, once this PR is merged, it will throw unused_imports warnings for re-export statements that are solely intended to re-export something in test files. Could this potentially cause confusion for users?

[petrochenkov]

Well, if some code is only used for tests it is better to put it under #[cfg(test)].
Preferably by moving into mod tests files, if possible.

[bors]

⌛ Testing commit 482275b with merge 41aa06e...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 41aa06e to master...

[rust-timer]

Finished benchmarking commit (41aa06e): 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.4%	[0.3%, 0.5%]	13
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.7%	[-3.7%, -3.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[-3.7%, 0.5%]	14

Max RSS (memory usage)

Results

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)	3.7%	[2.0%, 5.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results

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

Binary size

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

Bootstrap: missing data
Artifact size: 304.31 MiB -> 304.29 MiB (-0.01%)

[lqd]

(The syn-1.0.89 opt full benchmark is currently being noisy so we can ignore that. The other small regressions look indeed real.)

[rylev]

@bvanjoi @petrochenkov the small regressions in the serde benchmark do seem real. I'll leave it up to you whether you want to investigate. The magnitude is small enough that it might not be worth it.

[petrochenkov]

Maybe the change is due to rust-lang/rustc-perf#1736 and not this PR by itself.

The change this PR is making is basically adding 2 integer hash map lookups (unless the new diagnostics are actually emitted), so it should be entirely within the noise.

[Kobzol]

Maybe the change is due to rust-lang/rustc-perf#1736 and not this PR by itself.

I also thought so, but if that was the case, it would be seen in the master perf. results sooner, and not only after this PR.

That being said, I also think that the perf. regression is fine and I wouldn't worry about it.