Skip to content

Commit 4f520ff

Browse files
compiler-errorscompiler-errors
committed
Make if_cause less expensive
1 parent 51548ce commit 4f520ff

File tree

9 files changed

+81
-117
lines changed

9 files changed

+81
-117
lines changed

Diff for: ‎compiler/rustc_hir_typeck/src/_match.rs

+3-70
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
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;
@@ -412,89 +412,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
412412

413413
pub(crate) fn if_cause(
414414
&self,
415-
span: Span,
416-
cond_span: Span,
415+
expr_id: HirId,
417416
then_expr: &'tcx hir::Expr<'tcx>,
418417
else_expr: &'tcx hir::Expr<'tcx>,
419418
then_ty: Ty<'tcx>,
420419
else_ty: Ty<'tcx>,
421420
tail_defines_return_position_impl_trait: Option<LocalDefId>,
422421
) -> 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-
449422
let (error_sp, else_id) = if let ExprKind::Block(block, _) = &else_expr.kind {
450423
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-
487424
(self.find_block_span(block), block.hir_id)
488425
} else {
489426
(else_expr.span, else_expr.hir_id)
490427
};
491428

492429
let then_id = if let ExprKind::Block(block, _) = &then_expr.kind {
493430
let block = block.innermost_block();
494-
// Exclude overlapping spans
495-
if block.expr.is_none() && block.stmts.is_empty() {
496-
outer_span = None;
497-
}
498431
block.hir_id
499432
} else {
500433
then_expr.hir_id
@@ -504,11 +437,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
504437
self.cause(
505438
error_sp,
506439
ObligationCauseCode::IfExpression(Box::new(IfExpressionCause {
440+
expr_id,
507441
else_id,
508442
then_id,
509443
then_ty,
510444
else_ty,
511-
outer_span,
512445
tail_defines_return_position_impl_trait,
513446
})),
514447
)

Diff for: ‎compiler/rustc_hir_typeck/src/expr.rs

+3-3
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>>,
@@ -1338,8 +1339,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13381339
let tail_defines_return_position_impl_trait =
13391340
self.return_position_impl_trait_from_match_expectation(orig_expected);
13401341
let if_cause = self.if_cause(
1341-
sp,
1342-
cond_expr.span,
1342+
expr_id,
13431343
then_expr,
13441344
else_expr,
13451345
then_ty,

Diff for: ‎compiler/rustc_middle/src/traits/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -551,11 +551,11 @@ pub struct PatternOriginExpr {
551551
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
552552
#[derive(TypeFoldable, TypeVisitable, HashStable, TyEncodable, TyDecodable)]
553553
pub struct IfExpressionCause<'tcx> {
554+
pub expr_id: HirId,
554555
pub then_id: HirId,
555556
pub else_id: HirId,
556557
pub then_ty: Ty<'tcx>,
557558
pub else_ty: Ty<'tcx>,
558-
pub outer_span: Option<Span>,
559559
// Is the expectation of this match expression an RPIT?
560560
pub tail_defines_return_position_impl_trait: Option<LocalDefId>,
561561
}

Diff for: ‎compiler/rustc_span/src/lib.rs

+12-2
Original file line numberDiff line numberDiff line change
@@ -973,9 +973,19 @@ impl Span {
973973

974974
/// Prepare two spans to a combine operation like `to` or `between`.
975975
fn prepare_to_combine(
976-
a_orig: Span,
977-
b_orig: Span,
976+
mut a_orig: Span,
977+
mut b_orig: Span,
978978
) -> Result<(SpanData, SpanData, Option<LocalDefId>), Span> {
979+
// Peel of `CondTemporary` desugaring kind, whose desugaring mark may
980+
// prevent an condition from being combined with its parent expression,
981+
// since that kind does not functionally affect the `Span`.
982+
if a_orig.desugaring_kind() == Some(DesugaringKind::CondTemporary) {
983+
a_orig.remove_mark();
984+
}
985+
if b_orig.desugaring_kind() == Some(DesugaringKind::CondTemporary) {
986+
b_orig.remove_mark();
987+
}
988+
979989
let (a, b) = (a_orig.data(), b_orig.data());
980990
if a.ctxt == b.ctxt {
981991
return Ok((a, b, if a.parent == b.parent { a.parent } else { None }));

Diff for: ‎compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs

+21-1
Original file line numberDiff line numberDiff line change
@@ -614,11 +614,11 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
614614
}
615615
},
616616
ObligationCauseCode::IfExpression(box IfExpressionCause {
617+
expr_id,
617618
then_id,
618619
else_id,
619620
then_ty,
620621
else_ty,
621-
outer_span,
622622
..
623623
}) => {
624624
let then_span = self.find_block_span_from_hir_id(then_id);
@@ -632,9 +632,29 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
632632
err.note("consider adding an `else` block that evaluates to the expected type");
633633
}
634634
err.span_label(then_span, "expected because of this");
635+
636+
let expr_span = self.tcx.hir_span(expr_id);
637+
let outer_span = if self.tcx.sess.source_map().is_multiline(expr_span) {
638+
if then_span.hi() == expr_span.hi() || else_span.hi() == expr_span.hi() {
639+
// Point at condition only if either block has the same end point as
640+
// the whole expression, since that'll cause awkward overlapping spans.
641+
if let hir::Node::Expr(expr) = self.tcx.hir_node(expr_id)
642+
&& let hir::ExprKind::If(cond, _, _) = expr.kind
643+
{
644+
Some(expr_span.shrink_to_lo().to(cond.span))
645+
} else {
646+
None
647+
}
648+
} else {
649+
Some(expr_span)
650+
}
651+
} else {
652+
None
653+
};
635654
if let Some(sp) = outer_span {
636655
err.span_label(sp, "`if` and `else` have incompatible types");
637656
}
657+
638658
if let Some(subdiag) = self.suggest_remove_semi_or_return_binding(
639659
Some(then_id),
640660
then_ty,

Diff for: ‎tests/ui/expr/if/if-else-chain-missing-else.stderr

+9-12
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,15 @@
11
error[E0308]: `if` and `else` have incompatible types
22
--> $DIR/if-else-chain-missing-else.rs:12:12
33
|
4-
LL | let x = if let Ok(x) = res {
5-
| ______________-
6-
LL | | x
7-
| | - expected because of this
8-
LL | | } else if let Err(e) = res {
9-
| | ____________^
10-
LL | || return Err(e);
11-
LL | || };
12-
| || ^
13-
| ||_____|
14-
| |_____`if` and `else` have incompatible types
15-
| expected `i32`, found `()`
4+
LL | let x = if let Ok(x) = res {
5+
| ------------------ `if` and `else` have incompatible types
6+
LL | x
7+
| - expected because of this
8+
LL | } else if let Err(e) = res {
9+
| ____________^
10+
LL | | return Err(e);
11+
LL | | };
12+
| |_____^ expected `i32`, found `()`
1613
|
1714
= note: `if` expressions without `else` evaluate to `()`
1815
= note: consider adding an `else` block that evaluates to the expected type

Diff for: ‎tests/ui/expr/if/if-else-type-mismatch.stderr

+10-7
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,16 @@ LL | | };
9292
error[E0308]: `if` and `else` have incompatible types
9393
--> $DIR/if-else-type-mismatch.rs:37:9
9494
|
95-
LL | let _ = if true {
96-
| _____________________-
97-
LL | |
98-
LL | | } else {
99-
| |_____- expected because of this
100-
LL | 11u32
101-
| ^^^^^ expected `()`, found `u32`
95+
LL | let _ = if true {
96+
| ______________- -
97+
| | _____________________|
98+
LL | ||
99+
LL | || } else {
100+
| ||_____- expected because of this
101+
LL | | 11u32
102+
| | ^^^^^ expected `()`, found `u32`
103+
LL | | };
104+
| |______- `if` and `else` have incompatible types
102105

103106
error[E0308]: `if` and `else` have incompatible types
104107
--> $DIR/if-else-type-mismatch.rs:42:12

Diff for: ‎tests/ui/inference/deref-suggestion.stderr

+12-15
Original file line numberDiff line numberDiff line change
@@ -164,21 +164,18 @@ LL | *b
164164
error[E0308]: `if` and `else` have incompatible types
165165
--> $DIR/deref-suggestion.rs:69:12
166166
|
167-
LL | let val = if true {
168-
| ________________-
169-
LL | | *a
170-
| | -- expected because of this
171-
LL | | } else if true {
172-
| | ____________^
173-
LL | ||
174-
LL | || b
175-
LL | || } else {
176-
LL | || &0
177-
LL | || };
178-
| || ^
179-
| ||_____|
180-
| |_____`if` and `else` have incompatible types
181-
| expected `i32`, found `&{integer}`
167+
LL | let val = if true {
168+
| ------- `if` and `else` have incompatible types
169+
LL | *a
170+
| -- expected because of this
171+
LL | } else if true {
172+
| ____________^
173+
LL | |
174+
LL | | b
175+
LL | | } else {
176+
LL | | &0
177+
LL | | };
178+
| |_____^ expected `i32`, found `&{integer}`
182179

183180
error[E0308]: mismatched types
184181
--> $DIR/deref-suggestion.rs:81:15

Diff for: ‎tests/ui/suggestions/return-bindings.stderr

+10-6
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,16 @@ LL ~
6262
error[E0308]: `if` and `else` have incompatible types
6363
--> $DIR/return-bindings.rs:30:9
6464
|
65-
LL | let s = if let Some(s) = opt_str {
66-
| ______________________________________-
67-
LL | | } else {
68-
| |_____- expected because of this
69-
LL | String::new()
70-
| ^^^^^^^^^^^^^ expected `()`, found `String`
65+
LL | let s = if let Some(s) = opt_str {
66+
| ______________- -
67+
| | ______________________________________|
68+
LL | || } else {
69+
| ||_____- expected because of this
70+
LL | | String::new()
71+
| | ^^^^^^^^^^^^^ expected `()`, found `String`
72+
LL | |
73+
LL | | };
74+
| |______- `if` and `else` have incompatible types
7175
|
7276
help: consider returning the local binding `s`
7377
|

0 commit comments

Comments
 (0)