Auto merge of #150105 - jackh726:remove-expressions-coerce, r=BoxyUwU
Remove Expressions (and just use a Vec) in coerce
Let's see if this has much of a perf impact - would be nice to clean this up a bit
r? ghost
diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs
index 6467adb..ded03c8 100644
--- a/compiler/rustc_hir_typeck/src/_match.rs
+++ b/compiler/rustc_hir_typeck/src/_match.rs
@@ -10,7 +10,7 @@
};
use tracing::{debug, instrument};
-use crate::coercion::{AsCoercionSite, CoerceMany};
+use crate::coercion::CoerceMany;
use crate::{Diverges, Expectation, FnCtxt, GatherLocalsVisitor, Needs};
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
@@ -73,7 +73,7 @@ pub(crate) fn check_expr_match(
Expectation::ExpectHasType(ety) if ety != tcx.types.unit => ety,
_ => self.next_ty_var(expr.span),
};
- CoerceMany::with_coercion_sites(coerce_first, arms)
+ CoerceMany::with_capacity(coerce_first, arms.len())
};
let mut prior_non_diverging_arms = vec![]; // Used only for diagnostics.
@@ -269,16 +269,13 @@ fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm<'tcx>]) {
/// Handle the fallback arm of a desugared if(-let) like a missing else.
///
/// Returns `true` if there was an error forcing the coercion to the `()` type.
- pub(super) fn if_fallback_coercion<T>(
+ pub(super) fn if_fallback_coercion(
&self,
if_span: Span,
cond_expr: &'tcx hir::Expr<'tcx>,
then_expr: &'tcx hir::Expr<'tcx>,
- coercion: &mut CoerceMany<'tcx, '_, T>,
- ) -> bool
- where
- T: AsCoercionSite,
- {
+ coercion: &mut CoerceMany<'tcx>,
+ ) -> bool {
// If this `if` expr is the parent's function return expr,
// the cause of the type coercion is the return type, point at it. (#25228)
let hir_id = self.tcx.parent_hir_id(self.tcx.parent_hir_id(then_expr.hir_id));
diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs
index 127965c..b4a4354 100644
--- a/compiler/rustc_hir_typeck/src/coercion.rs
+++ b/compiler/rustc_hir_typeck/src/coercion.rs
@@ -1165,17 +1165,14 @@ fn sig_for_closure_coercion(
///
/// This is really an internal helper. From outside the coercion
/// module, you should instantiate a `CoerceMany` instance.
- fn try_find_coercion_lub<E>(
+ fn try_find_coercion_lub(
&self,
cause: &ObligationCause<'tcx>,
- exprs: &[E],
+ exprs: &[&'tcx hir::Expr<'tcx>],
prev_ty: Ty<'tcx>,
new: &hir::Expr<'_>,
new_ty: Ty<'tcx>,
- ) -> RelateResult<'tcx, Ty<'tcx>>
- where
- E: AsCoercionSite,
- {
+ ) -> RelateResult<'tcx, Ty<'tcx>> {
let prev_ty = self.try_structurally_resolve_type(cause.span, prev_ty);
let new_ty = self.try_structurally_resolve_type(new.span, new_ty);
debug!(
@@ -1269,7 +1266,7 @@ fn try_find_coercion_lub<E>(
ty::FnDef(..) => Adjust::Pointer(PointerCoercion::ReifyFnPointer(sig.safety())),
_ => span_bug!(new.span, "should not try to coerce a {new_ty} to a fn pointer"),
};
- for expr in exprs.iter().map(|e| e.as_coercion_site()) {
+ for expr in exprs.iter() {
self.apply_adjustments(
expr,
vec![Adjustment { kind: prev_adjustment.clone(), target: fn_ptr }],
@@ -1316,7 +1313,6 @@ fn try_find_coercion_lub<E>(
let (adjustments, target) = self.register_infer_ok_obligations(ok);
for expr in exprs {
- let expr = expr.as_coercion_site();
self.apply_adjustments(expr, adjustments.clone());
}
debug!(
@@ -1382,41 +1378,23 @@ pub fn can_coerce<'tcx>(
/// }
/// let final_ty = coerce.complete(fcx);
/// ```
-pub(crate) struct CoerceMany<'tcx, 'exprs, E: AsCoercionSite> {
+pub(crate) struct CoerceMany<'tcx> {
expected_ty: Ty<'tcx>,
final_ty: Option<Ty<'tcx>>,
- expressions: Expressions<'tcx, 'exprs, E>,
- pushed: usize,
+ expressions: Vec<&'tcx hir::Expr<'tcx>>,
}
-/// The type of a `CoerceMany` that is storing up the expressions into
-/// a buffer. We use this in `check/mod.rs` for things like `break`.
-pub(crate) type DynamicCoerceMany<'tcx> = CoerceMany<'tcx, 'tcx, &'tcx hir::Expr<'tcx>>;
-
-enum Expressions<'tcx, 'exprs, E: AsCoercionSite> {
- Dynamic(Vec<&'tcx hir::Expr<'tcx>>),
- UpFront(&'exprs [E]),
-}
-
-impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
- /// The usual case; collect the set of expressions dynamically.
- /// If the full set of coercion sites is known before hand,
- /// consider `with_coercion_sites()` instead to avoid allocation.
+impl<'tcx> CoerceMany<'tcx> {
+ /// Creates a `CoerceMany` with a default capacity of 1. If the full set of
+ /// coercion sites is known before hand, consider `with_capacity()` instead
+ /// to avoid allocation.
pub(crate) fn new(expected_ty: Ty<'tcx>) -> Self {
- Self::make(expected_ty, Expressions::Dynamic(vec![]))
+ Self::with_capacity(expected_ty, 1)
}
- /// As an optimization, you can create a `CoerceMany` with a
- /// preexisting slice of expressions. In this case, you are
- /// expected to pass each element in the slice to `coerce(...)` in
- /// order. This is used with arrays in particular to avoid
- /// needlessly cloning the slice.
- pub(crate) fn with_coercion_sites(expected_ty: Ty<'tcx>, coercion_sites: &'exprs [E]) -> Self {
- Self::make(expected_ty, Expressions::UpFront(coercion_sites))
- }
-
- fn make(expected_ty: Ty<'tcx>, expressions: Expressions<'tcx, 'exprs, E>) -> Self {
- CoerceMany { expected_ty, final_ty: None, expressions, pushed: 0 }
+ /// Creates a `CoerceMany` with a given capacity.
+ pub(crate) fn with_capacity(expected_ty: Ty<'tcx>, capacity: usize) -> Self {
+ CoerceMany { expected_ty, final_ty: None, expressions: Vec::with_capacity(capacity) }
}
/// Returns the "expected type" with which this coercion was
@@ -1529,7 +1507,7 @@ pub(crate) fn coerce_inner<'a>(
// Handle the actual type unification etc.
let result = if let Some(expression) = expression {
- if self.pushed == 0 {
+ if self.expressions.is_empty() {
// Special-case the first expression we are coercing.
// To be honest, I'm not entirely sure why we do this.
// We don't allow two-phase borrows, see comment in try_find_coercion_lub for why
@@ -1541,22 +1519,13 @@ pub(crate) fn coerce_inner<'a>(
Some(cause.clone()),
)
} else {
- match self.expressions {
- Expressions::Dynamic(ref exprs) => fcx.try_find_coercion_lub(
- cause,
- exprs,
- self.merged_ty(),
- expression,
- expression_ty,
- ),
- Expressions::UpFront(coercion_sites) => fcx.try_find_coercion_lub(
- cause,
- &coercion_sites[0..self.pushed],
- self.merged_ty(),
- expression,
- expression_ty,
- ),
- }
+ fcx.try_find_coercion_lub(
+ cause,
+ &self.expressions,
+ self.merged_ty(),
+ expression,
+ expression_ty,
+ )
}
} else {
// this is a hack for cases where we default to `()` because
@@ -1591,18 +1560,7 @@ pub(crate) fn coerce_inner<'a>(
Ok(v) => {
self.final_ty = Some(v);
if let Some(e) = expression {
- match self.expressions {
- Expressions::Dynamic(ref mut buffer) => buffer.push(e),
- Expressions::UpFront(coercion_sites) => {
- // if the user gave us an array to validate, check that we got
- // the next expression in the list, as expected
- assert_eq!(
- coercion_sites[self.pushed].as_coercion_site().hir_id,
- e.hir_id
- );
- }
- }
- self.pushed += 1;
+ self.expressions.push(e);
}
}
Err(coercion_error) => {
@@ -1955,45 +1913,12 @@ pub(crate) fn complete<'a>(self, fcx: &FnCtxt<'a, 'tcx>) -> Ty<'tcx> {
} else {
// If we only had inputs that were of type `!` (or no
// inputs at all), then the final type is `!`.
- assert_eq!(self.pushed, 0);
+ assert!(self.expressions.is_empty());
fcx.tcx.types.never
}
}
}
-/// Something that can be converted into an expression to which we can
-/// apply a coercion.
-pub(crate) trait AsCoercionSite {
- fn as_coercion_site(&self) -> &hir::Expr<'_>;
-}
-
-impl AsCoercionSite for hir::Expr<'_> {
- fn as_coercion_site(&self) -> &hir::Expr<'_> {
- self
- }
-}
-
-impl<'a, T> AsCoercionSite for &'a T
-where
- T: AsCoercionSite,
-{
- fn as_coercion_site(&self) -> &hir::Expr<'_> {
- (**self).as_coercion_site()
- }
-}
-
-impl AsCoercionSite for ! {
- fn as_coercion_site(&self) -> &hir::Expr<'_> {
- *self
- }
-}
-
-impl AsCoercionSite for hir::Arm<'_> {
- fn as_coercion_site(&self) -> &hir::Expr<'_> {
- self.body
- }
-}
-
/// Recursively visit goals to decide whether an unsizing is possible.
/// `Break`s when it isn't, and an error should be raised.
/// `Continue`s when an unsizing ok based on an implementation of the `Unsize` trait / lang item.
diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs
index 4808256..c4fa39c 100644
--- a/compiler/rustc_hir_typeck/src/expr.rs
+++ b/compiler/rustc_hir_typeck/src/expr.rs
@@ -40,7 +40,7 @@
use {rustc_ast as ast, rustc_hir as hir};
use crate::Expectation::{self, ExpectCastableToType, ExpectHasType, NoExpectation};
-use crate::coercion::{CoerceMany, DynamicCoerceMany};
+use crate::coercion::CoerceMany;
use crate::errors::{
AddressOfTemporaryTaken, BaseExpressionDoubleDot, BaseExpressionDoubleDotAddExpr,
BaseExpressionDoubleDotRemove, CantDereference, FieldMultiplySpecifiedInInitializer,
@@ -1227,7 +1227,7 @@ fn check_expr_if(
// (`only_has_type`); otherwise, we just go with a
// fresh type variable.
let coerce_to_ty = expected.coercion_target_type(self, sp);
- let mut coerce: DynamicCoerceMany<'_> = CoerceMany::new(coerce_to_ty);
+ let mut coerce = CoerceMany::with_capacity(coerce_to_ty, 2);
coerce.coerce(self, &self.misc(sp), then_expr, then_ty);
@@ -1681,7 +1681,7 @@ fn check_expr_array(
.to_option(self)
.and_then(|uty| self.try_structurally_resolve_type(expr.span, uty).builtin_index())
.unwrap_or_else(|| self.next_ty_var(expr.span));
- let mut coerce = CoerceMany::with_coercion_sites(coerce_to, args);
+ let mut coerce = CoerceMany::with_capacity(coerce_to, args.len());
for e in args {
let e_ty = self.check_expr_with_hint(e, coerce_to);
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
index d04133c..c07cbfa 100644
--- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
+++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
@@ -1033,11 +1033,7 @@ pub(in super::super) fn check_expr_block(
// break 'a 22; }` would not force the type of the block
// to be `()`).
let coerce_to_ty = expected.coercion_target_type(self, blk.span);
- let coerce = if blk.targeted_by_break {
- CoerceMany::new(coerce_to_ty)
- } else {
- CoerceMany::with_coercion_sites(coerce_to_ty, blk.expr.as_slice())
- };
+ let coerce = CoerceMany::new(coerce_to_ty);
let prev_diverges = self.diverges.get();
let ctxt = BreakableCtxt { coerce: Some(coerce), may_break: false };
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
index 998c5e6..c875e2e 100644
--- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
+++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
@@ -25,7 +25,7 @@
self, FulfillmentError, ObligationCause, ObligationCauseCode, ObligationCtxt,
};
-use crate::coercion::DynamicCoerceMany;
+use crate::coercion::CoerceMany;
use crate::fallback::DivergingFallbackBehavior;
use crate::fn_ctxt::checks::DivergingBlockBehavior;
use crate::{CoroutineTypes, Diverges, EnclosingBreakables, TypeckRootCtxt};
@@ -56,13 +56,13 @@ pub(crate) struct FnCtxt<'a, 'tcx> {
/// expressions. If `None`, this is in a context where return is
/// inappropriate, such as a const expression.
///
- /// This is a `RefCell<DynamicCoerceMany>`, which means that we
+ /// This is a `RefCell<CoerceMany>`, which means that we
/// can track all the return expressions and then use them to
/// compute a useful coercion from the set, similar to a match
/// expression or other branching context. You can use methods
/// like `expected_ty` to access the declared return type (if
/// any).
- pub(super) ret_coercion: Option<RefCell<DynamicCoerceMany<'tcx>>>,
+ pub(super) ret_coercion: Option<RefCell<CoerceMany<'tcx>>>,
/// First span of a return site that we find. Used in error messages.
pub(super) ret_coercion_span: Cell<Option<Span>>,
diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs
index d3ef1d6..39c28c4 100644
--- a/compiler/rustc_hir_typeck/src/lib.rs
+++ b/compiler/rustc_hir_typeck/src/lib.rs
@@ -61,7 +61,7 @@
use typeck_root_ctxt::TypeckRootCtxt;
use crate::check::check_fn;
-use crate::coercion::DynamicCoerceMany;
+use crate::coercion::CoerceMany;
use crate::diverges::Diverges;
use crate::expectation::Expectation;
use crate::fn_ctxt::LoweredTy;
@@ -349,7 +349,7 @@ pub struct BreakableCtxt<'tcx> {
// this is `null` for loops where break with a value is illegal,
// such as `while`, `for`, and `while let`
- coerce: Option<DynamicCoerceMany<'tcx>>,
+ coerce: Option<CoerceMany<'tcx>>,
}
pub struct EnclosingBreakables<'tcx> {
