Add a type annotation to generated code to allow type inference to work. #74
Conversation
Since the generated code wraps the original function body in an expression of the form
`let return = { /* original function body /* };`, this can cause type inference to not work on
return expressions like the following:
```rust
#[autometrics]
fn foo() -> Vec<usize> {
(0..10).collect()
}
```
This patch puts the function's return type as an explicit type annotation on the `let return =` in
the generated code.
There was a problem hiding this comment.
Thanks @adeschamps for the PR! And good catch about this issue 😊
Could you add a couple of tests to make sure that this works with various types of return values? The one that sticks out is the impl Trait but I wonder if there are any other cases that might cause problems.
| /// | ||
| /// fn my_function(&self) { | ||
| /// fn my_function(&self) -> Vec<usize> { | ||
| /// # (0..10).collect() |
There was a problem hiding this comment.
I'd prefer to have this as a test instead of in the example
| let metrics_docs = create_metrics_docs(&prometheus_url, &function_name, args.track_concurrency); | ||
|
|
||
| // Type annotation to allow type inference to work on return expressions (such as `.collect()`). | ||
| let return_type = match sig.output { |
There was a problem hiding this comment.
What happens if the function returns an impl Trait?
Maybe we could try to be done with #61 (I can edit the EDIT: Now that the CI is enabled though, we can see that the axum example fails to compile because of exactly that anonymous type issue |
|
Ah yes, I didn't consider fn foo() -> ReturnType {
// original function
fn __internal_foo() -> ReturnType {
// ...
}
/* header */
let result = __internal_foo(args...);
/* footer */
result
}I'll experiment with this. |
|
@adeschamps it might also be worth looking at how the |
This change is necessary to make spez work, otherwise the compiler gets confused about the specialization to choose in the spez::Match<i> generated traits. The issue is not spez-specific, as the macro is simple sugar over the auto-deref specialization trick in general. This commit is also the "maximalist" solution to #74, as it simply ignores the return type if it has syntax incompatible with the context we want to use. Closes: #74
This change is necessary to make spez work, otherwise the compiler gets confused about the specialization to choose in the spez::Match<i> generated traits. The issue is not spez-specific, as the macro is simple sugar over the auto-deref specialization trick in general. This commit is also the "maximalist" solution to #74, as it simply ignores the return type if it has syntax incompatible with the context we want to use. Closes: #74 Co-Authored-By: Anthony Deschamps <anthony.j.deschamps@gmail.com>
|
Actually I had to bring parts of the changes to make a feature in #61 to work. I dodged the anonymous type issue by simply not emitting any code for this specific case |
3 participants
Since the generated code wraps the original function body in an expression of the form
let return = { /* original function body /* };, this can cause type inference to not work onreturn expressions like the following:
This patch puts the function's return type as an explicit type annotation on the
let return =inthe generated code.