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

Type error regression when using enum called Result #905

Closed
thedjinn opened this issue Sep 4, 2023 · 10 comments
Closed

Type error regression when using enum called Result #905

thedjinn opened this issue Sep 4, 2023 · 10 comments

Comments

@thedjinn
Copy link

thedjinn commented Sep 4, 2023

I am working on a project that uses Prost (via Tonic) to compile a protobuf file for use with gRPC. The file compiles fine with prost 0.11.9, but when switching to prost 0.12.0 the generated Rust file contains type errors.

The minimal protobuf file required to reproduce the problem is this one:

syntax = "proto3";

package fail;

enum Result {
  HELLO = 0;
}

message FailMessage {
  Result result = 1;
}

When attempting to compile the generated rust file the following errors are returned:

error[E0107]: enum takes 0 generic arguments but 2 generic arguments were supplied
 --> /private/tmp/tonicfail/target/debug/build/tonicfail-ca0c3cb604ffea00/out/fail.rs:2:28
  |
2 | #[derive(Clone, PartialEq, ::prost::Message)]
  |                            ^^^^^^^^^^^^^^^^- help: remove these generics
  |                            |
  |                            expected 0 generic arguments
  |
note: enum defined here, with 0 generic parameters
 --> /private/tmp/tonicfail/target/debug/build/tonicfail-ca0c3cb604ffea00/out/fail.rs:9:10
  |
9 | pub enum Result {
  |          ^^^^^^
  = note: this error originates in the derive macro `::prost::Message` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
   --> /private/tmp/tonicfail/target/debug/build/tonicfail-ca0c3cb604ffea00/out/fail.rs:2:28
    |
2   | #[derive(Clone, PartialEq, ::prost::Message)]
    |                            ^^^^^^^^^^^^^^^^
    |                            |
    |                            expected `Result`, found `Result<_, _>`
    |                            expected due to this
    |
    = note: `std::result::Result<_, _>` and `proto::Result` have similar names, but are actually distinct types
note: `std::result::Result<_, _>` is defined in crate `core`
   --> /Users/djinn/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/result.rs:502:1
    |
502 | pub enum Result<T, E> {
    | ^^^^^^^^^^^^^^^^^^^^^
note: `proto::Result` is defined in the current crate
   --> /private/tmp/tonicfail/target/debug/build/tonicfail-ca0c3cb604ffea00/out/fail.rs:9:1
    |
9   | pub enum Result {
    | ^^^^^^^^^^^^^^^
    = note: this error originates in the derive macro `::prost::Message` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
   --> /private/tmp/tonicfail/target/debug/build/tonicfail-ca0c3cb604ffea00/out/fail.rs:2:28
    |
2   | #[derive(Clone, PartialEq, ::prost::Message)]
    |                            ^^^^^^^^^^^^^^^^
    |                            |
    |                            expected `Result`, found `Result<_, _>`
    |                            this expression has type `proto::Result`
    |
    = note: `std::result::Result<_, _>` and `proto::Result` have similar names, but are actually distinct types
note: `std::result::Result<_, _>` is defined in crate `core`
   --> /Users/djinn/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/result.rs:502:1
    |
502 | pub enum Result<T, E> {
    | ^^^^^^^^^^^^^^^^^^^^^
note: `proto::Result` is defined in the current crate
   --> /private/tmp/tonicfail/target/debug/build/tonicfail-ca0c3cb604ffea00/out/fail.rs:9:1
    |
9   | pub enum Result {
    | ^^^^^^^^^^^^^^^
    = note: this error originates in the derive macro `::prost::Message` (in Nightly builds, run with -Z macro-backtrace for more info)

Some errors have detailed explanations: E0107, E0308.
For more information about an error, try `rustc --explain E0107`.
error: could not compile `tonicfail` (bin "tonicfail") due to 3 previous errors

If desired I can provide a minimal Rust project that demonstrates the failed compilation.

@tustvold
Copy link

tustvold commented Sep 4, 2023

We're also running into this in https://github.com/apache/arrow-rs/tree/master/arrow-flight

@anstylian
Copy link
Contributor

I am facing the same one, with message Result and I am preparing a pull request
the issue introduced in this commit: ca73cbe

@LucioFranco
Copy link
Member

@anstylian does your commit work? Happy to review a PR that fixes it.

@anstylian
Copy link
Contributor

I have test it locally and works, but because you asked me that I have created a unit-test for it.
I have created a unit-test for this issue and it can be found here.
The above branch also contains a fix for the nix flakes of prost. cargo test was not working for me, running it from inside the tests folder. I need to add cmake and ninja in buildInputs.
My "test" basically makes compilation to fail if there is a collision between a users defined Result and Rust Result type.

After that I have created this where I have added the same unit-test for Option type.

@LucioFranco you can let me know which branch you want to merge and I can open you a new PR if its not the one already open.

@Victor-N-Suadicani
Copy link
Contributor

Ran into this issue as well. Would really appreciate a swift bug fix release 🙏

@c0mm4nd
Copy link

c0mm4nd commented Sep 13, 2023

Ran into this issue as well. Would really appreciate a swift bug fix release 🙏+1

@LucioFranco
Copy link
Member

@anstylian yeah go ahead and make a PR with both. Should be easier to review.

@anstylian
Copy link
Contributor

@LucioFranco
Currently I have 3 PR for this issue, you can choose to merge one, and close the others. Each PR contains its own description.

  1. PR Fixes 905, fix: Use full path of Result type + unit test + Option unti test #917
  2. PR Fixes 905, fix: Use full path of Result type + unit test #916
  3. PR Fixes: #905, fix: Use full path of Result type #906

@thedjinn
Copy link
Author

Thanks everyone for the work involved in getting this resolved.

@LucioFranco
Copy link
Member

This has been released as v0.12.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants