rust-lang / trait-system-refactor-initiative

The Rustc Trait System Refactor Initiative
21 stars 0 forks source link

`AliasBound` candidates for normalizeable aliases are unsound #6

Open lcnr opened 1 year ago

lcnr commented 1 year ago

from https://github.com/lcnr/solver-woes/issues/9#issuecomment-1483510789

Another alias bound unsoundness segfault in the new solver, with #![feature(trivial_bounds)], but it shouldn't be too hard to make this into not a trivial bound with some lifetime arg:

// compile-flags: -Ztrait-solver=next
#![feature(trivial_bounds)]

trait Foo {
    type Item: Copy where <Self as Foo>::Item: Copy;

    fn copy_me(x: &Self::Item) -> Self::Item {
        *x
    }
}

impl Foo for () {
    type Item = String where String: Copy;
}

fn main() {
    let x = String::from("hello, world");
    drop(<() as Foo>::copy_me(&x));
    println!("{x}");
}

if we have type Assoc: OtherTrait.

1) given <T as Trait>::Assoc, we can assume OtherTrait 2) for this, we have to prove <T as Trait>::Assoc: OtherTrait inside of each impl

there's also the following example once we stop deeply normalizing the param_env

from https://github.com/lcnr/solver-woes/issues/9#issuecomment-1456284249

given the following changes to rustc:

diff --git a/compiler/rustc_trait_selection/src/solve/assembly.rs b/compiler/rustc_trait_selection/src/solve/assembly.rs
index dec9f8016b0..0f633ab5cb6 100644
--- a/compiler/rustc_trait_selection/src/solve/assembly.rs
+++ b/compiler/rustc_trait_selection/src/solve/assembly.rs
@@ -484,6 +484,8 @@ pub(super) fn merge_candidates_and_discard_reservation_impls(
             _ => {}
         }

+        return self.try_merge_responses(candidates.iter().map(|c| Ok(c.result)));
+
         if candidates.len() > 1 {
             let mut i = 0;
             'outer: while i < candidates.len() {
diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs
index f7559a3f10a..12eb6f7d1e7 100644
--- a/compiler/rustc_trait_selection/src/traits/project.rs
+++ b/compiler/rustc_trait_selection/src/traits/project.rs
@@ -463,7 +463,7 @@ fn fold_binder<T: TypeFoldable<TyCtxt<'tcx>>>(
     }

     fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
-        if !needs_normalization(&ty, self.param_env.reveal()) {
+        if !needs_normalization(&ty, self.param_env.reveal()) || self.interner().trait_solver_next() {
             return ty;
         }

these end up removing eager normalization, and choose always applicable candidates in case there are multiple. Both changes we consider to be desirable.

This ends up causing the following example to segfault with -Ztrait-solver=next:

use std::rc::Rc;
trait Trait {
    type Assoc: Send;
}

impl<T> Trait for T
where
    <T as Trait>::Assoc: Send,
{
    type Assoc = Rc<String>;
}

fn send<T: Clone>(x: &T) {
    let _ = x.clone();
}

fn foo(x: Rc<String>) {
    std::thread::scope(|s| {
        loop {
            println!("strong count: {}", Rc::strong_count(&x));
            for _ in 0..100 {
                s.spawn(|| 
                    // oh no, `Rc<String>` is not `Send`
                    send::<<() as Trait>::Assoc>(&x)
                );
            }
        }
    })
}

fn main() {
    foo(Rc::new(String::from("hey, this is a string")));
}
lcnr commented 1 year ago

TODO: write down the solution we decided on in https://github.com/rust-lang/rust/pull/110673 (this solution ended up being unnecessary again :grin: )

lcnr commented 8 months ago

we changed our approach here in https://github.com/rust-lang/rust/pull/110673 to not require any special case as alias-bounds are not applied to normalizable aliases at all now.