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

alias bound is unsound #9

Closed
lcnr opened this issue Feb 20, 2023 · 3 comments
Closed

alias bound is unsound #9

lcnr opened this issue Feb 20, 2023 · 3 comments

Comments

@lcnr
Copy link
Owner

lcnr commented Feb 20, 2023

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

It feels like this should be unsound by using the assumption while proving that it holds, but may require more coinduction or something, idk.

There are sound alternatives to AliasBound candidates but they may slightly change inference or result in a performance regression.

@lcnr
Copy link
Owner Author

lcnr commented Mar 6, 2023

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")));
}

@compiler-errors
Copy link

compiler-errors commented Mar 24, 2023

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}");
}

@lcnr lcnr added the S-unsound label Mar 31, 2023
@lcnr lcnr changed the title alias bound might be unsound alias bound is unsound Mar 31, 2023
@lcnr
Copy link
Owner Author

lcnr commented Apr 17, 2023

@lcnr lcnr closed this as completed Apr 17, 2023
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

2 participants