Skip to content
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

AliasBound candidates for normalizeable aliases are unsound #6

Open
lcnr opened this issue Apr 17, 2023 · 2 comments
Open

AliasBound candidates for normalizeable aliases are unsound #6

lcnr opened this issue Apr 17, 2023 · 2 comments

Comments

@lcnr
Copy link
Contributor

lcnr commented Apr 17, 2023

from lcnr/solver-woes#9 (comment)

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 lcnr/solver-woes#9 (comment)

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
Copy link
Contributor Author

lcnr commented May 11, 2023

TODO: write down the solution we decided on in rust-lang/rust#110673 (this solution ended up being unnecessary again 😁 )

@lcnr lcnr removed the I-unresolved An unresolved issue label Jun 27, 2023
@lcnr
Copy link
Contributor Author

lcnr commented Feb 22, 2024

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

@lcnr lcnr changed the title AliasBound candidates are unsound AliasBound candidates for normalizeable aliases are unsound Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant