Skip to content

Commit

Permalink
Cleanup path to local checks
Browse files Browse the repository at this point in the history
  • Loading branch information
camsteffen committed Feb 5, 2021
1 parent 357c6a7 commit 56f7fbb
Show file tree
Hide file tree
Showing 14 changed files with 180 additions and 301 deletions.
22 changes: 9 additions & 13 deletions clippy_lints/src/collapsible_match.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{span_lint_and_then, SpanlessEq};
use crate::utils::{path_to_local, span_lint_and_then, SpanlessEq};
use if_chain::if_chain;
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, QPath, StmtKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{DefIdTree, TyCtxt};
use rustc_middle::ty::{DefIdTree, TyCtxt, TypeckResults};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{MultiSpan, Span};

Expand Down Expand Up @@ -72,7 +72,7 @@ fn check_arm(arm: &Arm<'_>, wild_outer_arm: &Arm<'_>, cx: &LateContext<'_>) {
if arms_inner.iter().all(|arm| arm.guard.is_none());
// match expression must be a local binding
// match <local> { .. }
if let Some(binding_id) = addr_adjusted_binding(expr_in, cx);
if let Some(binding_id) = path_to_local(strip_ref_operators(expr_in, cx.typeck_results()));
// one of the branches must be "wild-like"
if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| arm_is_wild_like(arm_inner, cx.tcx));
let (wild_inner_arm, non_wild_inner_arm) =
Expand Down Expand Up @@ -175,19 +175,15 @@ fn is_none_ctor(res: Res, tcx: TyCtxt<'_>) -> bool {
false
}

/// Retrieves a binding ID with optional `&` and/or `*` operators removed. (e.g. `&**foo`)
/// Returns `None` if a non-reference type is de-referenced.
/// For example, if `Vec` is de-referenced to a slice, `None` is returned.
fn addr_adjusted_binding(mut expr: &Expr<'_>, cx: &LateContext<'_>) -> Option<HirId> {
/// Removes `AddrOf` operators (`&`) or deref operators (`*`), but only if a reference type is
/// dereferenced. An overloaded deref such as `Vec` to slice would not be removed.
fn strip_ref_operators<'hir>(mut expr: &'hir Expr<'hir>, typeck_results: &TypeckResults<'_>) -> &'hir Expr<'hir> {
loop {
match expr.kind {
ExprKind::AddrOf(_, _, e) => expr = e,
ExprKind::Path(QPath::Resolved(None, path)) => match path.res {
Res::Local(binding_id) => break Some(binding_id),
_ => break None,
},
ExprKind::Unary(UnOp::UnDeref, e) if cx.typeck_results().expr_ty(e).is_ref() => expr = e,
_ => break None,
ExprKind::Unary(UnOp::UnDeref, e) if typeck_results.expr_ty(e).is_ref() => expr = e,
_ => break,
}
}
expr
}
60 changes: 23 additions & 37 deletions clippy_lints/src/eval_order_dependence.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::utils::{get_parent_expr, span_lint, span_lint_and_note};
use if_chain::if_chain;
use crate::utils::{get_parent_expr, path_to_local, path_to_local_id, span_lint, span_lint_and_note};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{def, BinOpKind, Block, Expr, ExprKind, Guard, HirId, Local, Node, QPath, Stmt, StmtKind};
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Guard, HirId, Local, Node, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_middle::ty;
Expand Down Expand Up @@ -72,20 +71,14 @@ impl<'tcx> LateLintPass<'tcx> for EvalOrderDependence {
// Find a write to a local variable.
match expr.kind {
ExprKind::Assign(ref lhs, ..) | ExprKind::AssignOp(_, ref lhs, _) => {
if let ExprKind::Path(ref qpath) = lhs.kind {
if let QPath::Resolved(_, ref path) = *qpath {
if path.segments.len() == 1 {
if let def::Res::Local(var) = cx.qpath_res(qpath, lhs.hir_id) {
let mut visitor = ReadVisitor {
cx,
var,
write_expr: expr,
last_expr: expr,
};
check_for_unsequenced_reads(&mut visitor);
}
}
}
if let Some(var) = path_to_local(lhs) {
let mut visitor = ReadVisitor {
cx,
var,
write_expr: expr,
last_expr: expr,
};
check_for_unsequenced_reads(&mut visitor);
}
},
_ => {},
Expand Down Expand Up @@ -304,27 +297,20 @@ impl<'a, 'tcx> Visitor<'tcx> for ReadVisitor<'a, 'tcx> {
return;
}

match expr.kind {
ExprKind::Path(ref qpath) => {
if_chain! {
if let QPath::Resolved(None, ref path) = *qpath;
if path.segments.len() == 1;
if let def::Res::Local(local_id) = self.cx.qpath_res(qpath, expr.hir_id);
if local_id == self.var;
// Check that this is a read, not a write.
if !is_in_assignment_position(self.cx, expr);
then {
span_lint_and_note(
self.cx,
EVAL_ORDER_DEPENDENCE,
expr.span,
"unsequenced read of a variable",
Some(self.write_expr.span),
"whether read occurs before this write depends on evaluation order"
);
}
}
if path_to_local_id(expr, self.var) {
// Check that this is a read, not a write.
if !is_in_assignment_position(self.cx, expr) {
span_lint_and_note(
self.cx,
EVAL_ORDER_DEPENDENCE,
expr.span,
"unsequenced read of a variable",
Some(self.write_expr.span),
"whether read occurs before this write depends on evaluation order",
);
}
}
match expr.kind {
// We're about to descend a closure. Since we don't know when (or
// if) the closure will be evaluated, any reads in it might not
// occur here (or ever). Like above, bail to avoid false positives.
Expand Down
35 changes: 17 additions & 18 deletions clippy_lints/src/functions.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use crate::utils::{
attr_by_name, attrs::is_proc_macro, is_must_use_ty, is_trait_impl_item, is_type_diagnostic_item, iter_input_pats,
last_path_segment, match_def_path, must_use_attr, return_ty, snippet, snippet_opt, span_lint, span_lint_and_help,
span_lint_and_then, trait_ref_of_method, type_is_unsafe_function,
last_path_segment, match_def_path, must_use_attr, path_to_local, return_ty, snippet, snippet_opt, span_lint,
span_lint_and_help, span_lint_and_then, trait_ref_of_method, type_is_unsafe_function,
};
use if_chain::if_chain;
use rustc_ast::ast::Attribute;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::intravisit;
use rustc_hir::{def::Res, def_id::DefId};
use rustc_hir::{def::Res, def_id::DefId, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
Expand Down Expand Up @@ -658,16 +658,14 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> {

impl<'a, 'tcx> DerefVisitor<'a, 'tcx> {
fn check_arg(&self, ptr: &hir::Expr<'_>) {
if let hir::ExprKind::Path(ref qpath) = ptr.kind {
if let Res::Local(id) = self.cx.qpath_res(qpath, ptr.hir_id) {
if self.ptrs.contains(&id) {
span_lint(
self.cx,
NOT_UNSAFE_PTR_ARG_DEREF,
ptr.span,
"this public function dereferences a raw pointer but is not marked `unsafe`",
);
}
if let Some(id) = path_to_local(ptr) {
if self.ptrs.contains(&id) {
span_lint(
self.cx,
NOT_UNSAFE_PTR_ARG_DEREF,
ptr.span,
"this public function dereferences a raw pointer but is not marked `unsafe`",
);
}
}
}
Expand Down Expand Up @@ -698,7 +696,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> {
arg.span,
&mut tys,
)
&& is_mutated_static(self.cx, arg)
&& is_mutated_static(arg)
{
self.mutates_static = true;
return;
Expand All @@ -707,7 +705,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> {
}
},
Assign(ref target, ..) | AssignOp(_, ref target, _) | AddrOf(_, hir::Mutability::Mut, ref target) => {
self.mutates_static |= is_mutated_static(self.cx, target)
self.mutates_static |= is_mutated_static(target)
},
_ => {},
}
Expand All @@ -718,12 +716,13 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> {
}
}

fn is_mutated_static(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool {
fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
use hir::ExprKind::{Field, Index, Path};

match e.kind {
Path(ref qpath) => !matches!(cx.qpath_res(qpath, e.hir_id), Res::Local(_)),
Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(cx, inner),
Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)),
Path(_) => true,
Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(inner),
_ => false,
}
}
Expand Down
17 changes: 5 additions & 12 deletions clippy_lints/src/let_if_seq.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::utils::{snippet, span_lint_and_then, visitors::LocalUsedVisitor};
use crate::utils::{path_to_local_id, snippet, span_lint_and_then, visitors::LocalUsedVisitor};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::BindingAnnotation;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -66,7 +65,7 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
if let hir::ExprKind::If(ref cond, ref then, ref else_) = if_.kind;
if !LocalUsedVisitor::new(canonical_id).check_expr(cond);
if let hir::ExprKind::Block(ref then, _) = then.kind;
if let Some(value) = check_assign(cx, canonical_id, &*then);
if let Some(value) = check_assign(canonical_id, &*then);
if !LocalUsedVisitor::new(canonical_id).check_expr(value);
then {
let span = stmt.span.to(if_.span);
Expand All @@ -79,7 +78,7 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {

let (default_multi_stmts, default) = if let Some(ref else_) = *else_ {
if let hir::ExprKind::Block(ref else_, _) = else_.kind {
if let Some(default) = check_assign(cx, canonical_id, else_) {
if let Some(default) = check_assign(canonical_id, else_) {
(else_.stmts.len() > 1, default)
} else if let Some(ref default) = local.init {
(true, &**default)
Expand Down Expand Up @@ -134,19 +133,13 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
}
}

fn check_assign<'tcx>(
cx: &LateContext<'tcx>,
decl: hir::HirId,
block: &'tcx hir::Block<'_>,
) -> Option<&'tcx hir::Expr<'tcx>> {
fn check_assign<'tcx>(decl: hir::HirId, block: &'tcx hir::Block<'_>) -> Option<&'tcx hir::Expr<'tcx>> {
if_chain! {
if block.expr.is_none();
if let Some(expr) = block.stmts.iter().last();
if let hir::StmtKind::Semi(ref expr) = expr.kind;
if let hir::ExprKind::Assign(ref var, ref value, _) = expr.kind;
if let hir::ExprKind::Path(ref qpath) = var.kind;
if let Res::Local(local_id) = cx.qpath_res(qpath, var.hir_id);
if decl == local_id;
if path_to_local_id(var, decl);
then {
let mut v = LocalUsedVisitor::new(decl);

Expand Down
Loading

0 comments on commit 56f7fbb

Please sign in to comment.