Rollup merge of rust-lang#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.
jhpratt · Mar 9, 2024 · 32b2e63 · 32b2e63
2 parents 3c6b895 + 98dbe9a
commit 32b2e63
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 30 deletions.
diff --git a/Cargo.lock b/Cargo.lock
@@ -4133,7 +4133,6 @@ dependencies = [
  "rustc_target",
  "rustc_trait_selection",
  "rustc_type_ir",
- "smallvec",
  "tracing",
  "unicode-security",
 ]

diff --git a/compiler/rustc_lint/Cargo.toml b/compiler/rustc_lint/Cargo.toml
@@ -23,7 +23,6 @@ rustc_span = { path = "../rustc_span" }
 rustc_target = { path = "../rustc_target" }
 rustc_trait_selection = { path = "../rustc_trait_selection" }
 rustc_type_ir = { path = "../rustc_type_ir" }
-smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
 tracing = "0.1"
 unicode-security = "0.1.0"
 # tidy-alphabetical-end
diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs
@@ -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::{sym, symbol::kw, ExpnKind, MacroKind};
 
-use smallvec::{smallvec, SmallVec};
-
 use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag};
 use crate::{LateContext, LateLintPass, LintContext};
 
@@ -85,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
             if let Some(def_id) = oexpn.macro_def_id
                 && let ExpnKind::Macro(macro_kind, macro_name) = oexpn.kind
                 && def_id.krate != LOCAL_CRATE
-                && std::env::var_os("CARGO").is_some()
+                && rustc_session::utils::was_invoked_from_cargo()
             {
                 Some(NonLocalDefinitionsCargoUpdateNote {
                     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.
 
                 // We also ignore anon-const in item by including the anon-const
-                // parent as well; and since it's quite uncommon, we use smallvec
-                // to avoid unnecessary heap allocations.
-                let local_parents: SmallVec<[DefId; 1]> = if parent_def_kind == DefKind::Const
+                // parent as well.
+                let parent_parent = if parent_def_kind == DefKind::Const
                     && parent_opt_item_name == Some(kw::Underscore)
                 {
-                    smallvec![parent, cx.tcx.parent(parent)]
+                    Some(cx.tcx.parent(parent))
                 } else {
-                    smallvec![parent]
+                    None
                 };
 
                 let self_ty_has_local_parent = match impl_.self_ty.kind {
                     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, ..], _, _) => {
                         path_has_local_parent(
                             principle_poly_trait_ref.trait_ref.path,
                             cx,
-                            &*local_parents,
+                            parent,
+                            parent_parent,
                         )
                     }
                     TyKind::TraitObject([], _, _)
@@ -154,7 +152,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
 
                 let of_trait_has_local_parent = impl_
                     .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);
 
                 // 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>>
 ///    ^^^^^^^^^^^^^^^^^^^^^^^
 /// ```
-fn path_has_local_parent(path: &Path<'_>, cx: &LateContext<'_>, local_parents: &[DefId]) -> bool {
-    path.res.opt_def_id().is_some_and(|did| local_parents.contains(&cx.tcx.parent(did)))
+fn path_has_local_parent(
+    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
+        }
+    })
 }
diff --git a/tests/ui/lint/non_local_definitions.rs b/tests/ui/lint/non_local_definitions.rs
@@ -1,7 +1,7 @@
 //@ check-pass
 //@ edition:2021
 //@ aux-build:non_local_macro.rs
-//@ rustc-env:CARGO=/usr/bin/cargo
+//@ rustc-env:CARGO_CRATE_NAME=non_local_def
 
 #![feature(inline_const)]
 #![warn(non_local_definitions)]
@@ -245,6 +245,26 @@ fn bad() {
     //~^ 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);
 
 fn fun() {

diff --git a/tests/ui/lint/non_local_definitions.stderr b/tests/ui/lint/non_local_definitions.stderr
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 | |
@@ -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>
 
 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 {}
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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)
 
 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);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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)
 
 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);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -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: 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