Skip to content

Commit

Permalink
Auto merge of #4628 - flip1995:rustup, r=phansch
Browse files Browse the repository at this point in the history
Rustup to rust-lang/rust#64874

TODO:
- [x] replace `rvalue_promotable_map` in [1]
- [ ] ~~fix [2] according to this comment rust-lang/rust#64874 (comment) this should be merged with `consume`, but I didn't figure out how to merge them, yet.~~
- [ ] ~~fix [3]; What to do with `LoanCause`?~~

[2]+[3] probably have to be resolved by a rewrite of the lint. #4628 (comment)

[1]
https://github.com/rust-lang/rust-clippy/blob/54bf4ffd626970e831bb80c037f804a3b3450835/clippy_lints/src/methods/mod.rs#L1292-L1299

[2]
https://github.com/rust-lang/rust-clippy/blob/54bf4ffd626970e831bb80c037f804a3b3450835/clippy_lints/src/escape.rs#L126

[3]
https://github.com/rust-lang/rust-clippy/blob/54bf4ffd626970e831bb80c037f804a3b3450835/clippy_lints/src/escape.rs#L166-L176

I could need some help with [1]. The purpose of this is to "don't lint for constant values". cc @matthewjasper

For now I see what I can do with [2].

changelog: Temporary break `boxed_local` lint.
  • Loading branch information
bors committed Oct 8, 2019
2 parents 54bf4ff + 3d39379 commit df80193
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 260 deletions.
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>) {
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

0 comments on commit df80193

Please sign in to comment.