Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc improvements to non local defs lint implementation #121813

Merged
merged 4 commits into from
Mar 9, 2024
Merged

Misc improvements to non local defs lint implementation #121813

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5c87ca2
Add some weird test cases to the non_local_definitions lint tests
Urgau Feb 29, 2024
20200f6
Remove useless smallvec dependency in rustc_lint::non_local_def
Urgau Feb 29, 2024
6c4eadd
Add early-return when checking if a path is local
Urgau Feb 29, 2024
98dbe9a
Use was_invoked_from_cargo method instead of hand-written one
Urgau Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Remove useless smallvec dependency in rustc_lint::non_local_def
  • Loading branch information
@Urgau
Urgau committed Mar 7, 2024
commit 20200f65ca63c4b3cd75aa1ebb9829188c78b388
1 change: 0 additions & 1 deletion Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4133,7 +4133,6 @@ dependencies = [
"rustc_target",
"rustc_trait_selection",
"rustc_type_ir",
"smallvec",
"tracing",
"unicode-security",
]
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_lint/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
30 changes: 18 additions & 12 deletions compiler/rustc_lint/src/non_local_def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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([], _, _)
Expand All @@ -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
Expand Down Expand Up @@ -218,6 +216,14 @@ 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| {
let res_parent = cx.tcx.parent(did);
res_parent == impl_parent || Some(res_parent) == impl_parent_parent
})
}