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

Bug info about demux_enum #1201

Closed
neurusL opened this issue May 17, 2024 · 3 comments · Fixed by #1280
Closed

Bug info about demux_enum #1201

neurusL opened this issue May 17, 2024 · 3 comments · Fixed by #1280
Assignees
Labels
debugging/observability hydroflow syntax Hydroflow's custom surface syntax

Comments

@neurusL
Copy link

neurusL commented May 17, 2024

Hi, we are a team from Carnegie Mellon University using Hydroflow for reactive programming. The idea Hydroflow representing code as dataflow is really inspiring and convenient for us to use. Thanks for your great project!
A slight issue we encounter is when we use the operator demux_enum, the debug info auto-generated by rust compiler is a bit confusing. For example, we didn't notice there's a requirement for exhaustive matching for succeeding dataflow of demux_enum, while the error message is, for example:

   --> src/server.rs:33:29
   |
33 |       let mut hf: Hydroflow = hydroflow_syntax! {
   |  _____________________________^
34 | |         // Define shared inbound and outbound channels
35 | |         outbound_chan = union() -> dest_sink_serde(outbound);
36 | |         inbound_chan = source_stream_serde(inbound)
...  |
72 | |      
73 | |     };
   | |_____^ expected `(_, ())`, found `()`
   |
   = note:  expected tuple `(_, ())`
           found unit type `()`

Another example is when succeeding dataflow's input type doesn't match demux_enum's output type, the error message refers to demux_enum rather than succeeding dataflow, for example:

  --> src/server.rs:39:16
   |
39 |             -> demux_enum::<MessageWithAddr>();
   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `(SocketAddr,)`, found `()`
   |
   = note:  expected tuple `(std::net::SocketAddr,)`
           found unit type `()`
   = note: required for `(impl Pusherator<Item = ()>, (_, ()))` to implement `PusheratorListForItems<((std::net::SocketAddr,), ((std::net::SocketAddr,), ()))>`
   = note: 3 redundant requirements hidden
   = note: required for `(impl Pusherator<Item = (SocketAddr,)>, (impl Pusherator<Item = (SocketAddr,)>, (..., ...)))` to implement `PusheratorListForItems<((std::net::SocketAddr,), ((std::net::SocketAddr,), ((std::net::SocketAddr, protocol::Entry), ((std::net::SocketAddr,), ((std::net::SocketAddr,), ())))))>`

We feel like maybe in future sprints, you can improve these bug reports to make Hydroflow more user friendly. Thanks!

@MingweiSamuel MingweiSamuel self-assigned this May 17, 2024
@MingweiSamuel MingweiSamuel added bug Something isn't working hydroflow syntax Hydroflow's custom surface syntax debugging/observability and removed bug Something isn't working labels May 17, 2024
MingweiSamuel added a commit to MingweiSamuel/hydroflow that referenced this issue May 18, 2024
MingweiSamuel added a commit to MingweiSamuel/hydroflow that referenced this issue May 18, 2024
@MingweiSamuel
Copy link
Member

Will simplify the implementation a bit. Hands a bit tied by the way macros work, but should be a bit better #1204

MingweiSamuel added a commit that referenced this issue Jun 5, 2024
…1280)

Inspired by fixing the spans in `demux_enum` in #1271
* re-span `#root` to `op_span` for better diagnostics
* use better span `func.inputs` in `demux` and `demux_enum`
* clippy fixups in `source_json`, `source_stdin`
* fix #1201 (for the most part)
@MingweiSamuel
Copy link
Member

Can't really make the error messages perfect due to how proc macros work but they should be a lot better now. Feel free to post any weird error messages you get and we can see if it can be improved further

@neurusL
Copy link
Author

neurusL commented Jun 6, 2024

Thanks a lot! It's indeed more readable now.

jhellerstein pushed a commit that referenced this issue Jun 17, 2024
…1280)

Inspired by fixing the spans in `demux_enum` in #1271
* re-span `#root` to `op_span` for better diagnostics
* use better span `func.inputs` in `demux` and `demux_enum`
* clippy fixups in `source_json`, `source_stdin`
* fix #1201 (for the most part)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debugging/observability hydroflow syntax Hydroflow's custom surface syntax
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants