Skip to content

re-use Sized fast-path #139577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1872,14 +1872,14 @@ impl<'tcx> Ty<'tcx> {

/// Fast path helper for testing if a type is `Sized`.
///
/// Returning true means the type is known to be sized. Returning
/// `false` means nothing -- could be sized, might not be.
/// Returning true means the type is known to implement `Sized`. Returning `false` means
/// nothing -- could be sized, might not be.
///
/// Note that we could never rely on the fact that a type such as `[_]` is
/// trivially `!Sized` because we could be in a type environment with a
/// bound such as `[_]: Copy`. A function with such a bound obviously never
/// can be called, but that doesn't mean it shouldn't typecheck. This is why
/// this method doesn't return `Option<bool>`.
/// Note that we could never rely on the fact that a type such as `[_]` is trivially `!Sized`
/// because we could be in a type environment with a bound such as `[_]: Copy`. A function with
/// such a bound obviously never can be called, but that doesn't mean it shouldn't typecheck.
/// This is why this method doesn't return `Option<bool>`.
#[instrument(skip(tcx), level = "debug")]
pub fn is_trivially_sized(self, tcx: TyCtxt<'tcx>) -> bool {
match self.kind() {
ty::Infer(ty::IntVar(_) | ty::FloatVar(_))
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_trait_selection/src/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ use super::{
};
use crate::error_reporting::InferCtxtErrorExt;
use crate::infer::{InferCtxt, TyOrConstInferVar};
use crate::traits::EvaluateConstErr;
use crate::traits::normalize::normalize_with_depth_to;
use crate::traits::project::{PolyProjectionObligation, ProjectionCacheKeyExt as _};
use crate::traits::query::evaluate_obligation::InferCtxtExt;
use crate::traits::{EvaluateConstErr, sizedness_fast_path};

pub(crate) type PendingPredicateObligations<'tcx> = ThinVec<PendingPredicateObligation<'tcx>>;

Expand Down Expand Up @@ -335,6 +335,10 @@ impl<'a, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'tcx> {

let infcx = self.selcx.infcx;

if sizedness_fast_path(infcx.tcx, obligation.predicate) {
return ProcessResult::Changed(thin_vec::thin_vec![]);
}

if obligation.predicate.has_aliases() {
let mut obligations = PredicateObligations::new();
let predicate = normalize_with_depth_to(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ pub use self::specialize::{
pub use self::structural_normalize::StructurallyNormalizeExt;
pub use self::util::{
BoundVarReplacer, PlaceholderReplacer, elaborate, expand_trait_aliases, impl_item_is_final,
supertrait_def_ids, supertraits, transitive_bounds_that_define_assoc_item, upcast_choices,
with_replaced_escaping_bound_vars,
sizedness_fast_path, supertrait_def_ids, supertraits, transitive_bounds_that_define_assoc_item,
upcast_choices, with_replaced_escaping_bound_vars,
};
use crate::error_reporting::InferCtxtErrorExt;
use crate::infer::outlives::env::OutlivesEnvironment;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use rustc_hir::LangItem;
use rustc_infer::traits::Obligation;
use rustc_middle::traits::ObligationCause;
use rustc_middle::traits::query::NoSolution;
Expand All @@ -7,7 +6,7 @@ use rustc_middle::ty::{self, ParamEnvAnd, TyCtxt};
use rustc_span::Span;

use crate::infer::canonical::{CanonicalQueryInput, CanonicalQueryResponse};
use crate::traits::ObligationCtxt;
use crate::traits::{ObligationCtxt, sizedness_fast_path};

impl<'tcx> super::QueryTypeOp<'tcx> for ProvePredicate<'tcx> {
type QueryResponse = ();
Expand All @@ -16,15 +15,7 @@ impl<'tcx> super::QueryTypeOp<'tcx> for ProvePredicate<'tcx> {
tcx: TyCtxt<'tcx>,
key: &ParamEnvAnd<'tcx, Self>,
) -> Option<Self::QueryResponse> {
// Proving Sized, very often on "obviously sized" types like
// `&T`, accounts for about 60% percentage of the predicates
// we have to prove. No need to canonicalize and all that for
// such cases.
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(trait_ref)) =
key.value.predicate.kind().skip_binder()
&& tcx.is_lang_item(trait_ref.def_id(), LangItem::Sized)
&& trait_ref.self_ty().is_trivially_sized(tcx)
{
if sizedness_fast_path(tcx, key.value.predicate) {
return Some(());
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ use crate::infer::{InferCtxt, InferOk, TypeFreshener};
use crate::solve::InferCtxtSelectExt as _;
use crate::traits::normalize::{normalize_with_depth, normalize_with_depth_to};
use crate::traits::project::{ProjectAndUnifyResult, ProjectionCacheKeyExt};
use crate::traits::{EvaluateConstErr, ProjectionCacheKey, Unimplemented, effects};
use crate::traits::{
EvaluateConstErr, ProjectionCacheKey, Unimplemented, effects, sizedness_fast_path,
};

mod _match;
mod candidate_assembly;
Expand Down Expand Up @@ -603,6 +605,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
None => self.check_recursion_limit(&obligation, &obligation)?,
}

if sizedness_fast_path(self.tcx(), obligation.predicate) {
return Ok(EvaluatedToOk);
}

ensure_sufficient_stack(|| {
let bound_predicate = obligation.predicate.kind();
match bound_predicate.skip_binder() {
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_trait_selection/src/traits/util.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::collections::{BTreeMap, VecDeque};

use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_hir::LangItem;
use rustc_hir::def_id::DefId;
use rustc_infer::infer::InferCtxt;
pub use rustc_infer::traits::util::*;
Expand Down Expand Up @@ -504,3 +505,21 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for PlaceholderReplacer<'_, 'tcx> {
}
}
}

pub fn sizedness_fast_path<'tcx>(tcx: TyCtxt<'tcx>, predicate: ty::Predicate<'tcx>) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe name it is_sized_pred_for_trivially_sized_type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't name it that because I add to it later in the Sized work with MetaSized and "we have a MetaSized predicate that we can skip because there's a Sized predicate with the same self_ty in the ParamEnv" - I can rename it in this PR as you suggest, and then rename it back to something more general later if you want?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... I guess. It's nonobvious enough that you need to read the function anyway unless you already know what it is, then the name is unique enough.

// Proving `Sized` very often on "obviously sized" types like `&T`, accounts for about 60%
// percentage of the predicates we have to prove. No need to canonicalize and all that for
// such cases.
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(trait_ref)) =
predicate.kind().skip_binder()
{
if tcx.is_lang_item(trait_ref.def_id(), LangItem::Sized)
&& trait_ref.self_ty().is_trivially_sized(tcx)
{
debug!("fast path -- trivial sizedness");
return true;
}
}

false
}
11 changes: 9 additions & 2 deletions compiler/rustc_traits/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::bug;
use rustc_middle::traits::CodegenObligationError;
use rustc_middle::ty::{self, PseudoCanonicalInput, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{self, PseudoCanonicalInput, TyCtxt, TypeVisitableExt, Upcast};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use rustc_trait_selection::traits::{
ImplSource, Obligation, ObligationCause, ObligationCtxt, ScrubbedTraitError, SelectionContext,
Unimplemented,
Unimplemented, sizedness_fast_path,
};
use tracing::debug;

Expand All @@ -34,6 +34,13 @@ pub(crate) fn codegen_select_candidate<'tcx>(
let (infcx, param_env) = tcx.infer_ctxt().ignoring_regions().build_with_typing_env(typing_env);
let mut selcx = SelectionContext::new(&infcx);

if sizedness_fast_path(tcx, trait_ref.upcast(tcx)) {
return Ok(&*tcx.arena.alloc(ImplSource::Builtin(
ty::solve::BuiltinImplSource::Trivial,
Default::default(),
)));
}

let obligation_cause = ObligationCause::dummy();
let obligation = Obligation::new(tcx, obligation_cause, param_env, trait_ref);

Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_traits/src/evaluate_obligation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_span::DUMMY_SP;
use rustc_trait_selection::traits::query::CanonicalPredicateGoal;
use rustc_trait_selection::traits::{
EvaluationResult, Obligation, ObligationCause, OverflowError, SelectionContext, TraitQueryMode,
sizedness_fast_path,
};
use tracing::debug;

Expand All @@ -23,6 +24,10 @@ fn evaluate_obligation<'tcx>(
debug!("evaluate_obligation: goal={:#?}", goal);
let ParamEnvAnd { param_env, value: predicate } = goal;

if sizedness_fast_path(tcx, predicate) {
return Ok(EvaluationResult::EvaluatedToOk);
}

let mut selcx = SelectionContext::with_query_mode(infcx, TraitQueryMode::Canonical);
let obligation = Obligation::new(tcx, ObligationCause::dummy(), param_env, predicate);

Expand Down
1 change: 1 addition & 0 deletions tests/ui/closures/issue-78720.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
fn server() -> impl {
//~^ ERROR at least one trait must be specified
//~^^ ERROR type annotations needed
().map2(|| "")
}

Expand Down
16 changes: 11 additions & 5 deletions tests/ui/closures/issue-78720.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | fn server() -> impl {
| ^^^^

error[E0412]: cannot find type `F` in this scope
--> $DIR/issue-78720.rs:13:12
--> $DIR/issue-78720.rs:14:12
|
LL | _func: F,
| ^
Expand All @@ -22,8 +22,14 @@ help: you might be missing a type parameter
LL | struct Map2<Segment2, F> {
| +++

error[E0282]: type annotations needed
--> $DIR/issue-78720.rs:1:16
|
LL | fn server() -> impl {
| ^^^^ cannot infer type
Comment on lines +28 to +29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lmao is this the {error type}: Sized fast path?


error[E0308]: mismatched types
--> $DIR/issue-78720.rs:7:39
--> $DIR/issue-78720.rs:8:39
|
LL | fn map2<F>(self, f: F) -> Map2<F> {}
| ^^ expected `Map2<F>`, found `()`
Expand All @@ -32,7 +38,7 @@ LL | fn map2<F>(self, f: F) -> Map2<F> {}
found unit type `()`

error[E0277]: the size for values of type `Self` cannot be known at compilation time
--> $DIR/issue-78720.rs:7:16
--> $DIR/issue-78720.rs:8:16
|
LL | fn map2<F>(self, f: F) -> Map2<F> {}
| ^^^^ doesn't have a size known at compile-time
Expand All @@ -47,7 +53,7 @@ help: function arguments must have a statically known size, borrowed types alway
LL | fn map2<F>(&self, f: F) -> Map2<F> {}
| +

error: aborting due to 4 previous errors
error: aborting due to 5 previous errors

Some errors have detailed explanations: E0277, E0308, E0412.
Some errors have detailed explanations: E0277, E0282, E0308, E0412.
For more information about an error, try `rustc --explain E0277`.
2 changes: 1 addition & 1 deletion tests/ui/codegen/overflow-during-mono.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//~ ERROR overflow evaluating the requirement `{closure@$DIR/overflow-during-mono.rs:13:41: 13:44}: Sized`
//~ ERROR overflow evaluating the requirement `for<'a> {closure@$DIR/overflow-during-mono.rs:13:41: 13:44}: FnMut(&'a _)`
//@ build-fail

#![recursion_limit = "32"]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/codegen/overflow-during-mono.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0275]: overflow evaluating the requirement `{closure@$DIR/overflow-during-mono.rs:13:41: 13:44}: Sized`
error[E0275]: overflow evaluating the requirement `for<'a> {closure@$DIR/overflow-during-mono.rs:13:41: 13:44}: FnMut(&'a _)`
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "64"]` attribute to your crate (`overflow_during_mono`)
= note: required for `Filter<std::array::IntoIter<i32, 11>, {closure@$DIR/overflow-during-mono.rs:13:41: 13:44}>` to implement `Iterator`
Expand Down
Loading