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

Implementation of trait "not general enough" in nightly, works fine on stable #131488

Open
ejmount opened this issue Oct 10, 2024 · 9 comments
Open
Labels
C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.

Comments

@ejmount
Copy link

ejmount commented Oct 10, 2024

Code

I tried this (reduced) code:

use tokio::sync::mpsc::{unbounded_channel,UnboundedSender};

pub trait Role: 'static + Sync + Send {
    type Return : Send;
}

pub trait BasicRole: 'static + Send + Sync {}
impl Role for dyn BasicRole + '_ {
    type Return = ();
}

pub struct ReturnEnvelope<R: Role + ?Sized> {
	pub return_path: Option<R::Return>,
}

pub fn start() {
    let (_basic_role_input, mut basic_role_output): (UnboundedSender<ReturnEnvelope<dyn BasicRole>>, _) = unbounded_channel();

    let event_loop = async move {
        let _ = basic_role_output.recv().await;
    };
    let _ = ::tokio::task::spawn(event_loop);
}

(playground)

I expected to see this happen: successful compile.

Instead, this happened: I received this error:

error: implementation of `Role` is not general enough
  --> src/lib.rs:22:13
   |
22 |     let _ = ::tokio::task::spawn(event_loop);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Role` is not general enough
   |
   = note: `Role` would have to be implemented for the type `(dyn BasicRole + '0)`, for any lifetime `'0`...
   = note: ...but `Role` is actually implemented for the type `(dyn BasicRole + '1)`, for some specific lifetime `'1`

This can also be seen on the playground configured to use nightly.

I am aware that handling this construct correctly seems to be an outstanding issue, I'm specifically reporting the inconsistency that it does work on stable and not nightly.

Version it worked on

It most recently worked on: 1.81.0 stable (most recent)

Version with regression

rustc --version --verbose:

rustc 1.83.0-nightly (eb4e23467 2024-10-09)
binary: rustc
commit-hash: eb4e2346748e1760f74fcaa27b42431e0b95f8f3
commit-date: 2024-10-09
host: x86_64-pc-windows-msvc
release: 1.83.0-nightly
LLVM version: 19.1.1

However, this has been an issue since at least the 2024-08-17 nightly (feeba19)

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

@ejmount ejmount added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Oct 10, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-untriaged Untriaged performance or correctness regression. labels Oct 10, 2024
@lqd
Copy link
Member

lqd commented Oct 10, 2024

cc @compiler-errors (if bisection is accurate)

@compiler-errors
Copy link
Member

That bisection was wrong. It's likely due to #124336, and the correct fix would be to change:

impl Role for dyn BasicRole + '_ {
    type Return = ();
}

to:

impl Role for dyn BasicRole + 'static {
    type Return = ();
}

Because BasicRole requires that the self type outlives 'static.

@compiler-errors
Copy link
Member

Actually, wait, this is not fixed by that change. This may be a knock-on issue related to #110338 :(

@ejmount
Copy link
Author

ejmount commented Oct 10, 2024 via email

@compiler-errors
Copy link
Member

The only fix I expect to maybe work is if you remove the 'static supertrait bound from Role and BasicRole. I don't know if that's possible given your unminimized code.

@ejmount
Copy link
Author

ejmount commented Oct 10, 2024

The only fix I expect to maybe work is if you remove the 'static supertrait bound from Role and BasicRole. I don't know if that's possible given your unminimized code.

Unfortunately that doesn't seem to be an option because I want to have Arc<dyn BasicRole> objects, which seems to imply that BasicRole must implement Any and therefore must be 'static. (Testing this immediately told me it was required, but the compiler never articulated where the 'static bound was coming from)

@bjorn3
Copy link
Member

bjorn3 commented Oct 10, 2024

If you want to avoid the 'static bound, use Arc<dyn BasicRole + '_>. If a trait object is used without explicit lifetime bound, it is inferred to be 'static.

@ejmount
Copy link
Author

ejmount commented Oct 11, 2024

I checked more carefully just now, and removing the 'static constraint from Role actually broke a number of functions taking a generic Arc<R: Role> and wanting to spawn tasks with it. Fixing this by putting the constraint into the signature so it took a Arc<R: Role + 'static> meant the code now built, including some unit tests that created Arc<dyn BasicRole> values (without having to specify Arc<dyn + '_>), so it seems that my project works for now.

@lqd
Copy link
Member

lqd commented Oct 15, 2024

This actually bisects to #124336. It fixed soundness issues, so some breakage was expected, but also some unintended ones that we didn't encounter on crater. I'll let Michael chime in on whether this is the latter.

@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 9, 2024
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. I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.
Projects
None yet
Development

No branches or pull requests

6 participants