Skip to content

(Anti-)regression between Rust 1.78.0 and Rust 1.79.0 with struct containing Cow<[Self]> #129541

Description

@zachs18

Code

I tried this code:

#![allow(unused)]
#[derive(Clone)] // nothing substantial changes if this is replaced with a manual impl Clone for Test
struct Hello {
    a: std::borrow::Cow<'static, [Self]>,
}
fn main(){}

I expected to see this happen: On 1.78 and below, this fails to compile with several E0277s (trait bound not satisfied)

full error message
error[E0277]: the trait bound `[Hello]: ToOwned` is not satisfied in `Hello`
 --> src/main.rs:4:32
  |
4 |   a: std::borrow::Cow<'static, [Self]>,
  |                                ^^^^^^ within `Hello`, the trait `ToOwned` is not implemented for `[Hello]`, which is required by `Hello: Sized`
  |
  = help: the trait `ToOwned` is implemented for `[T]`
note: required because it appears within the type `Hello`
 --> src/main.rs:3:8
  |
3 | struct Hello {
  |        ^^^^^
  = note: slice and array elements must have `Sized` type

error[E0277]: the trait bound `[Hello]: ToOwned` is not satisfied in `Hello`
 --> src/main.rs:2:10
  |
2 | #[derive(Clone)]
  |          ^^^^^ within `Hello`, the trait `ToOwned` is not implemented for `[Hello]`, which is required by `Hello: Sized`
  |
  = help: the trait `ToOwned` is implemented for `[T]`
note: required because it appears within the type `Hello`
 --> src/main.rs:3:8
  |
3 | struct Hello {
  |        ^^^^^
note: required by a bound in `Clone`
 --> /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/clone.rs:147:1
  = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `[Hello]: ToOwned` is not satisfied in `Hello`
 --> src/main.rs:2:10
  |
2 | #[derive(Clone)]
  |          ^^^^^ within `Hello`, the trait `ToOwned` is not implemented for `[Hello]`, which is required by `Hello: Sized`
  |
  = help: the trait `ToOwned` is implemented for `[T]`
note: required because it appears within the type `Hello`
 --> src/main.rs:3:8
  |
3 | struct Hello {
  |        ^^^^^
  = note: the return type of a function must have a statically known size
  = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `[Hello]: ToOwned` is not satisfied in `Cow<'_, [Hello]>`
 --> src/main.rs:4:3
  |
2 | #[derive(Clone)]
  |          ----- in this derive macro expansion
3 | struct Hello {
4 |   a: std::borrow::Cow<'static, [Self]>,
  |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ within `Cow<'_, [Hello]>`, the trait `ToOwned` is not implemented for `[Hello]`, which is required by `Cow<'_, [Hello]>: Sized`
  |
  = help: the trait `ToOwned` is implemented for `[T]`
note: required because it appears within the type `Cow<'_, [Hello]>`
 --> /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/alloc/src/borrow.rs:180:10
note: required by a bound in `clone`
 --> /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/clone.rs:160:5
  = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `bisectee` (bin "bisectee") due to 4 previous errors

Instead, this happened: On Rust 1.79 and above, it compiles without error

Version it didn't work on

It most recently didn't work on: Rust 1.78.0

Version with anti-regression

rustc --version --verbose:

rustc 1.79.0 (129f3b996 2024-06-10)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: x86_64-unknown-linux-gnu
release: 1.79.0
LLVM version: 18.1.7

Bisection:

searched nightlies: from nightly-2024-03-16 to nightly-2024-04-28
regressed nightly: nightly-2024-03-20
searched commit range: 3c85e56...a7e4de1
regressed commit: 196ff44

bisected with cargo-bisect-rustc v0.6.9

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

cargo bisect-rustc --start=1.78.0 --end=1.79.0 --regress success --preserve 

cc @lukas-code The description of #122493 says "This should not make more or less code compile, but it can change error output in some rare cases."

Probably nothing to do but add a test to make sure this keeps compiling in the future?

Activity

  1. added
    C-bugCategory: This is a bug.
    regression-untriagedUntriaged performance or correctness regression.
    on Aug 25, 2024
  2. added
    I-prioritizeIssue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}
    needs-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
    on Aug 25, 2024
  3. theemathas commented on Aug 25, 2024

    @theemathas
    Contributor

    The code you show has Self but the error has [Self]. Is this intentional?

  4. workingjubilee commented on Aug 25, 2024

    @workingjubilee
    Member

    Fixed. The gression did indeed happen for [Self]:

    https://rust.godbolt.org/z/4TY7a54Ps

  5. added
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    on Aug 25, 2024
  6. compiler-errors commented on Aug 25, 2024

    @compiler-errors
    Contributor

    So I expect this code to compile, especially given that Cow<'_, [T]> is Sized, so this struct is coinductively Sized. There's a slight "risk" w/ this code compiling wrt the way we handle coinduction the solver, but I'll leave that to @lcnr to judge.

    It would be interesting to know what specifically about #122493 caused this to compile, though I haven't looked at all about it.

  7. WaffleLapkin commented on Aug 25, 2024

    @WaffleLapkin
    Member

    I think this is more about Clone rather than Sized-ness by itself. (for Sized there is an example which compiled since forever, Cow<[T]> doesn't require coinduction to be Sized -- it contains a &[T] (trivially sized) and <[T] as ToOwned>::Owned (which has an implicit : Sized bound)).

    What I think happened is that #[derive(Clone)] expansion requires Cow<'static, [Self]>: Clone which requires Cow<'static, [Self]>: Sized which, before #122493, required wf(Cow<'static, [Self]>) which requires [Self]: ToOwned which requires Self: Clone which requires Cow<'static, [Self]>: Clone making a cycle.

    I think some details in my explanation are wrong (particularly the last step?), because I'm not familiar enough with type checking in rustc; but. I do think that "not requiring wf(Enum) when evaluating Enum: Sized" makes it so we don't evaluate bounds which lead to a cycle.

  8. lukas-code commented on Aug 25, 2024

    @lukas-code
    Contributor

    This was properly fixed in #122791, which also landed in 1.78.0.

    That PR also allows this similar code to compile, which still fails after #122493:

    #[derive(Clone)]
    struct Hello {
        a: <[Hello] as ToOwned>::Owned,
    }

    This happens, because normalizing <Hello as ToOwned>::Owned in this example involves a cycle during candidate selection, which is no longer an error after #122791:

    normalize <[Hello] as ToOwned>::Owned
        candidate #1: impl ToOwned for T where T: Sized [+ Clone]
            requires [Hello]: Sized
            -> error (unimplemented)
        candidate #2: impl ToOwned for [T] where T: Sized [+ Clone]
            requires Hello: Sized
                requires <[Hello] as ToOwned>::Owned: Sized
                    normalize <[Hello] as ToOwned>::Owned
            -> ambiguous (cycle)
        -> candidate #2 is ambiguous, but it is the only applicable candidate
    -> normalized to Vec<Hello> via candidate #2
    

    The Code in the OP compiles with just #122493, because that PR changed Cow<T>: Sized to always hold and not require checking <T as ToOwned>::Owned: Sized and therefore avoid the cycle.

  9. marmeladema commented on Aug 25, 2024

    @marmeladema
    Contributor

    I think this has solved #100347

  10. apiraino commented on Aug 26, 2024

    @apiraino
    Contributor

    WG-prioritization assigning priority (Zulip discussion).

    @rustbot label -I-prioritize +P-medium

  11. added and removed
    I-prioritizeIssue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}
    on Aug 26, 2024
  12. 39 remaining items

  13. added a commit that references this issue on Sep 20, 2024
  14. lcnr commented on Sep 21, 2024

    @lcnr
    Contributor

    please also add the 3 tests from #129541 (comment)

  15. reopened this on Sep 21, 2024
  16. Enselic commented on Nov 25, 2024

    @Enselic
    Member

    @lcnr I think you link to the wrong comment? I think you mean "tests from issues referenced from #129541 (comment)" ?

  17. lcnr commented on Nov 25, 2024

    @lcnr
    Contributor

    no, to close this issue I would like to also want the following tests from my FCP proposal to get added as regression tests:

  18. Enselic commented on Nov 25, 2024

    @Enselic
    Member

    PR that adds those tests: #133467
    I'm a bit confused though because one of them still fails. If I did something wrong it's hopefully easy for you to point out what I should change.

  19. lcnr commented on Nov 25, 2024

    @lcnr
    Contributor

    I want to merge the failing test to also catch when its behavior changes, even if it goes from pass to fail. I want to make sure we realize that we accidentally stabilized something before it hits stable 😁

  20. added a commit that references this issue on Nov 26, 2024
  21. added a commit that references this issue on Nov 26, 2024
  22. added a commit that references this issue on Nov 26, 2024
  23. added 2 commits that reference this issue on Nov 30, 2024
  24. added a commit that references this issue on Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: This is a bug.E-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.T-typesRelevant to the types team, which will review and decide on the PR/issue.disposition-mergeThis issue / PR is in PFCP or FCP with a disposition to merge it.finished-final-comment-periodThe final comment period is finished for this PR / Issue.regression-untriagedUntriaged performance or correctness regression.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions