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

Closure Does/Doesn't compile depending purely on usage of a {} body with --edition=2021 #105699

Open
Olipro opened this issue Dec 14, 2022 · 3 comments
Labels
A-borrow-checker Area: The borrow checker A-closures Area: Closures (`|…| { … }`) A-coercions Area: implicit and explicit `expr as Type` coercions C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@Olipro
Copy link

Olipro commented Dec 14, 2022

Example available on https://rust.godbolt.org/z/bzjPv6TGT

You get a failure if you remove the edition flag, but it's completely different and that may or may not also be incorrect.

trait Tr {}

struct T {
    obj: Box<dyn Tr>,
}
struct U;

impl Tr for U{}

impl T {
    fn works<'a>(&'a mut self) -> Option<&'a mut dyn Tr> {
        true.then(|| { self.obj.as_mut() })
    }

    fn fails<'a>(&'a mut self) -> Option<&'a mut dyn Tr> {
        true.then(|| self.obj.as_mut())
    }
}

fn main() {
    let mut t = T{obj: Box::new(U{})};
    let _u = t.works();
}
@Olipro Olipro added the C-bug Category: This is a bug. label Dec 14, 2022
@BoxyUwU
Copy link
Member

BoxyUwU commented Dec 14, 2022

i assume this caused by || a not allowing a to be coerced to anything, for example the following also fails to compile:

fn main() {
    let a: &[i32; 2] = &[10; 2];
    let b: Option<&[i32]> = true.then(|| a);
}

the coercion in your code example would be the type of self.obj.as_mut() which is &'a mut (dyn Tr + 'static) -> &'a mut (dyn Tr + 'a)

edit: apparently true.then(|| { a }) doesnt compile either and neither does:

fn blah(a: &[i32; 2]) -> Option<&[i32]> {
    true.then(|| { a })
}

I am suddenly feeling very distrustful of rust's coercions 🤣

@megakorre
Copy link
Contributor

megakorre commented Feb 7, 2023

I could reproduce the same issue with these examples.

fn fails(arg: &mut Box<dyn Drop>) -> Option<&mut dyn Drop> {
    true.then(|| arg.as_mut())
}
fn works(arg: &mut Box<dyn Drop>) -> Option<&mut dyn Drop> {
    true.then(|| { arg.as_mut() })
}

I would have expected them both to de-sugar to the same thing but they produce different HIR and different MIR.
The MIR of works gets a extra indeterminate assignment and a cast.

MIR for the fails version

fn fails::{closure#0}(_1: [closure@issues/issue_105699.rs:2:15: 2:17]) -> &mut dyn Drop {
    debug input => (*(_1.0: &mut std::boxed::Box<dyn std::ops::Drop>)); // in scope 0 at issues/issue_105699.rs:1:10: 1:15
    let mut _0: &mut dyn std::ops::Drop; // return place in scope 0 at issues/issue_105699.rs:2:18: 2:18
    let mut _2: &mut std::boxed::Box<dyn std::ops::Drop>; // in scope 0 at issues/issue_105699.rs:2:18: 2:32

    bb0: {
        StorageLive(_2);                 // scope 0 at issues/issue_105699.rs:2:18: 2:32
        _2 = &mut (*(_1.0: &mut std::boxed::Box<dyn std::ops::Drop>)); // scope 0 at issues/issue_105699.rs:2:18: 2:32
        _0 = <Box<dyn Drop> as AsMut<dyn Drop>>::as_mut(move _2) -> [return: bb1, unwind: bb2]; // scope 0 at issues/issue_105699.rs:2:18: 2:32
                                         // mir::Constant
                                         // + span: issues/issue_105699.rs:2:24: 2:30
                                         // + literal: Const { ty: for<'a> fn(&'a mut Box<dyn Drop>) -> &'a mut dyn Drop {<Box<dyn Drop> as AsMut<dyn Drop>>::as_mut}, val: Value(<ZST>) }
    }

    bb1: {
        StorageDead(_2);                 // scope 0 at issues/issue_105699.rs:2:31: 2:32
        return;                          // scope 0 at issues/issue_105699.rs:2:32: 2:32
    }

    bb2 (cleanup): {
        resume;                          // scope 0 at issues/issue_105699.rs:2:15: 2:32
    }
}

MIR for the works version

fn works::{closure#0}(_1: [closure@issues/issue_105699.rs:2:15: 2:17]) -> &mut dyn Drop {
    debug input => (*(_1.0: &mut std::boxed::Box<dyn std::ops::Drop>)); // in scope 0 at issues/issue_105699.rs:1:10: 1:15
    let mut _0: &mut dyn std::ops::Drop; // return place in scope 0 at issues/issue_105699.rs:2:18: 2:18
    let mut _2: &mut dyn std::ops::Drop; // in scope 0 at issues/issue_105699.rs:2:20: 2:34
    let mut _3: &mut dyn std::ops::Drop; // in scope 0 at issues/issue_105699.rs:2:20: 2:34
    let mut _4: &mut std::boxed::Box<dyn std::ops::Drop>; // in scope 0 at issues/issue_105699.rs:2:20: 2:34

    bb0: {
        StorageLive(_2);                 // scope 0 at issues/issue_105699.rs:2:20: 2:34
        StorageLive(_3);                 // scope 0 at issues/issue_105699.rs:2:20: 2:34
        StorageLive(_4);                 // scope 0 at issues/issue_105699.rs:2:20: 2:34
        _4 = &mut (*(_1.0: &mut std::boxed::Box<dyn std::ops::Drop>)); // scope 0 at issues/issue_105699.rs:2:20: 2:34
        _3 = <Box<dyn Drop> as AsMut<dyn Drop>>::as_mut(move _4) -> [return: bb1, unwind: bb2]; // scope 0 at issues/issue_105699.rs:2:20: 2:34
                                         // mir::Constant
                                         // + span: issues/issue_105699.rs:2:26: 2:32
                                         // + literal: Const { ty: for<'a> fn(&'a mut Box<dyn Drop>) -> &'a mut dyn Drop {<Box<dyn Drop> as AsMut<dyn Drop>>::as_mut}, val: Value(<ZST>) }
    }

    bb1: {
        StorageDead(_4);                 // scope 0 at issues/issue_105699.rs:2:33: 2:34
        _2 = &mut (*_3);                 // scope 0 at issues/issue_105699.rs:2:20: 2:34
        _0 = move _2 as &mut dyn std::ops::Drop (Pointer(Unsize)); // scope 0 at issues/issue_105699.rs:2:20: 2:34
        StorageDead(_3);                 // scope 0 at issues/issue_105699.rs:2:35: 2:36
        StorageDead(_2);                 // scope 0 at issues/issue_105699.rs:2:35: 2:36
        return;                          // scope 0 at issues/issue_105699.rs:2:36: 2:36
    }

    bb2 (cleanup): {
        resume;                          // scope 0 at issues/issue_105699.rs:2:15: 2:36
    }
}

The differences in the MIR output (the one with a extra block gets a extra assignment and a cast) is also presents in these read only versions. But here the borrow checker is happy regardless.

fn works(arg: &Box<dyn Drop>) -> Option<&dyn Drop> {
    true.then(|| { arg.as_ref() })
}
fn also_works(arg: &Box<dyn Drop>) -> Option<&dyn Drop> {
    true.then(|| arg.as_ref())
}

I could only make it fail with dyn traits. normal types and other dynamic sized types compiles.
I also tried a DST struct with a drop impl and it also works.

@TheElectronWill
Copy link

TheElectronWill commented Nov 29, 2023

This is still an issue in rustc 1.74.0 and 1.76.0-nightly (2023-11-28 5facb42).
See this thread on the Rust users forum.

It's a bit annoying, because rustfmt automatically remove the braces, so that's not a good workaround.
Inserting a cast does work:

fn works<'a>(&'a mut self) -> Option<&'a mut dyn Tr> {
    true.then(|| self.obj.as_mut() as _) // ok
}

Also, the following code (proposed on the forum by @steffahn) produces an invalid suggestion from the compiler: applying it breaks compilation.

trait Data {}

struct Registry {
    map: HashMap<String, Box<dyn Data>>,
}

impl Registry {
    fn get_mut(&mut self, key: &str) -> Option<&mut dyn Data> {
        // I originally wanted to write this but it triggers the previously mentioned lifetime issue
        // self.map.get_mut(key).map(|b| &mut **b)

        // This compiles but the compiler suggests to remove the braces, which won't compile
        self.map.get_mut(key).map(move |b| return {&mut **b})
    }
}

@fmease fmease added A-closures Area: Closures (`|…| { … }`) A-borrow-checker Area: The borrow checker T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-coercions Area: implicit and explicit `expr as Type` coercions T-types Relevant to the types team, which will review and decide on the PR/issue. and removed needs-triage-legacy labels Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-closures Area: Closures (`|…| { … }`) A-coercions Area: implicit and explicit `expr as Type` coercions C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants