1
Fork 0

Rollup merge of #135639 - lqd:trivial-builtin-impls, r=lcnr

new solver: prefer trivial builtin impls

As discussed [on zulip](https://rust-lang.zulipchat.com/#narrow/channel/364551-t-types.2Ftrait-system-refactor/topic/needs_help.3A.20trivial.20builtin.20impls), this PR:
- adds a new `BuiltinImplSource::Trivial` source, and marks the `Sized` builtin impls as trivial
- prefers these trivial builtin impls in `merge_trait_candidates`

The comments can likely be wordsmithed a bit better, and I ~stole~ was inspired by the old solver ones. Let me know how you want them improved.

When enabling the new solver for tests, 3 UI tests now pass:
- `regions/issue-26448-1.rs` and its sibling `regions/issue-26448-2.rs` were rejected by the new solver but accepted by the old one
- and `issues/issue-42796.rs` where the old solver emitted some overflow errors in addition to the expected error

(For some reason one of these tests is run-pass, but I can take care of that another day)

r? lcnr
This commit is contained in:
Matthias Krüger 2025-01-18 09:11:06 +01:00 committed by GitHub
commit 862a17cd17
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 41 additions and 6 deletions

View file

@ -4259,6 +4259,7 @@ dependencies = [
"rustc_serialize",
"rustc_type_ir",
"rustc_type_ir_macros",
"smallvec",
"tracing",
]

View file

@ -13,6 +13,7 @@ rustc_macros = { path = "../rustc_macros", optional = true }
rustc_serialize = { path = "../rustc_serialize", optional = true }
rustc_type_ir = { path = "../rustc_type_ir", default-features = false }
rustc_type_ir_macros = { path = "../rustc_type_ir_macros" }
smallvec = "1.8.1"
tracing = "0.1"
# tidy-alphabetical-end

View file

@ -8,6 +8,7 @@ use rustc_type_ir::lang_items::TraitSolverLangItem;
use rustc_type_ir::solve::CanonicalResponse;
use rustc_type_ir::visit::TypeVisitableExt as _;
use rustc_type_ir::{self as ty, Interner, TraitPredicate, TypingMode, Upcast as _, elaborate};
use smallvec::SmallVec;
use tracing::{instrument, trace};
use crate::delegate::SolverDelegate;
@ -225,7 +226,7 @@ where
}
ecx.probe_and_evaluate_goal_for_constituent_tys(
CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
CandidateSource::BuiltinImpl(BuiltinImplSource::Trivial),
goal,
structural_traits::instantiate_constituent_tys_for_sized_trait,
)
@ -1194,7 +1195,30 @@ where
};
}
// FIXME: prefer trivial builtin impls
// We prefer trivial builtin candidates, i.e. builtin impls without any
// nested requirements, over all others. This is a fix for #53123 and
// prevents where-bounds from accidentally extending the lifetime of a
// variable.
if candidates
.iter()
.any(|c| matches!(c.source, CandidateSource::BuiltinImpl(BuiltinImplSource::Trivial)))
{
let trivial_builtin_impls: SmallVec<[_; 1]> = candidates
.iter()
.filter(|c| {
matches!(c.source, CandidateSource::BuiltinImpl(BuiltinImplSource::Trivial))
})
.map(|c| c.result)
.collect();
// There should only ever be a single trivial builtin candidate
// as they would otherwise overlap.
assert_eq!(trivial_builtin_impls.len(), 1);
return if let Some(response) = self.try_merge_responses(&trivial_builtin_impls) {
Ok((response, Some(TraitGoalProvenVia::Misc)))
} else {
Ok((self.bail_with_ambiguity(&trivial_builtin_impls), None))
};
}
// If there are non-global where-bounds, prefer where-bounds
// (including global ones) over everything else.

View file

@ -991,7 +991,7 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
Err(ErrorGuaranteed { .. }) => true,
}
}
ImplSource::Builtin(BuiltinImplSource::Misc, _) => {
ImplSource::Builtin(BuiltinImplSource::Misc | BuiltinImplSource::Trivial, _) => {
// While a builtin impl may be known to exist, the associated type may not yet
// be known. Any type with multiple potential associated types is therefore
// not eligible.
@ -1296,7 +1296,7 @@ fn confirm_select_candidate<'cx, 'tcx>(
) -> Progress<'tcx> {
match impl_source {
ImplSource::UserDefined(data) => confirm_impl_candidate(selcx, obligation, data),
ImplSource::Builtin(BuiltinImplSource::Misc, data) => {
ImplSource::Builtin(BuiltinImplSource::Misc | BuiltinImplSource::Trivial, data) => {
let tcx = selcx.tcx();
let trait_def_id = obligation.predicate.trait_def_id(tcx);
if tcx.is_lang_item(trait_def_id, LangItem::Coroutine) {

View file

@ -248,7 +248,7 @@ fn resolve_associated_item<'tcx>(
})
}
}
traits::ImplSource::Builtin(BuiltinImplSource::Misc, _) => {
traits::ImplSource::Builtin(BuiltinImplSource::Misc | BuiltinImplSource::Trivial, _) => {
if tcx.is_lang_item(trait_ref.def_id, LangItem::Clone) {
// FIXME(eddyb) use lang items for methods instead of names.
let name = tcx.item_name(trait_item_id);

View file

@ -169,6 +169,9 @@ pub enum CandidateSource<I: Interner> {
#[derive(Clone, Copy, Hash, PartialEq, Eq, Debug)]
#[cfg_attr(feature = "nightly", derive(HashStable_NoContext, TyEncodable, TyDecodable))]
pub enum BuiltinImplSource {
/// A built-in impl that is considered trivial, without any nested requirements. They
/// are preferred over where-clauses, and we want to track them explicitly.
Trivial,
/// Some built-in impl we don't need to differentiate. This should be used
/// unless more specific information is necessary.
Misc,

View file

@ -1,4 +1,7 @@
//@ run-pass
//@ revisions: current next
//@ [next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)
//@ check-pass
pub trait Foo<T> {
fn foo(self) -> T;

View file

@ -1,3 +1,6 @@
//@ revisions: current next
//@ [next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)
//@ check-pass
pub struct Bar<T> {