expr_use_ctxt changes:

* 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`
This commit is contained in:
Jason Newcomb 2024-02-12 18:33:10 -05:00
parent 9f5d60f160
commit a002f93e51
4 changed files with 120 additions and 102 deletions

View File

@ -22,9 +22,9 @@ pub(super) fn check<'tcx>(
if matches!(cast_from.kind(), ty::Ref(..)) if matches!(cast_from.kind(), ty::Ref(..))
&& let ty::RawPtr(_, to_mutbl) = cast_to.kind() && 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 // 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 core_or_std = if is_no_std_crate(cx) { "core" } else { "std" };
let fn_name = match to_mutbl { let fn_name = match to_mutbl {

View File

@ -260,18 +260,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
(None, kind) => { (None, kind) => {
let expr_ty = typeck.expr_ty(expr); let expr_ty = typeck.expr_ty(expr);
let use_cx = expr_use_ctxt(cx, expr); let use_cx = expr_use_ctxt(cx, expr);
let adjusted_ty = match &use_cx { let adjusted_ty = use_cx.adjustments.last().map_or(expr_ty, |a| a.target);
Some(use_cx) => match use_cx.adjustments {
[.., a] => a.target,
_ => expr_ty,
},
_ => typeck.expr_ty_adjusted(expr),
};
match (use_cx, kind) { match kind {
(Some(use_cx), RefOp::Deref) => { RefOp::Deref if use_cx.same_ctxt => {
let use_node = use_cx.use_node(cx);
let sub_ty = typeck.expr_ty(sub_expr); 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 && !use_cx.moved_before_use
&& !ty_contains_field(sub_ty, name.name) && !ty_contains_field(sub_ty, name.name)
{ {
@ -288,9 +283,9 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
} else if sub_ty.is_ref() } else if sub_ty.is_ref()
// Linting method receivers would require verifying that name lookup // Linting method receivers would require verifying that name lookup
// would resolve the same way. This is complicated by trait methods. // would resolve the same way. This is complicated by trait methods.
&& !use_cx.node.is_recv() && !use_node.is_recv()
&& let Some(ty) = use_cx.node.defined_ty(cx) && let Some(ty) = use_node.defined_ty(cx)
&& TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()).is_deref_stable() && TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return()).is_deref_stable()
{ {
self.state = Some(( self.state = Some((
State::ExplicitDeref { mutability: None }, State::ExplicitDeref { mutability: None },
@ -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) if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id)
// Allow explicit deref in method chains. e.g. `foo.deref().bar()` // Allow explicit deref in method chains. e.g. `foo.deref().bar()`
&& (is_ufcs || !in_postfix_position(cx, expr)) => && (is_ufcs || !in_postfix_position(cx, expr)) =>
@ -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. // Find the number of times the borrow is auto-derefed.
let mut iter = use_cx.adjustments.iter(); let mut iter = use_cx.adjustments.iter();
let mut deref_count = 0usize; let mut deref_count = 0usize;
@ -338,10 +333,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
}; };
}; };
let stability = use_cx.node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| { let use_node = use_cx.use_node(cx);
TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()) 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(_) ExprUseNode::FieldAccess(_)
if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) => if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) =>
{ {
@ -353,7 +349,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
// deref through `ManuallyDrop<_>` will not compile. // deref through `ManuallyDrop<_>` will not compile.
!adjust_derefs_manually_drop(use_cx.adjustments, expr_ty) !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 => { ExprUseNode::MethodArg(hir_id, _, 0) if !use_cx.moved_before_use => {
// Check for calls to trait methods where the trait is implemented // Check for calls to trait methods where the trait is implemented
// on a reference. // on a reference.
@ -363,9 +359,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
// priority. // priority.
if let Some(fn_id) = typeck.type_dependent_def_id(hir_id) if let Some(fn_id) = typeck.type_dependent_def_id(hir_id)
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id) && let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
&& let arg_ty = cx && let arg_ty = cx.tcx.erase_regions(adjusted_ty)
.tcx
.erase_regions(use_cx.adjustments.last().map_or(expr_ty, |a| a.target))
&& let ty::Ref(_, sub_ty, _) = *arg_ty.kind() && let ty::Ref(_, sub_ty, _) = *arg_ty.kind()
&& let args = && let args =
typeck.node_args_opt(hir_id).map(|args| &args[1..]).unwrap_or_default() typeck.node_args_opt(hir_id).map(|args| &args[1..]).unwrap_or_default()
@ -443,7 +437,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
count: deref_count - required_refs, count: deref_count - required_refs,
msg, msg,
stability, 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 && !use_cx.moved_before_use
{ {
Some(name.name) Some(name.name)
@ -453,7 +447,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
}), }),
StateData { StateData {
first_expr: expr, first_expr: expr,
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target), adjusted_ty,
}, },
)); ));
} else if stability.is_deref_stable() } else if stability.is_deref_stable()
@ -465,12 +459,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
State::Borrow { mutability }, State::Borrow { mutability },
StateData { StateData {
first_expr: expr, first_expr: expr,
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target), adjusted_ty,
}, },
)); ));
} }
}, },
(None, _) | (_, RefOp::Method { .. }) => (), _ => {},
} }
}, },
( (

View File

@ -80,11 +80,13 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowsForGenericArgs<'tcx> {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if matches!(expr.kind, ExprKind::AddrOf(..)) if matches!(expr.kind, ExprKind::AddrOf(..))
&& !expr.span.from_expansion() && !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 && !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 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(_, _, 0) => None,
ExprUseNode::MethodArg(hir_id, None, i) => cx ExprUseNode::MethodArg(hir_id, None, i) => cx
.typeck_results() .typeck_results()

View File

@ -1,6 +1,7 @@
#![feature(array_chunks)] #![feature(array_chunks)]
#![feature(box_patterns)] #![feature(box_patterns)]
#![feature(control_flow_enum)] #![feature(control_flow_enum)]
#![feature(exhaustive_patterns)]
#![feature(if_let_guard)] #![feature(if_let_guard)]
#![feature(let_chains)] #![feature(let_chains)]
#![feature(lint_reasons)] #![feature(lint_reasons)]
@ -2664,13 +2665,80 @@ pub enum DefinedTy<'tcx> {
/// The context an expressions value is used in. /// The context an expressions value is used in.
pub struct ExprUseCtxt<'tcx> { pub struct ExprUseCtxt<'tcx> {
/// The parent node which consumes the value. /// 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. /// Any adjustments applied to the type.
pub adjustments: &'tcx [Adjustment<'tcx>], 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, 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, 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. /// The node which consumes a value.
@ -2691,7 +2759,8 @@ pub enum ExprUseNode<'tcx> {
Callee, Callee,
/// Access of a field. /// Access of a field.
FieldAccess(Ident), FieldAccess(Ident),
Expr, /// Borrow expression.
AddrOf(ast::BorrowKind, Mutability),
Other, Other,
} }
impl<'tcx> ExprUseNode<'tcx> { impl<'tcx> ExprUseNode<'tcx> {
@ -2768,26 +2837,25 @@ impl<'tcx> ExprUseNode<'tcx> {
let sig = cx.tcx.fn_sig(id).skip_binder(); let sig = cx.tcx.fn_sig(id).skip_binder();
Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i)))) 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. /// 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 adjustments = [].as_slice();
let mut is_ty_unified = false; let mut is_ty_unified = false;
let mut moved_before_use = false; let mut moved_before_use = false;
let mut same_ctxt = true;
let ctxt = e.span.ctxt(); 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() if adjustments.is_empty()
&& let Node::Expr(e) = cx.tcx.hir_node(child_id) && let Node::Expr(e) = cx.tcx.hir_node(child_id)
{ {
adjustments = cx.typeck_results().expr_adjustments(e); adjustments = cx.typeck_results().expr_adjustments(e);
} }
if cx.tcx.hir().span(parent_id).ctxt() != ctxt { same_ctxt &= cx.tcx.hir().span(parent_id).ctxt() == ctxt;
return ControlFlow::Break(());
}
if let Node::Expr(e) = parent { if let Node::Expr(e) = parent {
match e.kind { match e.kind {
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => { ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
@ -2803,71 +2871,25 @@ pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Optio
} }
} }
ControlFlow::Continue(()) ControlFlow::Continue(())
})? });
.continue_value() match node {
.map(|(use_node, child_id)| { Some(ControlFlow::Continue((node, child_id))) => ExprUseCtxt {
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 {
node, node,
child_id,
adjustments, adjustments,
is_ty_unified, is_ty_unified,
moved_before_use, 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. /// Tokenizes the input while keeping the text associated with each token.