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

Incremental compilation breaks(?) Rc::ptr_eq with trait objects across function call boundary #104446

Closed
isaacthefallenapple opened this issue Nov 15, 2022 · 2 comments
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@isaacthefallenapple
Copy link

I found this question on SO where two more or less identical calls to Rc::ptr_eq return different results, depending on where the Rc<RefCell<_>> is cast to a trait object but also how the program is compiled. It behaves inconsistently when the function in question is not inlined or when incremental compilation is turned on. I've whittled it down to a smaller example (though it can probably be reduced further):

use std::{cell::RefCell, rc::Rc};

struct S;
trait T {}
impl T for S {}

struct C {
    r: Rc<RefCell<dyn T>>,
}

impl C {
    fn has_dyn_other(&self, other: &Rc<RefCell<dyn T>>) -> bool {
        Rc::ptr_eq(&self.r, other)
    }
    fn has_other<A: T + 'static>(&self, other: &Rc<RefCell<A>>) -> bool {
        Rc::ptr_eq(&self.r, &(Rc::clone(other) as Rc<RefCell<dyn T>>))
    }
}

#[test]
fn weird_trait_obj() {
    let s = Rc::new(RefCell::new(S));
    let c = C {
        r: Rc::clone(&s) as Rc<RefCell<dyn T>>,
    };

    // passes
    assert!(Rc::ptr_eq(&(Rc::clone(&s) as Rc<RefCell<dyn T>>), &c.r));
    // passes
    assert!(Rc::ptr_eq(
        &(Rc::clone(&s) as Rc<RefCell<dyn T>>),
        &(Rc::clone(&s) as Rc<RefCell<dyn T>>)
    ));
    // passes
    assert!(c.has_dyn_other(&(Rc::clone(&s) as Rc<RefCell<dyn T>>)));
    // passes with `incremental = false` or `#[inline(always)]`, fails otherwise
    assert!(c.has_other(&s));
}

I expected to see this happen: I would have thought either all or none of the asserts would pass regardless of how it's compiled. I don't know how Rc<RefCell<T>> as Rc<RefCell<dyn Trait>> actually works, so I'm not sure which one. I'm guessing it looks more like Rc<dyn RefCell<Trait>> where the Rc points at a trait object that in turn points at the RefCell as its instance? I don't know how else it might work.

Instead, this happened: That last test is sensitive to at least two distinct factors: is incremental compilation on and is C::has_other inlined or not. Adding #[inline(always)] makes the assert pass regardless of incremental compilation.

Meta

rustc --version --verbose:

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: x86_64-unknown-linux-gnu
release: 1.65.0
LLVM version: 15.0.0
Backtrace

thread 'weird_trait_obj' panicked at 'assertion failed: c.has_other(&s)', src/lib.rs:37:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:584:5     
   1: core::panicking::panic_fmt
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:142:14   
   2: core::panicking::panic
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:48:5     
   3: rcrefcelltest::weird_trait_obj
             at ./src/lib.rs:37:5
   4: rcrefcelltest::weird_trait_obj::{{closure}}
             at ./src/lib.rs:21:1
   5: core::ops::function::FnOnce::call_once
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/ops/function.rs:248:5 
   6: core::ops::function::FnOnce::call_once
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/ops/function.rs:248:5 

@isaacthefallenapple isaacthefallenapple added the C-bug Category: This is a bug. label Nov 15, 2022
@the8472
Copy link
Member

the8472 commented Nov 15, 2022

The documentation does warn about comparing trait objects

Returns true if the two Rcs point to the same allocation in a vein similar to ptr::eq. See that function for caveats when comparing dyn Trait pointers.

@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
@Noratrieb
Copy link
Member

This has been fixed in #106450

@Noratrieb Noratrieb added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. labels Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants