mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #5434 from eddyb/rustup

Browse Source 
rustup: update for the new Ty::walk interface.

The first commit fixes a portability bug in `setup-toolchain.sh`, while the second rewrites the handling of "trait impl methods" in `use_self` - even if `Ty::walk` could've still been used, it was IMO a misuse.

This could also serve as a PSA: *please* use `hir_ty_to_ty` instead of trying to compare `hir::Ty`s between themselves or against semantic `Ty`s. Its "quasi-deprecation" is 3 years old and doesn't really mean anything, just that it's currently uncached and that we should eventually querify it (either for a single HIR node, or for all of the nodes in an entire definition).

---

changelog: none
This commit is contained in:
Philipp Krones 2020-04-07 22:40:18 +02:00 committed by GitHub
commit d342cee787
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 67 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
clippy_lints/src/let_underscore.rs
View File

@ -2,6 +2,7 @@
use rustc_hir::{Local, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use crate::utils::{is_must_use_func_call, is_must_use_ty, match_type, paths, span_lint_and_help};
@ -75,8 +76,15 @@ fn check_local(&mut self, cx: &LateContext<'_, '_>, local: &Local<'_>) {
if let PatKind::Wild = local.pat.kind;
if let Some(ref init) = local.init;
then {
let check_ty = |ty| SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, ty, path));
if cx.tables.expr_ty(init).walk().any(check_ty) {
let init_ty = cx.tables.expr_ty(init);
let contains_sync_guard = init_ty.walk().any(|inner| match inner.unpack() {
GenericArgKind::Type(inner_ty) => {
SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, inner_ty, path))
},
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
});
if contains_sync_guard {
span_lint_and_help(
cx,
LET_UNDERSCORE_LOCK,

25
clippy_lints/src/methods/mod.rs
View File

@ -15,6 +15,7 @@
use rustc_lint::{LateContext, LateLintPass, Lint, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::{self, Predicate, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
@ -1407,7 +1408,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id);
let item = cx.tcx.hir().expect_item(parent);
let def_id = cx.tcx.hir().local_def_id(item.hir_id);
let ty = cx.tcx.type_of(def_id);
let self_ty = cx.tcx.type_of(def_id);
if_chain! {
if let hir::ImplItemKind::Fn(ref sig, id) = impl_item.kind;
if let Some(first_arg) = iter_input_pats(&sig.decl, cx.tcx.hir().body(id)).next();
@ -1429,7 +1430,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
if name == method_name &&
sig.decl.inputs.len() == n_args &&
out_type.matches(cx, &sig.decl.output) &&
self_kind.matches(cx, ty, first_arg_ty) {
self_kind.matches(cx, self_ty, first_arg_ty) {
span_lint(cx, SHOULD_IMPLEMENT_TRAIT, impl_item.span, &format!(
"defining a method called `{}` on this type; consider implementing \
the `{}` trait or choosing a less ambiguous name", name, trait_name));
@ -1441,7 +1442,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
.iter()
.find(|(ref conv, _)| conv.check(&name))
{
if !self_kinds.iter().any(|k| k.matches(cx, ty, first_arg_ty)) {
if !self_kinds.iter().any(|k| k.matches(cx, self_ty, first_arg_ty)) {
let lint = if item.vis.node.is_pub() {
WRONG_PUB_SELF_CONVENTION
} else {
@ -1471,8 +1472,16 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
if let hir::ImplItemKind::Fn(_, _) = impl_item.kind {
let ret_ty = return_ty(cx, impl_item.hir_id);
let contains_self_ty = |ty: Ty<'tcx>| {
ty.walk().any(|inner| match inner.unpack() {
GenericArgKind::Type(inner_ty) => same_tys(cx, self_ty, inner_ty),
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
})
};
// walk the return type and check for Self (this does not check associated types)
if ret_ty.walk().any(|inner_type| same_tys(cx, ty, inner_type)) {
if contains_self_ty(ret_ty) {
return;
}
@ -1486,10 +1495,8 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
let associated_type = binder.skip_binder();
// walk the associated type and check for Self
for inner_type in associated_type.walk() {
if same_tys(cx, ty, inner_type) {
return;
}
if contains_self_ty(associated_type) {
return;
}
},
(_, _) => {},
@ -1497,7 +1504,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
}
}
if name == "new" && !same_tys(cx, ret_ty, ty) {
if name == "new" && !same_tys(cx, ret_ty, self_ty) {
span_lint(
cx,
NEW_RET_NO_SELF,

89
clippy_lints/src/use_self.rs
View File

@ -14,6 +14,7 @@
use rustc_middle::ty::{DefIdTree, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::kw;
use rustc_typeck::hir_ty_to_ty;
use crate::utils::{differing_macro_contexts, span_lint_and_sugg};
@ -80,37 +81,28 @@ fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path<'_>, last_segment: O
);
}
struct TraitImplTyVisitor<'a, 'tcx> {
item_type: Ty<'tcx>,
// FIXME: always use this (more correct) visitor, not just in method signatures.
struct SemanticUseSelfVisitor<'a, 'tcx> {
cx: &'a LateContext<'a, 'tcx>,
trait_type_walker: ty::walk::TypeWalker<'tcx>,
impl_type_walker: ty::walk::TypeWalker<'tcx>,
self_ty: Ty<'tcx>,
}
impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for SemanticUseSelfVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_ty(&mut self, t: &'tcx hir::Ty<'_>) {
let trait_ty = self.trait_type_walker.next();
let impl_ty = self.impl_type_walker.next();
if_chain! {
if let TyKind::Path(QPath::Resolved(_, path)) = &t.kind;
// The implementation and trait types don't match which means that
// the concrete type was specified by the implementation
if impl_ty != trait_ty;
if let Some(impl_ty) = impl_ty;
if self.item_type == impl_ty;
then {
match path.res {
def::Res::SelfTy(..) => {},
_ => span_use_self_lint(self.cx, path, None)
}
fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'_>) {
if let TyKind::Path(QPath::Resolved(_, path)) = &hir_ty.kind {
match path.res {
def::Res::SelfTy(..) => {},
_ => {
if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
span_use_self_lint(self.cx, path, None);
}
},
}
}
walk_ty(self, t)
walk_ty(self, hir_ty)
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
@ -120,10 +112,9 @@ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
fn check_trait_method_impl_decl<'a, 'tcx>(
cx: &'a LateContext<'a, 'tcx>,
item_type: Ty<'tcx>,
impl_item: &ImplItem<'_>,
impl_decl: &'tcx FnDecl<'_>,
impl_trait_ref: &ty::TraitRef<'_>,
impl_trait_ref: ty::TraitRef<'tcx>,
) {
let trait_method = cx
.tcx
@ -134,34 +125,35 @@ fn check_trait_method_impl_decl<'a, 'tcx>(
let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id);
let trait_method_sig = cx.tcx.erase_late_bound_regions(&trait_method_sig);
let impl_method_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id);
let impl_method_sig = cx.tcx.fn_sig(impl_method_def_id);
let impl_method_sig = cx.tcx.erase_late_bound_regions(&impl_method_sig);
let output_ty = if let FnRetTy::Return(ty) = &impl_decl.output {
let output_hir_ty = if let FnRetTy::Return(ty) = &impl_decl.output {
Some(&**ty)
} else {
None
};
// `impl_decl_ty` (of type `hir::Ty`) represents the type declared in the signature.
// `impl_ty` (of type `ty:TyS`) is the concrete type that the compiler has determined for
// that declaration. We use `impl_decl_ty` to see if the type was declared as `Self`
// and use `impl_ty` to check its concrete type.
for (impl_decl_ty, (impl_ty, trait_ty)) in impl_decl.inputs.iter().chain(output_ty).zip(
impl_method_sig
.inputs_and_output
.iter()
.zip(trait_method_sig.inputs_and_output),
) {
let mut visitor = TraitImplTyVisitor {
cx,
item_type,
trait_type_walker: trait_ty.walk(),
impl_type_walker: impl_ty.walk(),
};
// `impl_hir_ty` (of type `hir::Ty`) represents the type written in the signature.
// `trait_ty` (of type `ty::Ty`) is the semantic type for the signature in the trait.
// We use `impl_hir_ty` to see if the type was written as `Self`,
// `hir_ty_to_ty(...)` to check semantic types of paths, and
// `trait_ty` to determine which parts of the signature in the trait, mention
// the type being implemented verbatim (as opposed to `Self`).
for (impl_hir_ty, trait_ty) in impl_decl
.inputs
.iter()
.chain(output_hir_ty)
.zip(trait_method_sig.inputs_and_output)
{
// Check if the input/output type in the trait method specifies the implemented
// type verbatim, and only suggest `Self` if that isn't the case.
// This avoids suggestions to e.g. replace `Vec<u8>` with `Vec<Self>`,
// in an `impl Trait for u8`, when the trait always uses `Vec<u8>`.
// See also https://github.com/rust-lang/rust-clippy/issues/2894.
let self_ty = impl_trait_ref.self_ty();
if !trait_ty.walk().any(|inner| inner == self_ty.into()) {
let mut visitor = SemanticUseSelfVisitor { cx, self_ty };
visitor.visit_ty(&impl_decl_ty);
visitor.visit_ty(&impl_hir_ty);
}
}
}
@ -197,8 +189,7 @@ fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) {
let impl_item = cx.tcx.hir().impl_item(impl_item_ref.id);
if let ImplItemKind::Fn(FnSig{ decl: impl_decl, .. }, impl_body_id)
= &impl_item.kind {
let item_type = cx.tcx.type_of(impl_def_id);
check_trait_method_impl_decl(cx, item_type, impl_item, impl_decl, &impl_trait_ref);
check_trait_method_impl_decl(cx, impl_item, impl_decl, impl_trait_ref);
let body = cx.tcx.hir().body(*impl_body_id);
visitor.visit_body(body);

2
setup-toolchain.sh
View File

@ -1,4 +1,4 @@
#!/bin/bash
#!/usr/bin/env bash
# Set up the appropriate rustc toolchain
set -e