search

deislabs / wasi-experimental-http

Experimental outbound HTTP support for WebAssembly and WASI
MIT License
136 stars 33 forks source link

refactor!: change type of `HttpState::add_to_linker`'s `get_cx` parameter #96

Closed dicej closed 2 years ago

dicej commented 2 years ago

BREAKING CHANGE: The type of this parameter has changed from impl Fn(&T) -> &HttpCtx + Send + Sync + Copy + 'static to impl Fn(&T) -> HttpCtx + Send + Sync + 'static. Besides removing the Copy bound (which wasn't needed), I've changed the return type from &HttpCtx to HttpCtx.

The rationale for this is that the return value of get_cx was effectively being cloned anyway immediately after calling it, so there was no efficiency gain from returning a reference. Also, any implementation of the original type would have to ensure the returned reference has the same lifetime as the context parameter, which essentially means the implementation is forced to store the HttpCtx inside the context. That's overkill for e.g. wagi, which has no need for per-context HTTP configuration.

Signed-off-by: Joel Dice joel.dice@gmail.com

ghost commented 2 years ago

CLA assistant check
All CLA requirements met.

dicej commented 2 years ago

I wasn't trying to match anything, and I don't think it's necessary to do so -- HttpState::add_to_linker is just an inherent method, not part of a trait. Perhaps it was originally modeled after wit-bindgen's version, but I don't see any need to adhere to it. Let me know if I'm missing something, though.