Skip to content

Rustup to rust-lang/rust#64874 #4628

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

Merged
merged 6 commits into from
Oct 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
98 changes: 25 additions & 73 deletions clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxedLocal {

let fn_def_id = cx.tcx.hir().local_def_id(hir_id);
let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
ExprUseVisitor::new(
&mut v,
cx.tcx,
fn_def_id,
cx.param_env,
region_scope_tree,
cx.tables,
None,
)
.consume_body(body);
ExprUseVisitor::new(&mut v, cx.tcx, fn_def_id, cx.param_env, region_scope_tree, cx.tables).consume_body(body);

for node in v.set {
span_lint(
Expand All @@ -114,86 +105,47 @@ fn is_argument(map: &hir::map::Map<'_>, id: HirId) -> bool {
}

impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
fn consume(&mut self, _: HirId, _: Span, cmt: &cmt_<'tcx>, mode: ConsumeMode) {
fn consume(&mut self, cmt: &cmt_<'tcx>, mode: ConsumeMode) {
if let Categorization::Local(lid) = cmt.cat {
if let Move(DirectRefMove) | Move(CaptureMove) = mode {
if let ConsumeMode::Move = mode {
// moved out or in. clearly can't be localized
self.set.remove(&lid);
}
}
}
fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {}
fn consume_pat(&mut self, consume_pat: &Pat, cmt: &cmt_<'tcx>, _: ConsumeMode) {
let map = &self.cx.tcx.hir();
if is_argument(map, consume_pat.hir_id) {
// Skip closure arguments
let parent_id = map.get_parent_node(consume_pat.hir_id);
if let Some(Node::Expr(..)) = map.find(map.get_parent_node(parent_id)) {
return;
}

if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) {
self.set.insert(consume_pat.hir_id);
}
return;
}
if let Categorization::Rvalue(..) = cmt.cat {
if let Some(Node::Stmt(st)) = map.find(map.get_parent_node(cmt.hir_id)) {
if let StmtKind::Local(ref loc) = st.kind {
if let Some(ref ex) = loc.init {
if let ExprKind::Box(..) = ex.kind {
if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) {
// let x = box (...)
self.set.insert(consume_pat.hir_id);
}
// TODO Box::new
// TODO vec![]
// TODO "foo".to_owned() and friends
}
}
if let Categorization::Local(lid) = cmt.cat {
if let Some(Node::Binding(_)) = map.find(cmt.hir_id) {
if self.set.contains(&lid) {
// let y = x where x is known
// remove x, insert y
self.set.insert(cmt.hir_id);
self.set.remove(&lid);
}
}
}
}

fn borrow(&mut self, cmt: &cmt_<'tcx>, _: ty::BorrowKind) {
if let Categorization::Local(lid) = cmt.cat {
if self.set.contains(&lid) {
// let y = x where x is known
// remove x, insert y
self.set.insert(consume_pat.hir_id);
self.set.remove(&lid);
}
self.set.remove(&lid);
}
}
fn borrow(
&mut self,
_: HirId,
_: Span,
cmt: &cmt_<'tcx>,
_: ty::Region<'_>,
_: ty::BorrowKind,
loan_cause: LoanCause,
) {
if let Categorization::Local(lid) = cmt.cat {
match loan_cause {
// `x.foo()`
// Used without autoderef-ing (i.e., `x.clone()`).
LoanCause::AutoRef |

// `&x`
// `foo(&x)` where no extra autoref-ing is happening.
LoanCause::AddrOf |

// `match x` can move.
LoanCause::MatchDiscriminant => {
self.set.remove(&lid);
}
fn mutate(&mut self, cmt: &cmt_<'tcx>) {
let map = &self.cx.tcx.hir();
if is_argument(map, cmt.hir_id) {
// Skip closure arguments
let parent_id = map.get_parent_node(cmt.hir_id);
if let Some(Node::Expr(..)) = map.find(map.get_parent_node(parent_id)) {
return;
}

// Do nothing for matches, etc. These can't escape.
_ => {}
if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) {
self.set.insert(cmt.hir_id);
}
return;
}
}
fn decl_without_init(&mut self, _: HirId, _: Span) {}
fn mutate(&mut self, _: HirId, _: Span, _: &cmt_<'tcx>, _: MutateMode) {}
}

impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> {
Expand Down
21 changes: 7 additions & 14 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1547,37 +1547,31 @@ struct MutatePairDelegate {
}

impl<'tcx> Delegate<'tcx> for MutatePairDelegate {
fn consume(&mut self, _: HirId, _: Span, _: &cmt_<'tcx>, _: ConsumeMode) {}
fn consume(&mut self, _: &cmt_<'tcx>, _: ConsumeMode) {}

fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {}

fn consume_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: ConsumeMode) {}

fn borrow(&mut self, _: HirId, sp: Span, cmt: &cmt_<'tcx>, _: ty::Region<'_>, bk: ty::BorrowKind, _: LoanCause) {
fn borrow(&mut self, cmt: &cmt_<'tcx>, bk: ty::BorrowKind) {
if let ty::BorrowKind::MutBorrow = bk {
if let Categorization::Local(id) = cmt.cat {
if Some(id) == self.hir_id_low {
self.span_low = Some(sp)
self.span_low = Some(cmt.span)
}
if Some(id) == self.hir_id_high {
self.span_high = Some(sp)
self.span_high = Some(cmt.span)
}
}
}
}

fn mutate(&mut self, _: HirId, sp: Span, cmt: &cmt_<'tcx>, _: MutateMode) {
fn mutate(&mut self, cmt: &cmt_<'tcx>) {
if let Categorization::Local(id) = cmt.cat {
if Some(id) == self.hir_id_low {
self.span_low = Some(sp)
self.span_low = Some(cmt.span)
}
if Some(id) == self.hir_id_high {
self.span_high = Some(sp)
self.span_high = Some(cmt.span)
}
}
}

fn decl_without_init(&mut self, _: HirId, _: Span) {}
}

impl<'tcx> MutatePairDelegate {
Expand Down Expand Up @@ -1655,7 +1649,6 @@ fn check_for_mutation(
cx.param_env,
region_scope_tree,
cx.tables,
None,
)
.walk_expr(body);
delegate.mutation_span()
Expand Down
23 changes: 7 additions & 16 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ use syntax::symbol::{sym, LocalInternedString, Symbol};
use crate::utils::usage::mutated_variables;
use crate::utils::{
get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy,
is_ctor_function, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path,
match_qpath, match_trait_method, match_type, match_var, method_calls, method_chain_args, paths, remove_blocks,
return_ty, same_tys, single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite,
span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, sugg, walk_ptrs_ty,
walk_ptrs_ty_depth, SpanlessEq,
is_ctor_or_promotable_const_function, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment,
match_def_path, match_qpath, match_trait_method, match_type, match_var, method_calls, method_chain_args, paths,
remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_applicability,
snippet_with_macro_callsite, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then,
span_note_and_lint, sugg, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq,
};

declare_clippy_lint! {
Expand Down Expand Up @@ -1281,22 +1281,13 @@ fn lint_or_fun_call<'a, 'tcx>(
fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
let call_found = match &expr.kind {
// ignore enum and struct constructors
hir::ExprKind::Call(..) => !is_ctor_function(self.cx, expr),
hir::ExprKind::Call(..) => !is_ctor_or_promotable_const_function(self.cx, expr),
hir::ExprKind::MethodCall(..) => true,
_ => false,
};

if call_found {
// don't lint for constant values
let owner_def = self.cx.tcx.hir().get_parent_did(expr.hir_id);
let promotable = self
.cx
.tcx
.rvalue_promotable_map(owner_def)
.contains(&expr.hir_id.local_id);
if !promotable {
self.found |= true;
}
self.found |= true;
}

if !self.found {
Expand Down
115 changes: 13 additions & 102 deletions clippy_lints/src/needless_pass_by_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
spans_need_deref,
..
} = {
let mut ctx = MovedVariablesCtxt::new(cx);
let mut ctx = MovedVariablesCtxt::default();
let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
euv::ExprUseVisitor::new(
&mut ctx,
cx.tcx,
fn_def_id,
cx.param_env,
region_scope_tree,
cx.tables,
None,
)
.consume_body(body);
euv::ExprUseVisitor::new(&mut ctx, cx.tcx, fn_def_id, cx.param_env, region_scope_tree, cx.tables)
.consume_body(body);
ctx
};

Expand Down Expand Up @@ -325,115 +317,34 @@ fn requires_exact_signature(attrs: &[Attribute]) -> bool {
})
}

struct MovedVariablesCtxt<'a, 'tcx> {
cx: &'a LateContext<'a, 'tcx>,
#[derive(Default)]
struct MovedVariablesCtxt {
moved_vars: FxHashSet<HirId>,
/// Spans which need to be prefixed with `*` for dereferencing the
/// suggested additional reference.
spans_need_deref: FxHashMap<HirId, FxHashSet<Span>>,
}

impl<'a, 'tcx> MovedVariablesCtxt<'a, 'tcx> {
fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
Self {
cx,
moved_vars: FxHashSet::default(),
spans_need_deref: FxHashMap::default(),
}
}

fn move_common(&mut self, _consume_id: HirId, _span: Span, cmt: &mc::cmt_<'tcx>) {
impl MovedVariablesCtxt {
fn move_common(&mut self, cmt: &mc::cmt_<'_>) {
let cmt = unwrap_downcast_or_interior(cmt);

if let mc::Categorization::Local(vid) = cmt.cat {
self.moved_vars.insert(vid);
}
}

fn non_moving_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>) {
Copy link
Member Author

@flip1995 flip1995 Oct 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is removing this completely ok? It didn't break anything in the tests 🤔 cc @sinkuu since you added this function in 627d24c

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing is inserted to spans_need_deref after this change, so the suggestions to add * should be broken, but idk.

Anyway, Rust has gained default binding mode and this suggestion became useless. I think it's OK to remove it.

let cmt = unwrap_downcast_or_interior(cmt);

if let mc::Categorization::Local(vid) = cmt.cat {
let mut id = matched_pat.hir_id;
loop {
let parent = self.cx.tcx.hir().get_parent_node(id);
if id == parent {
// no parent
return;
}
id = parent;

if let Some(node) = self.cx.tcx.hir().find(id) {
match node {
Node::Expr(e) => {
// `match` and `if let`
if let ExprKind::Match(ref c, ..) = e.kind {
self.spans_need_deref
.entry(vid)
.or_insert_with(FxHashSet::default)
.insert(c.span);
}
},

Node::Stmt(s) => {
// `let <pat> = x;`
if_chain! {
if let StmtKind::Local(ref local) = s.kind;
then {
self.spans_need_deref
.entry(vid)
.or_insert_with(FxHashSet::default)
.insert(local.init
.as_ref()
.map(|e| e.span)
.expect("`let` stmt without init aren't caught by match_pat"));
}
}
},

_ => {},
}
}
}
}
}
}

impl<'a, 'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt<'a, 'tcx> {
fn consume(&mut self, consume_id: HirId, consume_span: Span, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
if let euv::ConsumeMode::Move(_) = mode {
self.move_common(consume_id, consume_span, cmt);
}
}

fn matched_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::MatchMode) {
if let euv::MatchMode::MovingMatch = mode {
self.move_common(matched_pat.hir_id, matched_pat.span, cmt);
} else {
self.non_moving_pat(matched_pat, cmt);
}
}

fn consume_pat(&mut self, consume_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
if let euv::ConsumeMode::Move(_) = mode {
self.move_common(consume_pat.hir_id, consume_pat.span, cmt);
impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt {
fn consume(&mut self, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
if let euv::ConsumeMode::Move = mode {
self.move_common(cmt);
}
}

fn borrow(
&mut self,
_: HirId,
_: Span,
_: &mc::cmt_<'tcx>,
_: ty::Region<'_>,
_: ty::BorrowKind,
_: euv::LoanCause,
) {
}

fn mutate(&mut self, _: HirId, _: Span, _: &mc::cmt_<'tcx>, _: euv::MutateMode) {}
fn borrow(&mut self, _: &mc::cmt_<'tcx>, _: ty::BorrowKind) {}

fn decl_without_init(&mut self, _: HirId, _: Span) {}
fn mutate(&mut self, _: &mc::cmt_<'tcx>) {}
}

fn unwrap_downcast_or_interior<'a, 'tcx>(mut cmt: &'a mc::cmt_<'tcx>) -> mc::cmt_<'tcx> {
Expand Down
12 changes: 7 additions & 5 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,13 +803,15 @@ pub fn is_copy<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
}

/// Checks if an expression is constructing a tuple-like enum variant or struct
pub fn is_ctor_function(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
pub fn is_ctor_or_promotable_const_function(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
if let ExprKind::Call(ref fun, _) = expr.kind {
if let ExprKind::Path(ref qp) = fun.kind {
return matches!(
cx.tables.qpath_res(qp, fun.hir_id),
def::Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(..), _)
);
let res = cx.tables.qpath_res(qp, fun.hir_id);
return match res {
def::Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(..), _) => true,
def::Res::Def(_, def_id) => cx.tcx.is_promotable_const_fn(def_id),
_ => false,
};
}
}
false
Expand Down
Loading