Skip to content

Commit

Permalink
expr_use_ctxt changes:
Browse files Browse the repository at this point in the history
* Delay the parsing of the use node
* Mark when the `SyntaxContext` changes rather than return `None`
* Return a default value if the HIR tree is broken rather than `None`
  • Loading branch information
Jarcho committed Jun 16, 2024
1 parent 9f5d60f commit a002f93
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 102 deletions.
4 changes: 2 additions & 2 deletions clippy_lints/src/casts/ref_as_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ pub(super) fn check<'tcx>(

if matches!(cast_from.kind(), ty::Ref(..))
&& let ty::RawPtr(_, to_mutbl) = cast_to.kind()
&& let Some(use_cx) = expr_use_ctxt(cx, expr)
&& let use_cx = expr_use_ctxt(cx, expr)
// TODO: only block the lint if `cast_expr` is a temporary
&& !matches!(use_cx.node, ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
&& !matches!(use_cx.use_node(cx), ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
{
let core_or_std = if is_no_std_crate(cx) { "core" } else { "std" };
let fn_name = match to_mutbl {
Expand Down
46 changes: 20 additions & 26 deletions clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,18 +260,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
(None, kind) => {
let expr_ty = typeck.expr_ty(expr);
let use_cx = expr_use_ctxt(cx, expr);
let adjusted_ty = match &use_cx {
Some(use_cx) => match use_cx.adjustments {
[.., a] => a.target,
_ => expr_ty,
},
_ => typeck.expr_ty_adjusted(expr),
};
let adjusted_ty = use_cx.adjustments.last().map_or(expr_ty, |a| a.target);

match (use_cx, kind) {
(Some(use_cx), RefOp::Deref) => {
match kind {
RefOp::Deref if use_cx.same_ctxt => {
let use_node = use_cx.use_node(cx);
let sub_ty = typeck.expr_ty(sub_expr);
if let ExprUseNode::FieldAccess(name) = use_cx.node
if let ExprUseNode::FieldAccess(name) = use_node
&& !use_cx.moved_before_use
&& !ty_contains_field(sub_ty, name.name)
{
Expand All @@ -288,9 +283,9 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
} else if sub_ty.is_ref()
// Linting method receivers would require verifying that name lookup
// would resolve the same way. This is complicated by trait methods.
&& !use_cx.node.is_recv()
&& let Some(ty) = use_cx.node.defined_ty(cx)
&& TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()).is_deref_stable()
&& !use_node.is_recv()
&& let Some(ty) = use_node.defined_ty(cx)
&& TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return()).is_deref_stable()
{
self.state = Some((
State::ExplicitDeref { mutability: None },
Expand All @@ -301,7 +296,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
));
}
},
(_, RefOp::Method { mutbl, is_ufcs })
RefOp::Method { mutbl, is_ufcs }
if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id)
// Allow explicit deref in method chains. e.g. `foo.deref().bar()`
&& (is_ufcs || !in_postfix_position(cx, expr)) =>
Expand All @@ -319,7 +314,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
},
));
},
(Some(use_cx), RefOp::AddrOf(mutability)) => {
RefOp::AddrOf(mutability) if use_cx.same_ctxt => {
// Find the number of times the borrow is auto-derefed.
let mut iter = use_cx.adjustments.iter();
let mut deref_count = 0usize;
Expand All @@ -338,10 +333,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
};
};

let stability = use_cx.node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return())
let use_node = use_cx.use_node(cx);
let stability = use_node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return())
});
let can_auto_borrow = match use_cx.node {
let can_auto_borrow = match use_node {
ExprUseNode::FieldAccess(_)
if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) =>
{
Expand All @@ -353,7 +349,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
// deref through `ManuallyDrop<_>` will not compile.
!adjust_derefs_manually_drop(use_cx.adjustments, expr_ty)
},
ExprUseNode::Callee | ExprUseNode::FieldAccess(_) => true,
ExprUseNode::Callee | ExprUseNode::FieldAccess(_) if !use_cx.moved_before_use => true,
ExprUseNode::MethodArg(hir_id, _, 0) if !use_cx.moved_before_use => {
// Check for calls to trait methods where the trait is implemented
// on a reference.
Expand All @@ -363,9 +359,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
// priority.
if let Some(fn_id) = typeck.type_dependent_def_id(hir_id)
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
&& let arg_ty = cx
.tcx
.erase_regions(use_cx.adjustments.last().map_or(expr_ty, |a| a.target))
&& let arg_ty = cx.tcx.erase_regions(adjusted_ty)
&& let ty::Ref(_, sub_ty, _) = *arg_ty.kind()
&& let args =
typeck.node_args_opt(hir_id).map(|args| &args[1..]).unwrap_or_default()
Expand Down Expand Up @@ -443,7 +437,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
count: deref_count - required_refs,
msg,
stability,
for_field_access: if let ExprUseNode::FieldAccess(name) = use_cx.node
for_field_access: if let ExprUseNode::FieldAccess(name) = use_node
&& !use_cx.moved_before_use
{
Some(name.name)
Expand All @@ -453,7 +447,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
}),
StateData {
first_expr: expr,
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
adjusted_ty,
},
));
} else if stability.is_deref_stable()
Expand All @@ -465,12 +459,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
State::Borrow { mutability },
StateData {
first_expr: expr,
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
adjusted_ty,
},
));
}
},
(None, _) | (_, RefOp::Method { .. }) => (),
_ => {},
}
},
(
Expand Down
8 changes: 5 additions & 3 deletions clippy_lints/src/needless_borrows_for_generic_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowsForGenericArgs<'tcx> {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if matches!(expr.kind, ExprKind::AddrOf(..))
&& !expr.span.from_expansion()
&& let Some(use_cx) = expr_use_ctxt(cx, expr)
&& let use_cx = expr_use_ctxt(cx, expr)
&& use_cx.same_ctxt
&& !use_cx.is_ty_unified
&& let Some(DefinedTy::Mir(ty)) = use_cx.node.defined_ty(cx)
&& let use_node = use_cx.use_node(cx)
&& let Some(DefinedTy::Mir(ty)) = use_node.defined_ty(cx)
&& let ty::Param(ty) = *ty.value.skip_binder().kind()
&& let Some((hir_id, fn_id, i)) = match use_cx.node {
&& let Some((hir_id, fn_id, i)) = match use_node {
ExprUseNode::MethodArg(_, _, 0) => None,
ExprUseNode::MethodArg(hir_id, None, i) => cx
.typeck_results()
Expand Down
164 changes: 93 additions & 71 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![feature(array_chunks)]
#![feature(box_patterns)]
#![feature(control_flow_enum)]
#![feature(exhaustive_patterns)]
#![feature(if_let_guard)]
#![feature(let_chains)]
#![feature(lint_reasons)]
Expand Down Expand Up @@ -2664,13 +2665,80 @@ pub enum DefinedTy<'tcx> {
/// The context an expressions value is used in.
pub struct ExprUseCtxt<'tcx> {
/// The parent node which consumes the value.
pub node: ExprUseNode<'tcx>,
pub node: Node<'tcx>,
/// The child id of the node the value came from.
pub child_id: HirId,
/// Any adjustments applied to the type.
pub adjustments: &'tcx [Adjustment<'tcx>],
/// Whether or not the type must unify with another code path.
/// Whether the type must unify with another code path.
pub is_ty_unified: bool,
/// Whether or not the value will be moved before it's used.
/// Whether the value will be moved before it's used.
pub moved_before_use: bool,
/// Whether the use site has the same `SyntaxContext` as the value.
pub same_ctxt: bool,
}
impl<'tcx> ExprUseCtxt<'tcx> {
pub fn use_node(&self, cx: &LateContext<'tcx>) -> ExprUseNode<'tcx> {
match self.node {
Node::LetStmt(l) => ExprUseNode::LetStmt(l),
Node::ExprField(field) => ExprUseNode::Field(field),

Node::Item(&Item {
kind: ItemKind::Static(..) | ItemKind::Const(..),
owner_id,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Const(..),
owner_id,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Const(..),
owner_id,
..
}) => ExprUseNode::ConstStatic(owner_id),

Node::Item(&Item {
kind: ItemKind::Fn(..),
owner_id,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Fn(..),
owner_id,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Fn(..),
owner_id,
..
}) => ExprUseNode::Return(owner_id),

Node::Expr(use_expr) => match use_expr.kind {
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
}),

ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == self.child_id) {
Some(i) => ExprUseNode::FnArg(func, i),
None => ExprUseNode::Callee,
},
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
use_expr.hir_id,
name.args,
args.iter()
.position(|arg| arg.hir_id == self.child_id)
.map_or(0, |i| i + 1),
),
ExprKind::Field(_, name) => ExprUseNode::FieldAccess(name),
ExprKind::AddrOf(kind, mutbl, _) => ExprUseNode::AddrOf(kind, mutbl),
_ => ExprUseNode::Other,
},
_ => ExprUseNode::Other,
}
}
}

/// The node which consumes a value.
Expand All @@ -2691,7 +2759,8 @@ pub enum ExprUseNode<'tcx> {
Callee,
/// Access of a field.
FieldAccess(Ident),
Expr,
/// Borrow expression.
AddrOf(ast::BorrowKind, Mutability),
Other,
}
impl<'tcx> ExprUseNode<'tcx> {
Expand Down Expand Up @@ -2768,26 +2837,25 @@ impl<'tcx> ExprUseNode<'tcx> {
let sig = cx.tcx.fn_sig(id).skip_binder();
Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i))))
},
Self::LetStmt(_) | Self::FieldAccess(..) | Self::Callee | Self::Expr | Self::Other => None,
Self::LetStmt(_) | Self::FieldAccess(..) | Self::Callee | Self::Other | Self::AddrOf(..) => None,
}
}
}

/// Gets the context an expression's value is used in.
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<ExprUseCtxt<'tcx>> {
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> ExprUseCtxt<'tcx> {
let mut adjustments = [].as_slice();
let mut is_ty_unified = false;
let mut moved_before_use = false;
let mut same_ctxt = true;
let ctxt = e.span.ctxt();
walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| {
let node = walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| -> ControlFlow<!> {
if adjustments.is_empty()
&& let Node::Expr(e) = cx.tcx.hir_node(child_id)
{
adjustments = cx.typeck_results().expr_adjustments(e);
}
if cx.tcx.hir().span(parent_id).ctxt() != ctxt {
return ControlFlow::Break(());
}
same_ctxt &= cx.tcx.hir().span(parent_id).ctxt() == ctxt;
if let Node::Expr(e) = parent {
match e.kind {
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
Expand All @@ -2803,71 +2871,25 @@ pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Optio
}
}
ControlFlow::Continue(())
})?
.continue_value()
.map(|(use_node, child_id)| {
let node = match use_node {
Node::LetStmt(l) => ExprUseNode::LetStmt(l),
Node::ExprField(field) => ExprUseNode::Field(field),

Node::Item(&Item {
kind: ItemKind::Static(..) | ItemKind::Const(..),
owner_id,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Const(..),
owner_id,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Const(..),
owner_id,
..
}) => ExprUseNode::ConstStatic(owner_id),

Node::Item(&Item {
kind: ItemKind::Fn(..),
owner_id,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Fn(..),
owner_id,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Fn(..),
owner_id,
..
}) => ExprUseNode::Return(owner_id),

Node::Expr(use_expr) => match use_expr.kind {
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
}),
ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == child_id) {
Some(i) => ExprUseNode::FnArg(func, i),
None => ExprUseNode::Callee,
},
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
use_expr.hir_id,
name.args,
args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
),
ExprKind::Field(child, name) if child.hir_id == e.hir_id => ExprUseNode::FieldAccess(name),
_ => ExprUseNode::Expr,
},
_ => ExprUseNode::Other,
};
ExprUseCtxt {
});
match node {
Some(ControlFlow::Continue((node, child_id))) => ExprUseCtxt {
node,
child_id,
adjustments,
is_ty_unified,
moved_before_use,
}
})
same_ctxt,
},
None => ExprUseCtxt {
node: Node::Crate(cx.tcx.hir().root_module()),
child_id: HirId::INVALID,
adjustments: &[],
is_ty_unified: true,
moved_before_use: true,
same_ctxt: false,
},
}
}

/// Tokenizes the input while keeping the text associated with each token.
Expand Down

0 comments on commit a002f93

Please sign in to comment.