Skip to content

Commit 61a738b

Browse files
compiler-errorscompiler-errors
committed
Make if_cause less expensive
1 parent 0d6ab20 commit 61a738b

File tree

15 files changed

+201
-264
lines changed

15 files changed

+201
-264
lines changed

‎compiler/rustc_hir_typeck/src/_match.rs‎

Lines changed: 5 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use rustc_errors::{Applicability, Diag};
22
use rustc_hir::def::{CtorOf, DefKind, Res};
33
use rustc_hir::def_id::LocalDefId;
4-
use rustc_hir::{self as hir, ExprKind, PatKind};
4+
use rustc_hir::{self as hir, ExprKind, HirId, PatKind};
55
use rustc_hir_pretty::ty_to_string;
66
use rustc_middle::ty::{self, Ty};
77
use rustc_span::Span;
88
use rustc_trait_selection::traits::{
9-
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
9+
MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
1010
};
1111
use tracing::{debug, instrument};
1212

@@ -414,105 +414,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
414414

415415
pub(crate) fn if_cause(
416416
&self,
417-
span: Span,
418-
cond_span: Span,
419-
then_expr: &'tcx hir::Expr<'tcx>,
417+
expr_id: HirId,
420418
else_expr: &'tcx hir::Expr<'tcx>,
421-
then_ty: Ty<'tcx>,
422-
else_ty: Ty<'tcx>,
423419
tail_defines_return_position_impl_trait: Option<LocalDefId>,
424420
) -> ObligationCause<'tcx> {
425-
let mut outer_span = if self.tcx.sess.source_map().is_multiline(span) {
426-
// The `if`/`else` isn't in one line in the output, include some context to make it
427-
// clear it is an if/else expression:
428-
// ```
429-
// LL | let x = if true {
430-
// | _____________-
431-
// LL || 10i32
432-
// || ----- expected because of this
433-
// LL || } else {
434-
// LL || 10u32
435-
// || ^^^^^ expected `i32`, found `u32`
436-
// LL || };
437-
// ||_____- `if` and `else` have incompatible types
438-
// ```
439-
Some(span)
440-
} else {
441-
// The entire expression is in one line, only point at the arms
442-
// ```
443-
// LL | let x = if true { 10i32 } else { 10u32 };
444-
// | ----- ^^^^^ expected `i32`, found `u32`
445-
// | |
446-
// | expected because of this
447-
// ```
448-
None
449-
};
450-
451-
let (error_sp, else_id) = if let ExprKind::Block(block, _) = &else_expr.kind {
452-
let block = block.innermost_block();
453-
454-
// Avoid overlapping spans that aren't as readable:
455-
// ```
456-
// 2 | let x = if true {
457-
// | _____________-
458-
// 3 | | 3
459-
// | | - expected because of this
460-
// 4 | | } else {
461-
// | |____________^
462-
// 5 | ||
463-
// 6 | || };
464-
// | || ^
465-
// | ||_____|
466-
// | |______if and else have incompatible types
467-
// | expected integer, found `()`
468-
// ```
469-
// by not pointing at the entire expression:
470-
// ```
471-
// 2 | let x = if true {
472-
// | ------- `if` and `else` have incompatible types
473-
// 3 | 3
474-
// | - expected because of this
475-
// 4 | } else {
476-
// | ____________^
477-
// 5 | |
478-
// 6 | | };
479-
// | |_____^ expected integer, found `()`
480-
// ```
481-
if block.expr.is_none()
482-
&& block.stmts.is_empty()
483-
&& let Some(outer_span) = &mut outer_span
484-
&& let Some(cond_span) = cond_span.find_ancestor_inside(*outer_span)
485-
{
486-
*outer_span = outer_span.with_hi(cond_span.hi())
487-
}
488-
489-
(self.find_block_span(block), block.hir_id)
490-
} else {
491-
(else_expr.span, else_expr.hir_id)
492-
};
493-
494-
let then_id = if let ExprKind::Block(block, _) = &then_expr.kind {
495-
let block = block.innermost_block();
496-
// Exclude overlapping spans
497-
if block.expr.is_none() && block.stmts.is_empty() {
498-
outer_span = None;
499-
}
500-
block.hir_id
501-
} else {
502-
then_expr.hir_id
503-
};
421+
let error_sp = self.find_block_span_from_hir_id(else_expr.hir_id);
504422

505423
// Finally construct the cause:
506424
self.cause(
507425
error_sp,
508-
ObligationCauseCode::IfExpression(Box::new(IfExpressionCause {
509-
else_id,
510-
then_id,
511-
then_ty,
512-
else_ty,
513-
outer_span,
514-
tail_defines_return_position_impl_trait,
515-
})),
426+
ObligationCauseCode::IfExpression { expr_id, tail_defines_return_position_impl_trait },
516427
)
517428
}
518429

‎compiler/rustc_hir_typeck/src/coercion.rs‎

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
4646
use rustc_infer::infer::relate::RelateResult;
4747
use rustc_infer::infer::{Coercion, DefineOpaqueTypes, InferOk, InferResult};
4848
use rustc_infer::traits::{
49-
IfExpressionCause, MatchExpressionArmCause, Obligation, PredicateObligation,
50-
PredicateObligations, SelectionError,
49+
MatchExpressionArmCause, Obligation, PredicateObligation, PredicateObligations, SelectionError,
5150
};
5251
use rustc_middle::span_bug;
5352
use rustc_middle::ty::adjustment::{
@@ -1710,39 +1709,30 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
17101709
);
17111710
}
17121711
}
1713-
ObligationCauseCode::IfExpression(box IfExpressionCause {
1714-
then_id,
1715-
else_id,
1716-
then_ty,
1717-
else_ty,
1712+
ObligationCauseCode::IfExpression {
1713+
expr_id,
17181714
tail_defines_return_position_impl_trait: Some(rpit_def_id),
1719-
..
1720-
}) => {
1715+
} => {
1716+
let hir::Node::Expr(hir::Expr {
1717+
kind: hir::ExprKind::If(_, then_expr, Some(else_expr)),
1718+
..
1719+
}) = fcx.tcx.hir_node(expr_id)
1720+
else {
1721+
unreachable!();
1722+
};
17211723
err = fcx.err_ctxt().report_mismatched_types(
17221724
cause,
17231725
fcx.param_env,
17241726
expected,
17251727
found,
17261728
coercion_error,
17271729
);
1728-
let then_span = fcx.find_block_span_from_hir_id(then_id);
1729-
let else_span = fcx.find_block_span_from_hir_id(else_id);
1730-
// don't suggest wrapping either blocks in `if .. {} else {}`
1731-
let is_empty_arm = |id| {
1732-
let hir::Node::Block(blk) = fcx.tcx.hir_node(id) else {
1733-
return false;
1734-
};
1735-
if blk.expr.is_some() || !blk.stmts.is_empty() {
1736-
return false;
1737-
}
1738-
let Some((_, hir::Node::Expr(expr))) =
1739-
fcx.tcx.hir_parent_iter(id).nth(1)
1740-
else {
1741-
return false;
1742-
};
1743-
matches!(expr.kind, hir::ExprKind::If(..))
1744-
};
1745-
if !is_empty_arm(then_id) && !is_empty_arm(else_id) {
1730+
let then_span = fcx.find_block_span_from_hir_id(then_expr.hir_id);
1731+
let else_span = fcx.find_block_span_from_hir_id(else_expr.hir_id);
1732+
// Don't suggest wrapping whole block in `Box::new`.
1733+
if then_span != then_expr.span && else_span != else_expr.span {
1734+
let then_ty = fcx.typeck_results.borrow().expr_ty(then_expr);
1735+
let else_ty = fcx.typeck_results.borrow().expr_ty(else_expr);
17461736
self.suggest_boxing_tail_for_return_position_impl_trait(
17471737
fcx,
17481738
&mut err,

‎compiler/rustc_hir_typeck/src/expr.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
557557
ascribed_ty
558558
}
559559
ExprKind::If(cond, then_expr, opt_else_expr) => {
560-
self.check_expr_if(cond, then_expr, opt_else_expr, expr.span, expected)
560+
self.check_expr_if(expr.hir_id, cond, then_expr, opt_else_expr, expr.span, expected)
561561
}
562562
ExprKind::DropTemps(e) => self.check_expr_with_expectation(e, expected),
563563
ExprKind::Array(args) => self.check_expr_array(args, expected, expr),
@@ -1314,6 +1314,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13141314
// or 'if-else' expression.
13151315
fn check_expr_if(
13161316
&self,
1317+
expr_id: HirId,
13171318
cond_expr: &'tcx hir::Expr<'tcx>,
13181319
then_expr: &'tcx hir::Expr<'tcx>,
13191320
opt_else_expr: Option<&'tcx hir::Expr<'tcx>>,
@@ -1353,15 +1354,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13531354

13541355
let tail_defines_return_position_impl_trait =
13551356
self.return_position_impl_trait_from_match_expectation(orig_expected);
1356-
let if_cause = self.if_cause(
1357-
sp,
1358-
cond_expr.span,
1359-
then_expr,
1360-
else_expr,
1361-
then_ty,
1362-
else_ty,
1363-
tail_defines_return_position_impl_trait,
1364-
);
1357+
let if_cause =
1358+
self.if_cause(expr_id, else_expr, tail_defines_return_position_impl_trait);
13651359

13661360
coerce.coerce(self, &if_cause, else_expr, else_ty);
13671361

‎compiler/rustc_infer/src/infer/mod.rs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use rustc_middle::ty::{
3535
PseudoCanonicalInput, Term, TermKind, Ty, TyCtxt, TyVid, TypeFoldable, TypeFolder,
3636
TypeSuperFoldable, TypeVisitable, TypeVisitableExt, TypingEnv, TypingMode, fold_regions,
3737
};
38-
use rustc_span::{Span, Symbol};
38+
use rustc_span::{DUMMY_SP, Span, Symbol};
3939
use snapshot::undo_log::InferCtxtUndoLogs;
4040
use tracing::{debug, instrument};
4141
use type_variable::TypeVariableOrigin;
@@ -1557,15 +1557,16 @@ impl<'tcx> InferCtxt<'tcx> {
15571557
}
15581558
}
15591559

1560-
/// Given a [`hir::HirId`] for a block, get the span of its last expression
1561-
/// or statement, peeling off any inner blocks.
1560+
/// Given a [`hir::HirId`] for a block (or an expr of a block), get the span
1561+
/// of its last expression or statement, peeling off any inner blocks.
15621562
pub fn find_block_span_from_hir_id(&self, hir_id: hir::HirId) -> Span {
15631563
match self.tcx.hir_node(hir_id) {
1564-
hir::Node::Block(blk) => self.find_block_span(blk),
1565-
// The parser was in a weird state if either of these happen, but
1566-
// it's better not to panic.
1564+
hir::Node::Block(blk)
1565+
| hir::Node::Expr(&hir::Expr { kind: hir::ExprKind::Block(blk, _), .. }) => {
1566+
self.find_block_span(blk)
1567+
}
15671568
hir::Node::Expr(e) => e.span,
1568-
_ => rustc_span::DUMMY_SP,
1569+
_ => DUMMY_SP,
15691570
}
15701571
}
15711572
}

‎compiler/rustc_middle/src/traits/mod.rs‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,11 @@ pub enum ObligationCauseCode<'tcx> {
332332
},
333333

334334
/// Computing common supertype in an if expression
335-
IfExpression(Box<IfExpressionCause<'tcx>>),
335+
IfExpression {
336+
expr_id: HirId,
337+
// Is the expectation of this match expression an RPIT?
338+
tail_defines_return_position_impl_trait: Option<LocalDefId>,
339+
},
336340

337341
/// Computing common supertype of an if expression with no else counter-part
338342
IfExpressionWithNoElse,
@@ -548,18 +552,6 @@ pub struct PatternOriginExpr {
548552
pub peeled_prefix_suggestion_parentheses: bool,
549553
}
550554

551-
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
552-
#[derive(TypeFoldable, TypeVisitable, HashStable, TyEncodable, TyDecodable)]
553-
pub struct IfExpressionCause<'tcx> {
554-
pub then_id: HirId,
555-
pub else_id: HirId,
556-
pub then_ty: Ty<'tcx>,
557-
pub else_ty: Ty<'tcx>,
558-
pub outer_span: Option<Span>,
559-
// Is the expectation of this match expression an RPIT?
560-
pub tail_defines_return_position_impl_trait: Option<LocalDefId>,
561-
}
562-
563555
#[derive(Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)]
564556
#[derive(TypeVisitable, TypeFoldable)]
565557
pub struct DerivedCause<'tcx> {

‎compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs‎

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,7 @@ use crate::infer;
8282
use crate::infer::relate::{self, RelateResult, TypeRelation};
8383
use crate::infer::{InferCtxt, InferCtxtExt as _, TypeTrace, ValuePairs};
8484
use crate::solve::deeply_normalize_for_diagnostics;
85-
use crate::traits::{
86-
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
87-
};
85+
use crate::traits::{MatchExpressionArmCause, ObligationCause, ObligationCauseCode};
8886

8987
mod note_and_explain;
9088
mod suggest;
@@ -613,28 +611,61 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
613611
}
614612
}
615613
},
616-
ObligationCauseCode::IfExpression(box IfExpressionCause {
617-
then_id,
618-
else_id,
619-
then_ty,
620-
else_ty,
621-
outer_span,
622-
..
623-
}) => {
624-
let then_span = self.find_block_span_from_hir_id(then_id);
625-
let else_span = self.find_block_span_from_hir_id(else_id);
626-
if let hir::Node::Expr(e) = self.tcx.hir_node(else_id)
627-
&& let hir::ExprKind::If(_cond, _then, None) = e.kind
614+
ObligationCauseCode::IfExpression { expr_id, .. } => {
615+
let hir::Node::Expr(&hir::Expr {
616+
kind: hir::ExprKind::If(cond_expr, then_expr, Some(else_expr)),
617+
span: expr_span,
618+
..
619+
}) = self.tcx.hir_node(expr_id)
620+
else {
621+
return;
622+
};
623+
let then_span = self.find_block_span_from_hir_id(then_expr.hir_id);
624+
let then_ty = self
625+
.typeck_results
626+
.as_ref()
627+
.expect("if expression only expected inside FnCtxt")
628+
.expr_ty(then_expr);
629+
let else_span = self.find_block_span_from_hir_id(else_expr.hir_id);
630+
let else_ty = self
631+
.typeck_results
632+
.as_ref()
633+
.expect("if expression only expected inside FnCtxt")
634+
.expr_ty(else_expr);
635+
if let hir::ExprKind::If(_cond, _then, None) = else_expr.kind
628636
&& else_ty.is_unit()
629637
{
630638
// Account for `let x = if a { 1 } else if b { 2 };`
631639
err.note("`if` expressions without `else` evaluate to `()`");
632640
err.note("consider adding an `else` block that evaluates to the expected type");
633641
}
634642
err.span_label(then_span, "expected because of this");
643+
644+
let outer_span = if self.tcx.sess.source_map().is_multiline(expr_span) {
645+
if then_span.hi() == expr_span.hi() || else_span.hi() == expr_span.hi() {
646+
// Point at condition only if either block has the same end point as
647+
// the whole expression, since that'll cause awkward overlapping spans.
648+
Some(expr_span.shrink_to_lo().to(cond_expr.peel_drop_temps().span))
649+
} else {
650+
Some(expr_span)
651+
}
652+
} else {
653+
None
654+
};
635655
if let Some(sp) = outer_span {
636656
err.span_label(sp, "`if` and `else` have incompatible types");
637657
}
658+
659+
let then_id = if let hir::ExprKind::Block(then_blk, _) = then_expr.kind {
660+
then_blk.hir_id
661+
} else {
662+
then_expr.hir_id
663+
};
664+
let else_id = if let hir::ExprKind::Block(else_blk, _) = else_expr.kind {
665+
else_blk.hir_id
666+
} else {
667+
else_expr.hir_id
668+
};
638669
if let Some(subdiag) = self.suggest_remove_semi_or_return_binding(
639670
Some(then_id),
640671
then_ty,

0 commit comments

Comments
 (0)