Replace str path utils with new PathLookup type#14705
Merged
Jarcho merged 2 commits intorust-lang:masterfrom May 5, 2025
Merged
Replace str path utils with new PathLookup type#14705Jarcho merged 2 commits intorust-lang:masterfrom
PathLookup type#14705Jarcho merged 2 commits intorust-lang:masterfrom
Conversation
Collaborator
2 tasks
This comment has been minimized.
This comment has been minimized.
Jarcho
reviewed
May 2, 2025
Contributor
Jarcho
left a comment
There was a problem hiding this comment.
This also breaks the author lint. It didn't really work properly before with match_qpath, but it's even more broken now.
clippy_utils/src/lib.rs
Outdated
Comment on lines
646
to
678
| let base = find_crates(tcx, base) | ||
| .iter() | ||
| .chain(find_primitive_impls(tcx, base)) | ||
| .copied() | ||
| .collect(); | ||
|
|
||
| def_path_res_with_base(tcx, crates, path) | ||
| lookup_path_with_base(tcx, base, path) | ||
| } |
Contributor
There was a problem hiding this comment.
Since we're changing things can you make lookup_path_with_base take a vec to write into.
Member
Author
|
Jarcho
approved these changes
May 4, 2025
Contributor
Jarcho
left a comment
There was a problem hiding this comment.
Definitely a lot nicer than what we had before. Just one question, but this looks good either way.
clippy_utils/src/lib.rs
Outdated
Comment on lines
667
to
678
| pub fn lookup_path(tcx: TyCtxt<'_>, ns: PathNS, path: &[Symbol]) -> Vec<DefId> { | ||
| let (root, rest) = match *path { | ||
| [] | [_] => return Vec::new(), | ||
| [root, ref rest @ ..] => (root, rest), | ||
| }; | ||
|
|
||
| let crates = find_primitive_impls(tcx, base) | ||
| .chain(local_crate) | ||
| .map(|id| Res::Def(tcx.def_kind(id), id)) | ||
| .chain(find_crates(tcx, base_sym)) | ||
| .collect(); | ||
|
|
||
| def_path_res_with_base(tcx, crates, path) | ||
| let mut out = Vec::new(); | ||
| for &base in find_crates(tcx, root).iter().chain(find_primitive_impls(tcx, root)) { | ||
| lookup_path_with_base(tcx, base, ns, rest, &mut out); | ||
| } | ||
| out | ||
| } |
Contributor
There was a problem hiding this comment.
Would it make sense to move this into the paths module? The crate root is kind of a mess right now.
Member
Author
There was a problem hiding this comment.
Yeah good plan, moved them across
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
&[&str]path basedclippy_utilshave been removed and replace with a new typePathLookup:match_trait_methodmatch_qpathmatch_pathmatch_any_def_pathsmatch_def_pathmatch_typeget_trait_def_idInternally
PathLookupis a lazy call tolookup_path(the new name fordef_path_resto distinguish it frompath_res)The
invalid_pathsinternal lint is removed, it could be reimplemented but it feels redundant since every path should be covered by a test anywayUser facing changes
manual_saturating_arithmeticnow checks foru32::MAX/MINinstead of only detecting the legacy numeric consts (std::u32::MAX/MIN),clippy::legacy_numeric_constantswill redirect usages of the legacy versions to the new oneallow-invalid = truenow suppresses all invalid path warnings, currently you can run into a warning that can't be ignored in some situations, e.g. withserdewithout thederivefeatureRe-exports of primitives types like
std::primitive::*no longer work indisallowed-types, this seems acceptable since it would be unusual to deny a primitive this way rather than writing e.g.usize. Type aliases such asc_charare unaffectedA similar slight performance improvement to Replace interning of string literals with preinterned symbols #14650
changelog: none