Skip to content

Commit b1a2bd9

Browse files
compiler-errorscompiler-errors
committed
Be more thorough in expr_constitutes_read
1 parent 1addbed commit b1a2bd9

File tree

2 files changed

+140
-42
lines changed

2 files changed

+140
-42
lines changed

‎compiler/rustc_hir_typeck/src/expr.rs

+139-41
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// ignore-tidy-filelength
2+
// FIXME: we should move the field error reporting code somewhere else.
3+
14
//! Type checking expressions.
25
//!
36
//! See [`rustc_hir_analysis::check`] for more context on type checking in general.
@@ -284,54 +287,149 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
284287
return true;
285288
}
286289

287-
// If this expression has any adjustments applied after the place expression,
288-
// they may constitute reads.
289-
if !self.typeck_results.borrow().expr_adjustments(expr).is_empty() {
290-
return true;
291-
}
290+
let parent_node = self.tcx.parent_hir_node(expr.hir_id);
291+
match parent_node {
292+
hir::Node::Expr(parent_expr) => {
293+
match parent_expr.kind {
294+
// Addr-of, field projections, and LHS of assignment don't constitute reads.
295+
// Assignment does call `drop_in_place`, though, but its safety
296+
// requirements are not the same.
297+
ExprKind::AddrOf(..) | hir::ExprKind::Field(..) => false,
298+
ExprKind::Assign(lhs, _, _) => {
299+
// Only the LHS does not constitute a read
300+
expr.hir_id != lhs.hir_id
301+
}
292302

293-
fn pat_does_read(pat: &hir::Pat<'_>) -> bool {
294-
let mut does_read = false;
295-
pat.walk(|pat| {
296-
if matches!(
297-
pat.kind,
298-
hir::PatKind::Wild | hir::PatKind::Never | hir::PatKind::Or(_)
299-
) {
300-
true
301-
} else {
302-
does_read = true;
303-
// No need to continue.
304-
false
305-
}
306-
});
307-
does_read
308-
}
303+
// If we have a subpattern that performs a read, we want to consider this
304+
// to diverge for compatibility to support something like `let x: () = *never_ptr;`.
305+
ExprKind::Match(scrutinee, arms, _) => {
306+
assert_eq!(scrutinee.hir_id, expr.hir_id);
307+
arms.iter().any(|arm| self.pat_constitutes_read(arm.pat))
308+
}
309+
ExprKind::Let(hir::LetExpr { init, pat, .. }) => {
310+
assert_eq!(init.hir_id, expr.hir_id);
311+
self.pat_constitutes_read(*pat)
312+
}
309313

310-
match self.tcx.parent_hir_node(expr.hir_id) {
311-
// Addr-of, field projections, and LHS of assignment don't constitute reads.
312-
// Assignment does call `drop_in_place`, though, but its safety
313-
// requirements are not the same.
314-
hir::Node::Expr(hir::Expr { kind: hir::ExprKind::AddrOf(..), .. }) => false,
315-
hir::Node::Expr(hir::Expr {
316-
kind: hir::ExprKind::Assign(target, _, _) | hir::ExprKind::Field(target, _),
317-
..
318-
}) if expr.hir_id == target.hir_id => false,
314+
// Any expression child of these expressions constitute reads.
315+
ExprKind::Array(_)
316+
| ExprKind::Call(_, _)
317+
| ExprKind::MethodCall(_, _, _, _)
318+
| ExprKind::Tup(_)
319+
| ExprKind::Binary(_, _, _)
320+
| ExprKind::Unary(_, _)
321+
| ExprKind::Cast(_, _)
322+
| ExprKind::Type(_, _)
323+
| ExprKind::DropTemps(_)
324+
| ExprKind::If(_, _, _)
325+
| ExprKind::Closure(_)
326+
| ExprKind::Block(_, _)
327+
| ExprKind::AssignOp(_, _, _)
328+
| ExprKind::Index(_, _, _)
329+
| ExprKind::Break(_, _)
330+
| ExprKind::Ret(_)
331+
| ExprKind::Become(_)
332+
| ExprKind::InlineAsm(_)
333+
| ExprKind::Struct(_, _, _)
334+
| ExprKind::Repeat(_, _)
335+
| ExprKind::Yield(_, _) => true,
336+
337+
// These expressions have no (direct) sub-exprs.
338+
ExprKind::ConstBlock(_)
339+
| ExprKind::Loop(_, _, _, _)
340+
| ExprKind::Lit(_)
341+
| ExprKind::Path(_)
342+
| ExprKind::Continue(_)
343+
| ExprKind::OffsetOf(_, _)
344+
| ExprKind::Err(_) => unreachable!("no sub-expr expected for {:?}", expr.kind),
345+
}
346+
}
319347

320348
// If we have a subpattern that performs a read, we want to consider this
321349
// to diverge for compatibility to support something like `let x: () = *never_ptr;`.
322-
hir::Node::LetStmt(hir::LetStmt { init: Some(target), pat, .. })
323-
| hir::Node::Expr(hir::Expr {
324-
kind: hir::ExprKind::Let(hir::LetExpr { init: target, pat, .. }),
350+
hir::Node::LetStmt(hir::LetStmt { init: Some(target), pat, .. }) => {
351+
assert_eq!(target.hir_id, expr.hir_id);
352+
self.pat_constitutes_read(*pat)
353+
}
354+
355+
// These nodes (if they have a sub-expr) do constitute a read.
356+
hir::Node::Block(_)
357+
| hir::Node::Arm(_)
358+
| hir::Node::ExprField(_)
359+
| hir::Node::AnonConst(_)
360+
| hir::Node::ConstBlock(_)
361+
| hir::Node::ConstArg(_)
362+
| hir::Node::Stmt(_)
363+
| hir::Node::Item(hir::Item {
364+
kind: hir::ItemKind::Const(..) | hir::ItemKind::Static(..),
325365
..
326-
}) if expr.hir_id == target.hir_id && !pat_does_read(*pat) => false,
327-
hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Match(target, arms, _), .. })
328-
if expr.hir_id == target.hir_id
329-
&& !arms.iter().any(|arm| pat_does_read(arm.pat)) =>
330-
{
331-
false
366+
})
367+
| hir::Node::TraitItem(hir::TraitItem {
368+
kind: hir::TraitItemKind::Const(..), ..
369+
})
370+
| hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Const(..), .. }) => true,
371+
372+
// These nodes do not have direct sub-exprs.
373+
hir::Node::Param(_)
374+
| hir::Node::Item(_)
375+
| hir::Node::ForeignItem(_)
376+
| hir::Node::TraitItem(_)
377+
| hir::Node::ImplItem(_)
378+
| hir::Node::Variant(_)
379+
| hir::Node::Field(_)
380+
| hir::Node::PathSegment(_)
381+
| hir::Node::Ty(_)
382+
| hir::Node::AssocItemConstraint(_)
383+
| hir::Node::TraitRef(_)
384+
| hir::Node::Pat(_)
385+
| hir::Node::PatField(_)
386+
| hir::Node::LetStmt(_)
387+
| hir::Node::Synthetic
388+
| hir::Node::Err(_)
389+
| hir::Node::Ctor(_)
390+
| hir::Node::Lifetime(_)
391+
| hir::Node::GenericParam(_)
392+
| hir::Node::Crate(_)
393+
| hir::Node::Infer(_)
394+
| hir::Node::WhereBoundPredicate(_)
395+
| hir::Node::ArrayLenInfer(_)
396+
| hir::Node::PreciseCapturingNonLifetimeArg(_) => {
397+
unreachable!("no sub-expr expected for {parent_node:?}")
398+
}
399+
}
400+
}
401+
402+
/// Whether this pattern constitutes a read of value of the scrutinee that
403+
/// it is matching against.
404+
///
405+
/// See above for the nuances of what happens when this returns true.
406+
pub(super) fn pat_constitutes_read(&self, pat: &hir::Pat<'_>) -> bool {
407+
let mut does_read = false;
408+
pat.walk(|pat| {
409+
match pat.kind {
410+
hir::PatKind::Wild | hir::PatKind::Never | hir::PatKind::Or(_) => {
411+
// Recurse
412+
true
413+
}
414+
hir::PatKind::Binding(_, _, _, _)
415+
| hir::PatKind::Struct(_, _, _)
416+
| hir::PatKind::TupleStruct(_, _, _)
417+
| hir::PatKind::Path(_)
418+
| hir::PatKind::Tuple(_, _)
419+
| hir::PatKind::Box(_)
420+
| hir::PatKind::Ref(_, _)
421+
| hir::PatKind::Deref(_)
422+
| hir::PatKind::Lit(_)
423+
| hir::PatKind::Range(_, _, _)
424+
| hir::PatKind::Slice(_, _, _)
425+
| hir::PatKind::Err(_) => {
426+
does_read = true;
427+
// No need to continue.
428+
false
429+
}
332430
}
333-
_ => true,
334-
}
431+
});
432+
does_read
335433
}
336434

337435
#[instrument(skip(self, expr), level = "debug")]

‎compiler/rustc_hir_typeck/src/pat.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
280280

281281
// All other patterns constitute a read, which causes us to diverge
282282
// if the type is never.
283-
if ty.is_never() && !matches!(pat.kind, PatKind::Wild | PatKind::Never | PatKind::Or(_)) {
283+
if ty.is_never() && self.pat_constitutes_read(pat) {
284284
self.diverges.set(self.diverges.get() | Diverges::always(pat.span));
285285
}
286286

0 commit comments

Comments
 (0)