Tracking import use types for more accurate redundant import checking

[surechen]

fixes #117448

By tracking import use types to check whether it is scope uses or the other situations like module-relative uses, we can do more accurate redundant import checking.

For example unnecessary imports in std::prelude that can be eliminated:

use std::option::Option::Some;//~ WARNING the item `Some` is imported redundantly
use std::option::Option::None; //~ WARNING the item `None` is imported redundantly

[rustbot]

r? @davidtwco

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

[petrochenkov]

This doesn't really need a separate lint, the warnings can be reported under regular unused_imports lint, like other redundant imports.

Ideally these cases should be reported by fn check_for_redundant_imports without hardcoding any specific paths, but the necessary usage tracking is not fully implemented for use items in modules, from what I remember.
(Non-module redundant imports are already reported there.)

[surechen]

This doesn't really need a separate lint, the warnings can be reported under regular unused_imports lint, like other redundant imports.

Ideally these cases should be reported by fn check_for_redundant_imports without hardcoding any specific paths, but the necessary usage tracking is not fully implemented for use items in modules, from what I remember. (Non-module redundant imports are already reported there.)

Thank you very much. Sorry for not replying immediately. I will deal with it as soon as possible according to the suggestions.

[petrochenkov]

The required use tracking should look approximately like this - https://github.com/petrochenkov/rust/commits/usetrack
I've seen a couple of false positives, so it may be required to choose Used values passed to fn record_use_inner more carefully.

Also it probably makes sense to move check_for_redundant_imports to a later stage (call it somewhere around check_unused).
Then we'll be able to replace the Visibility::Public with an effective_visibilities condition like in #116033 for better precision, and to skip this warning for fully unused imports (where a different warning is reported).
(This should be an orthogonal change.)

@rustbot author

[petrochenkov]

The "scope" uses are uses like this

use something::Something;

fn foo() { let _s = Something; } // First segment of a path, resolves to whatever is currently in scope

, other uses are typically module-relative uses like

// in module `my_mod`
use something::Something;

fn foo() { let _s = crate::my_mod::Something; } // Non-first segment, resolves to what is found in module `my_mod`

[petrochenkov]

The check_for_redundant_imports logic (skip the import and check what is next in scope) only makes sense for imports that are used only during scope-based resolution.

[rustbot]

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

cc @rust-lang/rust-analyzer

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino, @ouz-a

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

[petrochenkov]

@bors r+

[bors]

📌 Commit a61126c has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit a61126c with merge 8b21296...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 8b21296 to master...

[rust-timer]

Finished benchmarking commit (8b21296): comparison URL.

Overall result: ❌ regressions - 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.2%, 1.0%]	44
Regressions ❌ (secondary)	0.7%	[0.4%, 1.1%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.4%	[0.2%, 1.0%]	44

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)	1.9%	[1.5%, 2.1%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
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)	2.0%	[1.7%, 2.3%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 640.535s -> 640.087s (-0.07%)
Artifact size: 308.83 MiB -> 308.85 MiB (0.01%)

[nnethercote]

Lots of instruction count regressions in the perf results.

[petrochenkov]

Explanation for the perf changes - #117772 (comment).
Rejected attempt to address this in rustc-perf - rust-lang/rustc-perf#1793.

[correabuscar]

is already defined here doesn't point to the proper place, as far as I can tell.
playground link

use std::ffi::{CStr, CString};

use libc;

fn main() {
    let lib_name = CString::new("libEGL.so").unwrap();
    let lib_ptr = unsafe { libc::dlopen(lib_name.as_ptr(), libc::RTLD_NOW) };

    if lib_ptr.is_null() {
        let error_msg = unsafe { CStr::from_ptr(libc::dlerror()) }.to_str().unwrap();
        println!("Error loading library: {}", error_msg);
        return;
    }

    // Access functions from the library
    // ...

    unsafe { libc::dlclose(lib_ptr) };
}

   Compiling playground v0.0.1 (/playground)
warning: the item `libc` is imported redundantly
 --> src/main.rs:3:5
  |
3 | use libc;
  |     ^^^^ the item `libc` is already defined here
  |
  = note: `#[warn(unused_imports)]` on by default

warning: `playground` (bin "playground") generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.58s
     Running `target/debug/playground`

[fmease]

#117772 (comment)

Could you open an issue for that? Thanks. Related issue: #121684.