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

Cycle detected with significant destructor from 1.58 #93296

Closed
SuperFluffy opened this issue Jan 25, 2022 · 3 comments
Closed

Cycle detected with significant destructor from 1.58 #93296

SuperFluffy opened this issue Jan 25, 2022 · 3 comments
Labels
C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression.

Comments

@SuperFluffy
Copy link
Contributor

SuperFluffy commented Jan 25, 2022

MWE: https://github.com/SuperFluffy/google_proto_struct_mwe (see branches master, 1.57, 1.56, nightly).

I am getting a confusing error message when compiling a generated struct (see below). This compiles in 1.56 and 1.57, but no longer in 1.58.* or nightly.

Note that I cannot reproduce this locally! It happily builds on my local machine (MacOS M1, rustc 1.58.1), but it does not in CI (ubuntu-latest, 1.58.1).

I can reproduce this locally, as pointed out in #93296 (comment)

error[E0391]: cycle detected when computing when `Struct` has a significant destructor
 --> src/lib.rs:2:1
  |
2 | pub struct Struct {
  | ^^^^^^^^^^^^^^^^^
  |
  = note: ...which immediately requires computing when `Struct` has a significant destructor again
  = note: cycle used when computing whether `Struct` has a significant drop

Code

The code reproduced below is constructed by prost-build. It can be generated from scratch or imported via prost-types. The error occurs in both cases.

I have inlined the code into the repository to demonstrate the issue without extra dependencies.

I have made an MWE here: https://github.com/SuperFluffy/google_proto_struct_mwe

The error as copied verbatim can be seen in this github action: https://github.com/SuperFluffy/google_proto_struct_mwe/runs/4935446281

#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Struct {
    #[prost(map = "string, message", tag = "1")]
    pub fields: ::std::collections::HashMap<::prost::alloc::string::String, Value>,
}

#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Value {
    #[prost(oneof = "value::Kind", tags = "1, 2, 3, 4, 5, 6")]
    pub kind: ::core::option::Option<value::Kind>,
}
pub mod value {
    #[derive(Clone, PartialEq, ::prost::Oneof)]
    pub enum Kind {
        #[prost(enumeration = "super::NullValue", tag = "1")]
        NullValue(i32),
        #[prost(double, tag = "2")]
        NumberValue(f64),
        #[prost(string, tag = "3")]
        StringValue(::prost::alloc::string::String),
        #[prost(bool, tag = "4")]
        BoolValue(bool),
        #[prost(message, tag = "5")]
        StructValue(super::Struct),
        #[prost(message, tag = "6")]
        ListValue(super::ListValue),
    }
}

#[derive(Clone, PartialEq, ::prost::Message)]
pub struct ListValue {
    #[prost(message, repeated, tag = "1")]
    pub values: ::prost::alloc::vec::Vec<Value>,
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)]
#[repr(i32)]
pub enum NullValue {
    NullValue = 0,
}

I expected to see this happen:

It should compile. It used to compile, but it started breaking with 1.58.0 and 1.58.1, and also nightly. It works in 1.57 and 1.56.

Instead, this happens:

Compilation error, see above

Meta

Works

# Local

rustc 1.58.1 (db9d1b20b 2022-01-20)
binary: rustc
commit-hash: db9d1b20bba1968c1ec1fc49616d4742c1725b4b
commit-date: 2022-01-20
host: aarch64-apple-darwin
release: 1.58.1
LLVM version: 13.0.0

# CI

rustc 1.57.0 (f1edd0429 2021-11-29)
binary: rustc
commit-hash: f1edd0429582dd29cccacaf50fd134b05593bd9c
commit-date: 2021-11-29
host: x86_64-unknown-linux-gnu
release: 1.57.0
LLVM version: 13.0.0

Does not work

# CI

rustc 1.58.1 (db9d1b20b 2022-01-20)
binary: rustc
commit-hash: db9d1b20bba1968c1ec1fc49616d4742c1725b4b
commit-date: 2022-01-20
host: x86_64-unknown-linux-gnu
release: 1.58.1
LLVM version: 13.0.0

rustc 1.60.0-nightly (51126be1b 2022-01-24)
binary: rustc
commit-hash: 51126be1b260216b41143469086e6e6ee567647e
commit-date: 2022-01-24
host: x86_64-unknown-linux-gnu
release: 1.60.0-nightly
LLVM version: 13.0.0

Backtraces

Nothing in there; see the CI reports with RUST_BACKTRACE=1 enabled.

Version it worked on

1.57 (see 1.57 branch on cI)

Version with regression

1.58

@SuperFluffy SuperFluffy added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Jan 25, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jan 25, 2022
@steffahn
Copy link
Member

The way to reproduce this locally is to activate the -W rust-2021-compatibility flag that you also have on CI. Haven’t looked at the code at all here, but this could be a duplicate of #92725.

@SuperFluffy
Copy link
Contributor Author

Thank you, you are right. I am now able to reproduce this locally. I found #92725, but haven't made the connection because of the "closures" bit.

@apiraino
Copy link
Contributor

thanks @steffahn for the tip. This issue looks to be duplicate, so I'm going to close it.

(but feel free to reopen if it's not)

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jan 26, 2022
dfinity-bot pushed a commit to dfinity/ic that referenced this issue Sep 2, 2022
Rust 2021

Move to rust version 2021

Steps: 
- Bumpt rust to 1.61 to run `cargo fix --edition`. I encountered [this](rust-lang/rust#93296), which was resolved by bumping rust.
- `cargo fix --edition`
- `find ./ -type f -name "*Cargo\.toml"  -exec sed -i 's/2018/2021/g' {} \;`
- Manually remove clippy warnings. Mostly removing inline `std::convert::TryFrom`, which now is automatically imported.
- `allow[clippy::await_holding_lock]` for `replica_test` and and ledger canister notified test. 
- `find ./ -type f -name "*BUILD\.bazel"  -exec sed -i '/2021/d' {} \;`
- Go back to rust 1.60


Special attention is required for these files, where `cargo fix --edition`  did a larger change
``` 
bitcoin/adapter/src/connectionmanager.rs
http_handler/src/lib.rs
messaging/src/state_machine/tests.rs
nns/integration_tests/src/fuzz.rs
transport/src/data_plane.rs
xnet/endpoint/src/lib.rs

./experimental/
./bazel/
rustfmt.conf
./third_party/
``` 

See merge request dfinity-lab/public/ic!7306
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. regression-untriaged Untriaged performance or correctness regression.
Projects
None yet
Development

No branches or pull requests

4 participants