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

Trait upcasting shadows (trait object) deref coercion #89190

Closed
Tamschi opened this issue Sep 22, 2021 · 6 comments · Fixed by #89461
Closed

Trait upcasting shadows (trait object) deref coercion #89190

Tamschi opened this issue Sep 22, 2021 · 6 comments · Fixed by #89461
Labels
A-traits Area: Trait system C-bug Category: This is a bug. F-trait_upcasting `#![feature(trait_upcasting)]` P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Tamschi
Copy link

Tamschi commented Sep 22, 2021

Code

I tried this code:

#![allow(dead_code)]

use core::ops::Deref;

trait A {}
trait B: A {}
impl<'a> Deref for dyn 'a + B {
    type Target = dyn A;
    fn deref(&self) -> &Self::Target {
        todo!()
    }
}

fn take_a(_: &dyn A) { }

fn whoops(b: &dyn B) {
    take_a(b) // <-- Regression here.
}

I expected to see this happen: This should probably continue to compile as before, unless proven to be equivalent.

Instead, this happened: The detection for #65991 seems to be a little too aggressive, so it's flagging the coercion as feature-gated in beta even though a deref coercion is available:

   Compiling playground v0.0.1 (/playground)
error[E0658]: trait upcasting coercion is experimental
  --> src/lib.rs:17:12
   |
17 |     take_a(b)
   |            ^
   |
   = note: see issue #65991 <https://github.com/rust-lang/rust/issues/65991> for more information

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

Playground: https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=0d7f48d15e1aa249d953fa7d5b6cc2f9

Version it worked on

It most recently worked on: 1.55.0

rustc +stable --version --verbose:

rustc 1.55.0 (c8dfcfe04 2021-09-06)
binary: rustc
commit-hash: c8dfcfe046a7680554bf4eb612bad840e7631c4b
commit-date: 2021-09-06
host: x86_64-unknown-linux-gnu
release: 1.55.0
LLVM version: 12.0.1

Version with regression

rustc +beta --version --verbose:

rustc 1.56.0-beta.3 (deef86610 2021-09-17)
binary: rustc
commit-hash: deef866109022248b2506587b377c6b1c881d963
commit-date: 2021-09-17
host: x86_64-unknown-linux-gnu
release: 1.56.0-beta.3
LLVM version: 13.0.0

Backtrace

(not applicable)

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

@Tamschi Tamschi added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Sep 22, 2021
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Sep 22, 2021
@hkmatsumoto
Copy link
Member

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 23, 2021
@apiraino
Copy link
Contributor

Bisection:

searched nightlies: from nightly-2020-01-01 to nightly-2021-09-23
regressed nightly: nightly-2021-08-01
searched commits: from 1f0a591 to 4e28279
regressed commit: 7069a8c

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2020-01-01 

@nbdd0121
Copy link
Contributor

There is a Zulip thread that discusses this issue.

Tagging this with T-lang since this is a change in semantics.

@rustbot label +A-traits +F-trait_upcasting +T-lang +T-compiler

@rustbot rustbot added A-traits Area: Trait system F-trait_upcasting `#![feature(trait_upcasting)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Sep 23, 2021
Tamschi added a commit to Tamschi/fruit-salad that referenced this issue Sep 29, 2021
I'm reverting the workaround here as I'm done with debugging for the time being.
This code currently trips a beta regression. See <rust-lang/rust#89190>.
@bors bors closed this as completed in ab276b8 Oct 7, 2021
@Tamschi
Copy link
Author

Tamschi commented Nov 16, 2021

I'm not sure what the proper procedure is in this case, but I just noticed this hit stable in 1.56.0 (without the lint appearing beforehand) and broke my crate 😬

I'll work around it on my end, since I need this to continue work on a downstream crate.

Tamschi added a commit to Tamschi/fruit-salad that referenced this issue Nov 16, 2021
@nbdd0121
Copy link
Contributor

Hmm, it seems that the fix didn't get backported to 1.56. Does 1.57-beta work?

@Tamschi
Copy link
Author

Tamschi commented Nov 16, 2021

@nbdd0121 It does indeed: https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=0d7f48d15e1aa249d953fa7d5b6cc2f9

   Compiling playground v0.0.1 (/playground)
warning: `dyn B` implements `Deref` with supertrait `(dyn A + 'static)` as output
  --> src/lib.rs:17:12
   |
17 |     take_a(b)
   |            ^
   |
   = note: `#[warn(deref_into_dyn_supertrait)]` on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #89460 <https://github.com/rust-lang/rust/issues/89460>

warning: `playground` (lib) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 1.28s

The playground uses 1.57.0-beta.3 (2021-11-01 708d57e288d051a6238e) as of writing this.

@pnkfelix pnkfelix added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system C-bug Category: This is a bug. F-trait_upcasting `#![feature(trait_upcasting)]` P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants