Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Annotate temporary result type when possible
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 <[email protected]>
  • Loading branch information
Gerry Agbobada and adeschamps committed May 3, 2023
commit 6b1869a15d3e37c1e49f22770cf2f6a1a7d3cefa
11 changes: 9 additions & 2 deletions autometrics-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC};
use proc_macro2::TokenStream;
use quote::quote;
use std::env;
use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result};
use syn::{parse_macro_input, ImplItem, ItemFn, ItemImpl, Result, ReturnType, Type};

mod parse;
mod result_labels;
Expand Down Expand Up @@ -60,6 +60,13 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result<TokenStre
// Build the documentation we'll add to the function's RustDocs
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 {
ReturnType::Default => quote! { : () },
ReturnType::Type(_, ref t) if matches!(t.as_ref(), &Type::ImplTrait(_)) => quote! {},
ReturnType::Type(_, ref t) => quote! { : #t },
};

// Wrap the body of the original function, using a slightly different approach based on whether the function is async
let call_function = if sig.asyncness.is_some() {
quote! {
Expand Down Expand Up @@ -144,7 +151,7 @@ fn instrument_function(args: &AutometricsArgs, item: ItemFn) -> Result<TokenStre
AutometricsTracker::start(#gauge_labels)
};

let result = #call_function;
let result #return_type = #call_function;

{
use autometrics::__private::{HistogramLabels, TrackMetrics};
Expand Down
12 changes: 6 additions & 6 deletions autometrics/src/labels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ macro_rules! result_labels {
$crate::__private::spez! {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if we could call spez directly in the proc macro code and insert the token stream directly into the code the macro generates, instead of making it a dependency of the main autometrics crate and having the macro generate code that calls spez (unless there's a reason that cannot work)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling the function spez::spez to get a proc_macro::TokenStream would probably work but it doesn't really bring advantages: calling spez::spez means that there would still be a dependency to spez (even if the macro is moved to autometrics-macros, it would be AM needs AM-macros needs spez), and preparing an input TokenStream from code is one additional step that would be unnecessary added complexity in the macro.

Generating the match Result… TokenStream in the instrument_function directly has the drawback of having bad testability and makes instrument_function readability worse

for val = $e;

match<T: GetLabels, E: GetLabels> &Result<T, E> -> Option<ResultAndReturnTypeLabels> {
match<T, E> &::std::result::Result<T, E> where T: GetLabels, E: GetLabels -> ::std::option::Option<ResultAndReturnTypeLabels> {
match val {
Ok(ok) => Some((
ok.__autometrics_get_labels().unwrap_or(OK_KEY),
Expand All @@ -250,7 +250,7 @@ macro_rules! result_labels {
}
}

match<T, E: GetLabels> &Result<T, E> -> Option<ResultAndReturnTypeLabels> {
match<T, E> &::std::result::Result<T, E> where E: GetLabels -> ::std::option::Option<ResultAndReturnTypeLabels> {
match val {
Ok(ok) => Some((
OK_KEY,
Expand All @@ -263,7 +263,7 @@ macro_rules! result_labels {
}
}

match<T: GetLabels, E> &Result<T, E> -> Option<ResultAndReturnTypeLabels> {
match<T, E> &::std::result::Result<T, E> where T: GetLabels -> ::std::option::Option<ResultAndReturnTypeLabels> {
match val {
Ok(ok) => Some((
ok.__autometrics_get_labels().unwrap_or(OK_KEY),
Expand All @@ -276,7 +276,7 @@ macro_rules! result_labels {
}
}

match<T, E> &Result<T, E> -> Option<ResultAndReturnTypeLabels> {
match<T, E> &::std::result::Result<T, E> -> ::std::option::Option<ResultAndReturnTypeLabels> {
match val {
Ok(ok) => Some((
OK_KEY,
Expand All @@ -289,11 +289,11 @@ macro_rules! result_labels {
}
}

match<T: GetLabels> &T -> Option<ResultAndReturnTypeLabels> {
match<T> &T where T: GetLabels -> ::std::option::Option<ResultAndReturnTypeLabels> {
val.__autometrics_get_labels().map(|label| (label, val.__autometrics_static_str()))
}

match<T> T -> Option<ResultAndReturnTypeLabels> {
match<T> T -> ::std::option::Option<ResultAndReturnTypeLabels> {
None
}
}
Expand Down