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

Generic trait with constraint on Any does not compile anymore #100266

Closed
CLOVIS-AI opened this issue Aug 8, 2022 · 6 comments
Closed

Generic trait with constraint on Any does not compile anymore #100266

CLOVIS-AI opened this issue Aug 8, 2022 · 6 comments
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@CLOVIS-AI
Copy link

Code

I tried this code:

use std::any::Any;

// Generic trait used to store multiple different kinds of objects dynamically
trait Foo<T> : Any {
    fn as_any(&self) -> &dyn Any;
}

// Example (empty) implementation of the Foo trait
// The error is not related to this structure, it is only needed to have a meaningful example in the main function
#[derive(Copy, Clone, Debug)]
struct FooImpl;

impl Foo<usize> for FooImpl {
    fn as_any(&self) -> &dyn Any {
        self
    }
}

// Finds all instances of type F in the array of Foo trait objects
fn find<T, F : Foo<T> + Clone>(vec: &Vec<Box<dyn Foo<T>>>) -> Vec<F> {
    vec.iter()
        .filter_map(|foo| foo.as_ref().as_any().downcast_ref::<F>())
        .cloned()
        .collect()
}

fn main() {
    let mut vec: Vec<Box<dyn Foo<usize>>> = Default::default();
    vec.push(Box::new(FooImpl));

    // "find all instances of FooImpl in vec"
    let result = find::<usize, FooImpl>(&vec);
    println!("Results: {:?}", result);
}

I expected to see this happen: code compiles

Instead, this happened: code doesn't compile (see error below)

Version it worked on

It most recently worked on toolchain 1.60-x86_64-unknown-linux-gnu

rustup run 1.60-x86_64-unknown-linux-gnu rustc --version --verbose:

rustc 1.60.0 (7737e0b5c 2022-04-04)
binary: rustc
commit-hash: 7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c
commit-date: 2022-04-04
host: x86_64-unknown-linux-gnu
release: 1.60.0
LLVM version: 14.0.0

Version with regression

rustc --version --verbose:

rustc 1.62.1 (e092d0b6b 2022-07-16)
binary: rustc
commit-hash: e092d0b6b43f2de967af0887873151bb1c0b18d3
commit-date: 2022-07-16
host: x86_64-unknown-linux-gnu
release: 1.62.1
LLVM version: 14.0.5

Backtrace

Compilation error on stable

$ cargo build                                     
   Compiling test-annotation v0.1.0 ([REDACTED]/test-annotation)
error[E0310]: the parameter type `T` may not live long enough
  --> src/main.rs:18:40
   |
18 |         .filter_map(|foo| foo.as_ref().as_any().downcast_ref::<F>())
   |                                        ^^^^^^ ...so that the type `dyn Foo<T>` will meet its required lifetime bounds...
   |
note: ...that is required by this bound
  --> src/main.rs:3:16
   |
3  | trait Foo<T> : Any {
   |                ^^^
help: consider adding an explicit lifetime bound...
   |
16 | fn find<T: 'static, F : Foo<T> + Clone>(vec: &Vec<Box<dyn Foo<T>>>) -> Vec<F> {
   |          +++++++++

For more information about this error, try `rustc --explain E0310`.
error: could not compile `test-annotation` due to previous error

@CLOVIS-AI CLOVIS-AI added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Aug 8, 2022
@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed regression-untriaged Untriaged performance or correctness regression. labels Aug 8, 2022
@compiler-errors
Copy link
Member

This code was intentionally broken in #92285 because the current code you have is technically unsound if used incorrectly.

If you apply the suggestion in the help: message, your code should be fixed.

@CLOVIS-AI
Copy link
Author

@compiler-errors My understanding of the proposed solution is that it requires the generic to have a 'static lifetime, which I do not wish.

In which case making it possible makes the program unsound?

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 10, 2022
@compiler-errors
Copy link
Member

In which case making it possible makes the program unsound?

It's probably possible to use this API you've written to turn a &'a T into a &'static T by accident, for example. I haven't really looked into it, but I know for certain in general this in #92285 is a soundness fix.

@CLOVIS-AI
Copy link
Author

I see. I'm not really a fan of code just not compiling anymore randomly. Thankfully this was in a work-in-progress branch that hadn't been merged yet. I thought source code forward compatibility of safe stable Rust could only be broken with edition upgrades?

After checking my use case in more details, it is possible to use &'static on all usages.

@compiler-errors
Copy link
Member

I thought source code forward compatibility of safe stable Rust could only be broken with edition upgrades?

I want to remind you that this was a soundness fix in #92285. I'm pretty sure Rust has no guarantee on the stability of unsound code, because it should have never compiled in the first place.

After checking my use case in more details, it is possible to use &'static on all usages.

That's probably because the &'static bound that you had in your code was already being strongly implied by your usage of Any. I would be interested in any code that is now impossible to express, and how that could be used to invoke UB without unsafe. But anyways, I'm glad you could fix your code.

@apiraino
Copy link
Contributor

apiraino commented Sep 15, 2022

IIUC by the comments, this is caused by a fix to possibly unsound code and updating the code is here the desired behaviour. I'm going to close this but don't hesitate to reopen if in disagreement. thanks!

@rustbot label -regression-from-stable-to-stable -I-prioritize

@apiraino apiraino closed this as not planned Won't fix, can't repro, duplicate, stale Sep 15, 2022
@rustbot rustbot removed regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 15, 2022
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants