-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Replace str path utils with new PathLookup type
#14705
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.
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
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
| 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) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're changing things can you make lookup_path_with_base take a vec to write into.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely a lot nicer than what we had before. Just one question, but this looks good either way.
clippy_utils/src/lib.rs
Outdated
| 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 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to move this into the paths module? The crate root is kind of a mess right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good plan, moved them across
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