salsa-rs / salsa

A generic framework for on-demand, incrementalized computation. Inspired by adapton, glimmer, and rustc's query system.
https://salsa-rs.netlify.app/
Apache License 2.0
2.14k stars 152 forks source link

Parse queries that have a `()` return type #149

Open anp opened 5 years ago

anp commented 5 years ago

If I have a side-effectful query:

fn update(&self, scope: ScopeId) -> ()

clippy warns me:

warning: unneeded unit return type
   --> src/lib.rs:107:40
    |
107 |     fn update(&self, scope: ScopeId) -> ();
    |                                        ^^^^^ help: remove the `-> ()`
    |

and if I remove the explicit unit return, salsa can't parse my query group definition:

error: custom attribute panicked
  --> src/lib.rs:99:1
   |
99 | #[salsa::query_group(ComponentStorage)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: message: unsupported return type `Default` of `surface`

I can ignore this clippy lint on this particular item, but it would be nice to be able to type the function signatures similarly to what I would do outside of salsa. Is there something different I should be doing?

Zonico6 commented 5 years ago

I'm still trying to understand the codebase so I might be wrong, but I think that side-effectful shouldn't be a thing anyway. In the README under key ideas it states, that salsa's queries behave like functions without side effects. I know that there are ways to make a query results be recomputed on every revision but I'd still argue that side-effects defeat the purpose of queries in salsa and that it's indeed a good thing that the compiler warns about it.