Auto merge of #4628 - flip1995:rustup, r=phansch

Rustup to rust-lang/rust#64874

TODO:
- [x] replace `rvalue_promotable_map` in [1]
- [ ] ~~fix [2] according to this comment https://github.com/rust-lang/rust/pull/64874#issuecomment-536203626 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. https://github.com/rust-lang/rust-clippy/pull/4628#issuecomment-538574944

[1]
54bf4ffd62/clippy_lints/src/methods/mod.rs (L1292-L1299)

[2]
54bf4ffd62/clippy_lints/src/escape.rs (L126)

[3]
54bf4ffd62/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.
This commit is contained in:
bors 2019-10-08 05:09:53 +00:00
commit df80193bda
9 changed files with 72 additions and 261 deletions

View File

@ -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(
@ -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) {
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 {
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(consume_pat.hir_id);
let parent_id = map.get_parent_node(cmt.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);
self.set.insert(cmt.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 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);
}
}
}
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);
}
// Do nothing for matches, etc. These can't escape.
_ => {}
}
}
}
fn decl_without_init(&mut self, _: HirId, _: Span) {}
fn mutate(&mut self, _: HirId, _: Span, _: &cmt_<'tcx>, _: MutateMode) {}
}
impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> {

View File

@ -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 {
@ -1655,7 +1649,6 @@ fn check_for_mutation(
cx.param_env,
region_scope_tree,
cx.tables,
None,
)
.walk_expr(body);
delegate.mutation_span()

View File

@ -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! {
@ -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 {

View File

@ -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
};
@ -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);
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 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 borrow(&mut self, _: &mc::cmt_<'tcx>, _: ty::BorrowKind) {}
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);
}
}
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 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> {

View File

@ -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

View File

@ -6,7 +6,6 @@ use rustc::middle::mem_categorization::cmt_;
use rustc::middle::mem_categorization::Categorization;
use rustc::ty;
use rustc_data_structures::fx::FxHashSet;
use syntax::source_map::Span;
/// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined.
pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr, cx: &'a LateContext<'a, 'tcx>) -> Option<FxHashSet<HirId>> {
@ -23,7 +22,6 @@ pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr, cx: &'a LateContext<'a, 'tc
cx.param_env,
region_scope_tree,
cx.tables,
None,
)
.walk_expr(expr);
@ -66,21 +64,15 @@ impl<'tcx> MutVarsDelegate {
}
impl<'tcx> Delegate<'tcx> for MutVarsDelegate {
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, _: 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 {
self.update(&cmt.cat)
}
}
fn mutate(&mut self, _: HirId, _: Span, cmt: &cmt_<'tcx>, _: MutateMode) {
fn mutate(&mut self, cmt: &cmt_<'tcx>) {
self.update(&cmt.cat)
}
fn decl_without_init(&mut self, _: HirId, _: Span) {}
}

View File

@ -12,11 +12,5 @@ error: local variable doesn't need to be boxed here
LL | pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {}
| ^^^^^^^^^^^
error: local variable doesn't need to be boxed here
--> $DIR/escape_analysis.rs:170:23
|
LL | fn closure_borrow(x: Box<A>) {
| ^
error: aborting due to 3 previous errors
error: aborting due to 2 previous errors

View File

@ -2,7 +2,7 @@ error: attempt to mutate range bound within loop; note that the range of the loo
--> $DIR/mut_range_bound.rs:16:9
|
LL | m = 5;
| ^^^^^
| ^
|
= note: `-D clippy::mut-range-bound` implied by `-D warnings`
@ -10,19 +10,19 @@ error: attempt to mutate range bound within loop; note that the range of the loo
--> $DIR/mut_range_bound.rs:23:9
|
LL | m *= 2;
| ^^^^^^
| ^
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
--> $DIR/mut_range_bound.rs:31:9
|
LL | m = 5;
| ^^^^^
| ^
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
--> $DIR/mut_range_bound.rs:32:9
|
LL | n = 7;
| ^^^^^
| ^
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
--> $DIR/mut_range_bound.rs:46:22

View File

@ -28,12 +28,7 @@ error: this argument is passed by value, but not consumed in the function body
--> $DIR/needless_pass_by_value.rs:49:18
|
LL | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
| ^^^^^^^^^^^^^^^^^^^^^^
help: consider taking a reference instead
|
LL | fn test_match(x: &Option<Option<String>>, y: Option<Option<String>>) {
LL | match *x {
|
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&Option<Option<String>>`
error: this argument is passed by value, but not consumed in the function body
--> $DIR/needless_pass_by_value.rs:62:24
@ -45,14 +40,7 @@ error: this argument is passed by value, but not consumed in the function body
--> $DIR/needless_pass_by_value.rs:62:36
|
LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
| ^^^^^^^
help: consider taking a reference instead
|
LL | fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
LL | let Wrapper(s) = z; // moved
LL | let Wrapper(ref t) = *y; // not moved
LL | let Wrapper(_) = *y; // still not moved
|
| ^^^^^^^ help: consider taking a reference instead: `&Wrapper`
error: this argument is passed by value, but not consumed in the function body
--> $DIR/needless_pass_by_value.rs:78:49
@ -152,37 +140,25 @@ error: this argument is passed by value, but not consumed in the function body
--> $DIR/needless_pass_by_value.rs:131:45
|
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
| ^^^^^^^^^^^
| ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
|
help: consider marking this type as Copy
--> $DIR/needless_pass_by_value.rs:123:1
|
LL | struct CopyWrapper(u32);
| ^^^^^^^^^^^^^^^^^^^^^^^^
help: consider taking a reference instead
|
LL | fn test_destructure_copy(x: CopyWrapper, y: &CopyWrapper, z: CopyWrapper) {
LL | let CopyWrapper(s) = z; // moved
LL | let CopyWrapper(ref t) = *y; // not moved
LL | let CopyWrapper(_) = *y; // still not moved
|
error: this argument is passed by value, but not consumed in the function body
--> $DIR/needless_pass_by_value.rs:131:61
|
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
| ^^^^^^^^^^^
| ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
|
help: consider marking this type as Copy
--> $DIR/needless_pass_by_value.rs:123:1
|
LL | struct CopyWrapper(u32);
| ^^^^^^^^^^^^^^^^^^^^^^^^
help: consider taking a reference instead
|
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: &CopyWrapper) {
LL | let CopyWrapper(s) = *z; // moved
|
error: this argument is passed by value, but not consumed in the function body
--> $DIR/needless_pass_by_value.rs:143:40