Rollup merge of #121813 - Urgau:misc-non_local_defs-lint, r=cjgillot

Misc improvements to non local defs lint implementation

This PR is a collection of small improvements I found when I [needlessly tried](https://www.github.com/rust-lang/rust/pull/120393#issuecomment-1971787475) to fix a "perf-regression" in the lint implementation.

I recommend looking at each commit individually.
This commit is contained in:
Matthias Krüger 2024-03-09 16:21:15 +01:00 committed by GitHub
commit a979f971b4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 78 additions and 30 deletions

View File

@ -4133,7 +4133,6 @@ dependencies = [
"rustc_target", "rustc_target",
"rustc_trait_selection", "rustc_trait_selection",
"rustc_type_ir", "rustc_type_ir",
"smallvec",
"tracing", "tracing",
"unicode-security", "unicode-security",
] ]

View File

@ -23,7 +23,6 @@ rustc_span = { path = "../rustc_span" }
rustc_target = { path = "../rustc_target" } rustc_target = { path = "../rustc_target" }
rustc_trait_selection = { path = "../rustc_trait_selection" } rustc_trait_selection = { path = "../rustc_trait_selection" }
rustc_type_ir = { path = "../rustc_type_ir" } rustc_type_ir = { path = "../rustc_type_ir" }
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
tracing = "0.1" tracing = "0.1"
unicode-security = "0.1.0" unicode-security = "0.1.0"
# tidy-alphabetical-end # tidy-alphabetical-end

View File

@ -2,8 +2,6 @@ use rustc_hir::{def::DefKind, Body, Item, ItemKind, Node, Path, QPath, TyKind};
use rustc_span::def_id::{DefId, LOCAL_CRATE}; use rustc_span::def_id::{DefId, LOCAL_CRATE};
use rustc_span::{sym, symbol::kw, ExpnKind, MacroKind}; use rustc_span::{sym, symbol::kw, ExpnKind, MacroKind};
use smallvec::{smallvec, SmallVec};
use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag}; use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag};
use crate::{LateContext, LateLintPass, LintContext}; use crate::{LateContext, LateLintPass, LintContext};
@ -85,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
if let Some(def_id) = oexpn.macro_def_id if let Some(def_id) = oexpn.macro_def_id
&& let ExpnKind::Macro(macro_kind, macro_name) = oexpn.kind && let ExpnKind::Macro(macro_kind, macro_name) = oexpn.kind
&& def_id.krate != LOCAL_CRATE && def_id.krate != LOCAL_CRATE
&& std::env::var_os("CARGO").is_some() && rustc_session::utils::was_invoked_from_cargo()
{ {
Some(NonLocalDefinitionsCargoUpdateNote { Some(NonLocalDefinitionsCargoUpdateNote {
macro_kind: macro_kind.descr(), macro_kind: macro_kind.descr(),
@ -114,25 +112,25 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
// is using local items and so we don't lint on it. // is using local items and so we don't lint on it.
// We also ignore anon-const in item by including the anon-const // We also ignore anon-const in item by including the anon-const
// parent as well; and since it's quite uncommon, we use smallvec // parent as well.
// to avoid unnecessary heap allocations. let parent_parent = if parent_def_kind == DefKind::Const
let local_parents: SmallVec<[DefId; 1]> = if parent_def_kind == DefKind::Const
&& parent_opt_item_name == Some(kw::Underscore) && parent_opt_item_name == Some(kw::Underscore)
{ {
smallvec![parent, cx.tcx.parent(parent)] Some(cx.tcx.parent(parent))
} else { } else {
smallvec![parent] None
}; };
let self_ty_has_local_parent = match impl_.self_ty.kind { let self_ty_has_local_parent = match impl_.self_ty.kind {
TyKind::Path(QPath::Resolved(_, ty_path)) => { TyKind::Path(QPath::Resolved(_, ty_path)) => {
path_has_local_parent(ty_path, cx, &*local_parents) path_has_local_parent(ty_path, cx, parent, parent_parent)
} }
TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => { TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => {
path_has_local_parent( path_has_local_parent(
principle_poly_trait_ref.trait_ref.path, principle_poly_trait_ref.trait_ref.path,
cx, cx,
&*local_parents, parent,
parent_parent,
) )
} }
TyKind::TraitObject([], _, _) TyKind::TraitObject([], _, _)
@ -154,7 +152,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
let of_trait_has_local_parent = impl_ let of_trait_has_local_parent = impl_
.of_trait .of_trait
.map(|of_trait| path_has_local_parent(of_trait.path, cx, &*local_parents)) .map(|of_trait| path_has_local_parent(of_trait.path, cx, parent, parent_parent))
.unwrap_or(false); .unwrap_or(false);
// If none of them have a local parent (LOGICAL NOR) this means that // If none of them have a local parent (LOGICAL NOR) this means that
@ -218,6 +216,16 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
/// std::convert::PartialEq<Foo<Bar>> /// std::convert::PartialEq<Foo<Bar>>
/// ^^^^^^^^^^^^^^^^^^^^^^^ /// ^^^^^^^^^^^^^^^^^^^^^^^
/// ``` /// ```
fn path_has_local_parent(path: &Path<'_>, cx: &LateContext<'_>, local_parents: &[DefId]) -> bool { fn path_has_local_parent(
path.res.opt_def_id().is_some_and(|did| local_parents.contains(&cx.tcx.parent(did))) path: &Path<'_>,
cx: &LateContext<'_>,
impl_parent: DefId,
impl_parent_parent: Option<DefId>,
) -> bool {
path.res.opt_def_id().is_some_and(|did| {
did.is_local() && {
let res_parent = cx.tcx.parent(did);
res_parent == impl_parent || Some(res_parent) == impl_parent_parent
}
})
} }

View File

@ -1,7 +1,7 @@
//@ check-pass //@ check-pass
//@ edition:2021 //@ edition:2021
//@ aux-build:non_local_macro.rs //@ aux-build:non_local_macro.rs
//@ rustc-env:CARGO=/usr/bin/cargo //@ rustc-env:CARGO_CRATE_NAME=non_local_def
#![feature(inline_const)] #![feature(inline_const)]
#![warn(non_local_definitions)] #![warn(non_local_definitions)]
@ -245,6 +245,26 @@ fn bad() {
//~^ WARN non-local `impl` definition //~^ WARN non-local `impl` definition
} }
trait Uto9 {}
trait Uto10 {}
const _: u32 = {
let _a = || {
impl Uto9 for Test {}
//~^ WARN non-local `impl` definition
1
};
type A = [u32; {
impl Uto10 for Test {}
//~^ WARN non-local `impl` definition
1
}];
1
};
struct UwU<T>(T); struct UwU<T>(T);
fn fun() { fn fun() {

View File

@ -442,7 +442,29 @@ LL | impl<T> Uto8 for T {}
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:253:5 --> $DIR/non_local_definitions.rs:252:9
|
LL | impl Uto9 for Test {}
| ^^^^^^^^^^^^^^^^^^^^^
|
= help: move this `impl` block outside the of the current closure `<unnameable>` and up 2 bodies
= note: an `impl` definition is non-local if it is nested inside an item and neither the type nor the trait are at the same nesting level as the `impl` block
= note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:259:9
|
LL | impl Uto10 for Test {}
| ^^^^^^^^^^^^^^^^^^^^^^
|
= help: move this `impl` block outside the of the current constant expression `<unnameable>` and up 2 bodies
= note: an `impl` definition is non-local if it is nested inside an item and neither the type nor the trait are at the same nesting level as the `impl` block
= note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:273:5
| |
LL | / impl Default for UwU<OwO> { LL | / impl Default for UwU<OwO> {
LL | | LL | |
@ -458,7 +480,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:264:5 --> $DIR/non_local_definitions.rs:284:5
| |
LL | / impl From<Cat> for () { LL | / impl From<Cat> for () {
LL | | LL | |
@ -474,7 +496,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:273:5 --> $DIR/non_local_definitions.rs:293:5
| |
LL | / impl AsRef<Cat> for () { LL | / impl AsRef<Cat> for () {
LL | | LL | |
@ -488,7 +510,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:284:5 --> $DIR/non_local_definitions.rs:304:5
| |
LL | / impl PartialEq<B> for G { LL | / impl PartialEq<B> for G {
LL | | LL | |
@ -504,7 +526,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:301:5 --> $DIR/non_local_definitions.rs:321:5
| |
LL | / impl PartialEq<Dog> for &Dog { LL | / impl PartialEq<Dog> for &Dog {
LL | | LL | |
@ -520,7 +542,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:308:5 --> $DIR/non_local_definitions.rs:328:5
| |
LL | / impl PartialEq<()> for Dog { LL | / impl PartialEq<()> for Dog {
LL | | LL | |
@ -536,7 +558,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:315:5 --> $DIR/non_local_definitions.rs:335:5
| |
LL | / impl PartialEq<()> for &Dog { LL | / impl PartialEq<()> for &Dog {
LL | | LL | |
@ -552,7 +574,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:322:5 --> $DIR/non_local_definitions.rs:342:5
| |
LL | / impl PartialEq<Dog> for () { LL | / impl PartialEq<Dog> for () {
LL | | LL | |
@ -568,7 +590,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:344:5 --> $DIR/non_local_definitions.rs:364:5
| |
LL | / impl From<Wrap<Wrap<Lion>>> for () { LL | / impl From<Wrap<Wrap<Lion>>> for () {
LL | | LL | |
@ -584,7 +606,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:351:5 --> $DIR/non_local_definitions.rs:371:5
| |
LL | / impl From<()> for Wrap<Lion> { LL | / impl From<()> for Wrap<Lion> {
LL | | LL | |
@ -600,7 +622,7 @@ LL | | }
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363> = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:364:13 --> $DIR/non_local_definitions.rs:384:13
| |
LL | impl MacroTrait for OutsideStruct {} LL | impl MacroTrait for OutsideStruct {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -615,7 +637,7 @@ LL | m!();
= note: this warning originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info) = note: this warning originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
warning: non-local `impl` definition, they should be avoided as they go against expectation warning: non-local `impl` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:374:1 --> $DIR/non_local_definitions.rs:394:1
| |
LL | non_local_macro::non_local_impl!(CargoUpdate); LL | non_local_macro::non_local_impl!(CargoUpdate);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -628,7 +650,7 @@ LL | non_local_macro::non_local_impl!(CargoUpdate);
= note: this warning originates in the macro `non_local_macro::non_local_impl` (in Nightly builds, run with -Z macro-backtrace for more info) = note: this warning originates in the macro `non_local_macro::non_local_impl` (in Nightly builds, run with -Z macro-backtrace for more info)
warning: non-local `macro_rules!` definition, they should be avoided as they go against expectation warning: non-local `macro_rules!` definition, they should be avoided as they go against expectation
--> $DIR/non_local_definitions.rs:377:1 --> $DIR/non_local_definitions.rs:397:1
| |
LL | non_local_macro::non_local_macro_rules!(my_macro); LL | non_local_macro::non_local_macro_rules!(my_macro);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -640,5 +662,5 @@ LL | non_local_macro::non_local_macro_rules!(my_macro);
= note: the macro `non_local_macro::non_local_macro_rules` may come from an old version of the `non_local_macro` crate, try updating your dependency with `cargo update -p non_local_macro` = note: the macro `non_local_macro::non_local_macro_rules` may come from an old version of the `non_local_macro` crate, try updating your dependency with `cargo update -p non_local_macro`
= note: this warning originates in the macro `non_local_macro::non_local_macro_rules` (in Nightly builds, run with -Z macro-backtrace for more info) = note: this warning originates in the macro `non_local_macro::non_local_macro_rules` (in Nightly builds, run with -Z macro-backtrace for more info)
warning: 50 warnings emitted warning: 52 warnings emitted