Skip to content

Commit 1f6a188

Browse files
borsbors
committed
Auto merge of rust-lang#139594 - compiler-errors:if-cause, r=<try>
[experiment] How expensive is `if_cause`? TODO r? `@ghost`
2 parents 0fe8f34 + a999dda commit 1f6a188

File tree

12 files changed

+172
-226
lines changed

12 files changed

+172
-226
lines changed

‎compiler/rustc_hir_typeck/src/_match.rs

+5-94
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

@@ -412,105 +412,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
412412

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

503421
// Finally construct the cause:
504422
self.cause(
505423
error_sp,
506-
ObligationCauseCode::IfExpression(Box::new(IfExpressionCause {
507-
else_id,
508-
then_id,
509-
then_ty,
510-
else_ty,
511-
outer_span,
512-
tail_defines_return_position_impl_trait,
513-
})),
424+
ObligationCauseCode::IfExpression { expr_id, tail_defines_return_position_impl_trait },
514425
)
515426
}
516427

‎compiler/rustc_hir_typeck/src/coercion.rs

+17-27
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,
49+
MatchExpressionArmCause, Obligation, PredicateObligation, PredicateObligations,
5150
};
5251
use rustc_middle::span_bug;
5352
use rustc_middle::ty::adjustment::{
@@ -1695,39 +1694,30 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
16951694
);
16961695
}
16971696
}
1698-
ObligationCauseCode::IfExpression(box IfExpressionCause {
1699-
then_id,
1700-
else_id,
1701-
then_ty,
1702-
else_ty,
1697+
ObligationCauseCode::IfExpression {
1698+
expr_id,
17031699
tail_defines_return_position_impl_trait: Some(rpit_def_id),
1704-
..
1705-
}) => {
1700+
} => {
1701+
let hir::Node::Expr(hir::Expr {
1702+
kind: hir::ExprKind::If(_, then_expr, Some(else_expr)),
1703+
..
1704+
}) = fcx.tcx.hir_node(expr_id)
1705+
else {
1706+
unreachable!();
1707+
};
17061708
err = fcx.err_ctxt().report_mismatched_types(
17071709
cause,
17081710
fcx.param_env,
17091711
expected,
17101712
found,
17111713
coercion_error,
17121714
);
1713-
let then_span = fcx.find_block_span_from_hir_id(then_id);
1714-
let else_span = fcx.find_block_span_from_hir_id(else_id);
1715-
// don't suggest wrapping either blocks in `if .. {} else {}`
1716-
let is_empty_arm = |id| {
1717-
let hir::Node::Block(blk) = fcx.tcx.hir_node(id) else {
1718-
return false;
1719-
};
1720-
if blk.expr.is_some() || !blk.stmts.is_empty() {
1721-
return false;
1722-
}
1723-
let Some((_, hir::Node::Expr(expr))) =
1724-
fcx.tcx.hir_parent_iter(id).nth(1)
1725-
else {
1726-
return false;
1727-
};
1728-
matches!(expr.kind, hir::ExprKind::If(..))
1729-
};
1730-
if !is_empty_arm(then_id) && !is_empty_arm(else_id) {
1715+
let then_span = fcx.find_block_span_from_hir_id(then_expr.hir_id);
1716+
let else_span = fcx.find_block_span_from_hir_id(else_expr.hir_id);
1717+
// Don't suggest wrapping whole block in `Box::new`.
1718+
if then_span != then_expr.span && else_span != else_expr.span {
1719+
let then_ty = fcx.typeck_results.borrow().expr_ty(then_expr);
1720+
let else_ty = fcx.typeck_results.borrow().expr_ty(else_expr);
17311721
self.suggest_boxing_tail_for_return_position_impl_trait(
17321722
fcx,
17331723
&mut err,

‎compiler/rustc_hir_typeck/src/expr.rs

+4-10
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
566566
ascribed_ty
567567
}
568568
ExprKind::If(cond, then_expr, opt_else_expr) => {
569-
self.check_expr_if(cond, then_expr, opt_else_expr, expr.span, expected)
569+
self.check_expr_if(expr.hir_id, cond, then_expr, opt_else_expr, expr.span, expected)
570570
}
571571
ExprKind::DropTemps(e) => self.check_expr_with_expectation(e, expected),
572572
ExprKind::Array(args) => self.check_expr_array(args, expected, expr),
@@ -1298,6 +1298,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12981298
// or 'if-else' expression.
12991299
fn check_expr_if(
13001300
&self,
1301+
expr_id: HirId,
13011302
cond_expr: &'tcx hir::Expr<'tcx>,
13021303
then_expr: &'tcx hir::Expr<'tcx>,
13031304
opt_else_expr: Option<&'tcx hir::Expr<'tcx>>,
@@ -1337,15 +1338,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13371338

13381339
let tail_defines_return_position_impl_trait =
13391340
self.return_position_impl_trait_from_match_expectation(orig_expected);
1340-
let if_cause = self.if_cause(
1341-
sp,
1342-
cond_expr.span,
1343-
then_expr,
1344-
else_expr,
1345-
then_ty,
1346-
else_ty,
1347-
tail_defines_return_position_impl_trait,
1348-
);
1341+
let if_cause =
1342+
self.if_cause(expr_id, else_expr, tail_defines_return_position_impl_trait);
13491343

13501344
coerce.coerce(self, &if_cause, else_expr, else_ty);
13511345

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

+8-7
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use rustc_middle::ty::{
3535
TyVid, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable, TypeVisitableExt, TypingEnv,
3636
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;
@@ -1560,15 +1560,16 @@ impl<'tcx> InferCtxt<'tcx> {
15601560
}
15611561
}
15621562

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

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

+5-13
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

+46-15
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)