Skip to content

Assemble Unpin candidates specially for generators in new solver #110207

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 2 commits into from
Apr 14, 2023
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
8 changes: 8 additions & 0 deletions compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,14 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
) {
let lang_items = self.tcx().lang_items();
let trait_def_id = goal.predicate.trait_def_id(self.tcx());

// N.B. When assembling built-in candidates for lang items that are also
// `auto` traits, then the auto trait candidate that is assembled in
// `consider_auto_trait_candidate` MUST be disqualified to remain sound.
//
// Instead of adding the logic here, it's a better idea to add it in
// `EvalCtxt::disqualify_auto_trait_candidate_due_to_possible_impl` in
// `solve::trait_goals` instead.
let result = if self.tcx().trait_is_auto(trait_def_id) {
G::consider_auto_trait_candidate(self, goal)
} else if self.tcx().trait_is_alias(trait_def_id) {
Expand Down
155 changes: 94 additions & 61 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use super::assembly::{self, structural_traits};
use super::{EvalCtxt, SolverMode};
use rustc_hir::def_id::DefId;
use rustc_hir::LangItem;
use rustc_hir::{LangItem, Movability};
use rustc_infer::traits::query::NoSolution;
use rustc_infer::traits::util::supertraits;
use rustc_middle::traits::solve::{CanonicalResponse, Certainty, Goal, QueryResult};
Expand Down Expand Up @@ -147,66 +147,8 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx> {
let self_ty = goal.predicate.self_ty();
match *self_ty.kind() {
// Stall int and float vars until they are resolved to a concrete
// numerical type. That's because the check for impls below treats
// int vars as matching any impl. Even if we filtered such impls,
// we probably don't want to treat an `impl !AutoTrait for i32` as
// disqualifying the built-in auto impl for `i64: AutoTrait` either.
ty::Infer(ty::IntVar(_) | ty::FloatVar(_)) => {
return ecx.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS);
}

// These types cannot be structurally decomposed into constitutent
// types, and therefore have no builtin impl.
ty::Dynamic(..)
| ty::Param(..)
| ty::Foreign(..)
| ty::Alias(ty::Projection, ..)
| ty::Placeholder(..) => return Err(NoSolution),

ty::Infer(_) | ty::Bound(_, _) => bug!("unexpected type `{self_ty}`"),

// For rigid types, we only register a builtin auto implementation
// if there is no implementation that could ever apply to the self
// type.
//
// This differs from the current stable behavior and fixes #84857.
// Due to breakage found via crater, we currently instead lint
// patterns which can be used to exploit this unsoundness on stable,
// see #93367 for more details.
ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Str
| ty::Array(_, _)
| ty::Slice(_)
| ty::RawPtr(_)
| ty::Ref(_, _, _)
| ty::FnDef(_, _)
| ty::FnPtr(_)
| ty::Closure(_, _)
| ty::Generator(_, _, _)
| ty::GeneratorWitness(_)
| ty::GeneratorWitnessMIR(_, _)
| ty::Never
| ty::Tuple(_)
| ty::Error(_)
| ty::Adt(_, _)
| ty::Alias(ty::Opaque, _) => {
if let Some(def_id) = ecx.tcx().find_map_relevant_impl(
goal.predicate.def_id(),
goal.predicate.self_ty(),
TreatProjections::NextSolverLookup,
Some,
) {
debug!(?def_id, ?goal, "disqualified auto-trait implementation");
return Err(NoSolution);
}
}
if let Some(result) = ecx.disqualify_auto_trait_candidate_due_to_possible_impl(goal) {
return result;
}

ecx.probe_and_evaluate_goal_for_constituent_tys(
Expand Down Expand Up @@ -630,6 +572,97 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
}

impl<'tcx> EvalCtxt<'_, 'tcx> {
// Return `Some` if there is an impl (built-in or user provided) that may
// hold for the self type of the goal, which for coherence and soundness
// purposes must disqualify the built-in auto impl assembled by considering
// the type's constituent types.
fn disqualify_auto_trait_candidate_due_to_possible_impl(
&mut self,
goal: Goal<'tcx, TraitPredicate<'tcx>>,
) -> Option<QueryResult<'tcx>> {
let self_ty = goal.predicate.self_ty();
match *self_ty.kind() {
// Stall int and float vars until they are resolved to a concrete
// numerical type. That's because the check for impls below treats
// int vars as matching any impl. Even if we filtered such impls,
// we probably don't want to treat an `impl !AutoTrait for i32` as
// disqualifying the built-in auto impl for `i64: AutoTrait` either.
ty::Infer(ty::IntVar(_) | ty::FloatVar(_)) => {
Some(self.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS))
}

// These types cannot be structurally decomposed into constitutent
// types, and therefore have no built-in auto impl.
ty::Dynamic(..)
| ty::Param(..)
| ty::Foreign(..)
| ty::Alias(ty::Projection, ..)
| ty::Placeholder(..) => Some(Err(NoSolution)),

ty::Infer(_) | ty::Bound(_, _) => bug!("unexpected type `{self_ty}`"),

// Generators have one special built-in candidate, `Unpin`, which
// takes precedence over the structural auto trait candidate being
// assembled.
ty::Generator(_, _, movability)
if Some(goal.predicate.def_id()) == self.tcx().lang_items().unpin_trait() =>
{
match movability {
Movability::Static => Some(Err(NoSolution)),
Movability::Movable => {
Some(self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes))
}
}
}

// For rigid types, any possible implementation that could apply to
// the type (even if after unification and processing nested goals
// it does not hold) will disqualify the built-in auto impl.
//
// This differs from the current stable behavior and fixes #84857.
// Due to breakage found via crater, we currently instead lint
// patterns which can be used to exploit this unsoundness on stable,
// see #93367 for more details.
ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Str
| ty::Array(_, _)
| ty::Slice(_)
| ty::RawPtr(_)
| ty::Ref(_, _, _)
| ty::FnDef(_, _)
| ty::FnPtr(_)
| ty::Closure(_, _)
| ty::Generator(_, _, _)
| ty::GeneratorWitness(_)
| ty::GeneratorWitnessMIR(_, _)
| ty::Never
| ty::Tuple(_)
| ty::Adt(_, _)
// FIXME: Handling opaques here is kinda sus. Especially because we
// simplify them to PlaceholderSimplifiedType.
| ty::Alias(ty::Opaque, _) => {
if let Some(def_id) = self.tcx().find_map_relevant_impl(
goal.predicate.def_id(),
goal.predicate.self_ty(),
TreatProjections::NextSolverLookup,
Some,
) {
debug!(?def_id, ?goal, "disqualified auto-trait implementation");
// No need to actually consider the candidate here,
// since we do that in `consider_impl_candidate`.
return Some(Err(NoSolution));
} else {
None
}
}
ty::Error(_) => None,
}
}

/// Convenience function for traits that are structural, i.e. that only
/// have nested subgoals that only change the self type. Unlike other
/// evaluate-like helpers, this does a probe, so it doesn't need to be
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/generator/non-static-is-unpin.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// revisions: current next
//[next] compile-flags: -Ztrait-solver=next
// run-pass

#![feature(generators, generator_trait)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
error[E0277]: `[static generator@$DIR/static-not-unpin.rs:11:25: 11:34]` cannot be unpinned
--> $DIR/static-not-unpin.rs:14:18
error[E0277]: `[static generator@$DIR/static-not-unpin.rs:14:25: 14:34]` cannot be unpinned
--> $DIR/static-not-unpin.rs:17:18
|
LL | assert_unpin(generator);
| ------------ ^^^^^^^^^ the trait `Unpin` is not implemented for `[static generator@$DIR/static-not-unpin.rs:11:25: 11:34]`
| ------------ ^^^^^^^^^ the trait `Unpin` is not implemented for `[static generator@$DIR/static-not-unpin.rs:14:25: 14:34]`
| |
| required by a bound introduced by this call
|
= note: consider using the `pin!` macro
consider using `Box::pin` if you need to access the pinned value outside of the current scope
note: required by a bound in `assert_unpin`
--> $DIR/static-not-unpin.rs:7:20
--> $DIR/static-not-unpin.rs:10:20
|
LL | fn assert_unpin<T: Unpin>(_: T) {
| ^^^^^ required by this bound in `assert_unpin`
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/generator/static-not-unpin.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error[E0277]: `[static generator@$DIR/static-not-unpin.rs:14:25: 14:34]` cannot be unpinned
--> $DIR/static-not-unpin.rs:17:18
|
LL | assert_unpin(generator);
| ------------ ^^^^^^^^^ the trait `Unpin` is not implemented for `[static generator@$DIR/static-not-unpin.rs:14:25: 14:34]`
| |
| required by a bound introduced by this call
|
= note: consider using the `pin!` macro
consider using `Box::pin` if you need to access the pinned value outside of the current scope
note: required by a bound in `assert_unpin`
--> $DIR/static-not-unpin.rs:10:20
|
LL | fn assert_unpin<T: Unpin>(_: T) {
| ^^^^^ required by this bound in `assert_unpin`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
3 changes: 3 additions & 0 deletions tests/ui/generator/static-not-unpin.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// revisions: current next
//[next] compile-flags: -Ztrait-solver=next

#![feature(generators)]

// normalize-stderr-test "std::pin::Unpin" -> "std::marker::Unpin"
Expand Down