Skip to content

Commit

Permalink
chore(turbo-tasks-macros): Remove use of associated items for NativeF…
Browse files Browse the repository at this point in the history
…unction construction (#73929)

After the toolchain upgrade, causing a compilation error in a `#[turbo_tasks::value]` (or somewhere adjacent) can cause this additional non-fatal compilation warning/note:

```
note: erroneous constant encountered
   --> crates/next-api/src/project.rs:580:1
    |
580 | #[turbo_tasks::value_impl]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^ in this procedural macro expansion
    |
   ::: /home/bgw.linux/next.js/turbopack/crates/turbo-tasks-macros/src/lib.rs:119:1
    |
119 | #[proc_macro_error]
    | ------------------- in this expansion of `#[turbo_tasks::value_impl]`
```

The use of const expressions here was a little whacky. This removes the need for the const associated items by inlining their (one and only) use later when creating the `Lazy` static.

This simplifies the macro implementation, reduces the total resulting code size, and makes the expanded output a little more legible.

## Before

```
// ===========================================
// Recursive expansion of the value_impl macro
// ===========================================

impl Completion {
    #[doc = " This will always be the same and never invalidates the reading task."]
    pub fn immutable() -> Vc<Self> {
        let inputs = std::boxed::Box::new(());
        let persistence =
            turbo_tasks::macro_helpers::get_non_local_persistence_from_inputs(&*inputs);
        <Vc<Self> as turbo_tasks::task::TaskOutput>::try_from_raw_vc(turbo_tasks::dynamic_call(
            *COMPLETION_IMPL_IMMUTABLE_FUNCTION_ID,
            inputs as std::boxed::Box<dyn turbo_tasks::MagicAny>,
            persistence,
        ))
    }
}
#[doc(hidden)]
impl Completion {
    #[allow(declare_interior_mutable_const)]
    #[doc(hidden)]
    const COMPLETION_IMPL_IMMUTABLE_FUNCTION: turbo_tasks::macro_helpers::Lazy<
        turbo_tasks::NativeFunction,
    > = turbo_tasks::macro_helpers::Lazy::new(|| {
        #[allow(deprecated)]
        turbo_tasks::NativeFunction::new_function(
            "Completion::immutable".to_owned(),
            turbo_tasks::FunctionMeta { local_cells: false },
            <Completion>::immutable_turbo_tasks_function_inline,
        )
    });
    #[allow(declare_interior_mutable_const)]
    #[doc(hidden)]
    const COMPLETION_IMPL_IMMUTABLE_FUNCTION_ID: turbo_tasks::macro_helpers::Lazy<
        turbo_tasks::FunctionId,
    > = turbo_tasks::macro_helpers::Lazy::new(|| {
        turbo_tasks::registry::get_function_id(&*COMPLETION_IMPL_IMMUTABLE_FUNCTION)
    });
    #[doc = " This will always be the same and never invalidates the reading task."]
    #[doc(hidden)]
    #[deprecated(
        note = "This function is only exposed for use in macros. Do not call it directly."
    )]
    pub(self) fn immutable_turbo_tasks_function_inline() -> Vc<Self> {
        Completion::cell(Completion)
    }
}
#[doc(hidden)]
pub(crate) static COMPLETION_IMPL_IMMUTABLE_FUNCTION: turbo_tasks::macro_helpers::Lazy<
    turbo_tasks::NativeFunction,
> = <Completion>::COMPLETION_IMPL_IMMUTABLE_FUNCTION;
#[doc(hidden)]
pub(crate) static COMPLETION_IMPL_IMMUTABLE_FUNCTION_ID: turbo_tasks::macro_helpers::Lazy<
    turbo_tasks::FunctionId,
> = <Completion>::COMPLETION_IMPL_IMMUTABLE_FUNCTION_ID;
```

## After

```
// ===========================================
// Recursive expansion of the value_impl macro
// ===========================================

impl Completion {
    #[doc = " This will always be the same and never invalidates the reading task."]
    pub fn immutable() -> Vc<Self> {
        let inputs = std::boxed::Box::new(());
        let persistence =
            turbo_tasks::macro_helpers::get_non_local_persistence_from_inputs(&*inputs);
        <Vc<Self> as turbo_tasks::task::TaskOutput>::try_from_raw_vc(turbo_tasks::dynamic_call(
            *COMPLETION_IMPL_IMMUTABLE_FUNCTION_ID,
            inputs as std::boxed::Box<dyn turbo_tasks::MagicAny>,
            persistence,
        ))
    }
}
#[doc(hidden)]
impl Completion {
    #[doc = " This will always be the same and never invalidates the reading task."]
    #[doc(hidden)]
    #[deprecated(
        note = "This function is only exposed for use in macros. Do not call it directly."
    )]
    pub(self) fn immutable_turbo_tasks_function_inline() -> Vc<Self> {
        Completion::cell(Completion)
    }
}
#[doc(hidden)]
pub(crate) static COMPLETION_IMPL_IMMUTABLE_FUNCTION: turbo_tasks::macro_helpers::Lazy<
    turbo_tasks::NativeFunction,
> = turbo_tasks::macro_helpers::Lazy::new(|| {
    #[allow(deprecated)]
    turbo_tasks::NativeFunction::new_function(
        "Completion::immutable".to_owned(),
        turbo_tasks::FunctionMeta { local_cells: false },
        <Completion>::immutable_turbo_tasks_function_inline,
    )
});
#[doc(hidden)]
pub(crate) static COMPLETION_IMPL_IMMUTABLE_FUNCTION_ID: turbo_tasks::macro_helpers::Lazy<
    turbo_tasks::FunctionId,
> = turbo_tasks::macro_helpers::Lazy::new(|| {
    turbo_tasks::registry::get_function_id(&*COMPLETION_IMPL_IMMUTABLE_FUNCTION)
});
```
  • Loading branch information
bgw authored Dec 16, 2024
1 parent 73cfb96 commit 7ae9c79
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 63 deletions.
20 changes: 9 additions & 11 deletions turbopack/crates/turbo-tasks-macros/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -979,10 +979,10 @@ impl NativeFn {
}

pub fn ty(&self) -> Type {
parse_quote! { turbo_tasks::macro_helpers::Lazy<turbo_tasks::NativeFunction> }
parse_quote! { turbo_tasks::NativeFunction }
}

pub fn definition(&self) -> Expr {
pub fn definition(&self) -> TokenStream {
let Self {
function_path_string,
function_path,
Expand All @@ -996,8 +996,8 @@ impl NativeFn {
quote! { new_function }
};

parse_quote! {
turbo_tasks::macro_helpers::Lazy::new(|| {
quote! {
{
#[allow(deprecated)]
turbo_tasks::NativeFunction::#constructor(
#function_path_string.to_owned(),
Expand All @@ -1006,19 +1006,17 @@ impl NativeFn {
},
#function_path,
)
})
}
}
}

pub fn id_ty(&self) -> Type {
parse_quote! { turbo_tasks::macro_helpers::Lazy<turbo_tasks::FunctionId> }
parse_quote! { turbo_tasks::FunctionId }
}

pub fn id_definition(&self, native_function_id_path: &Path) -> Expr {
parse_quote! {
turbo_tasks::macro_helpers::Lazy::new(|| {
turbo_tasks::registry::get_function_id(&*#native_function_id_path)
})
pub fn id_definition(&self, native_function_id_path: &Path) -> TokenStream {
quote! {
turbo_tasks::registry::get_function_id(&*#native_function_id_path)
}
}
}
8 changes: 6 additions & 2 deletions turbopack/crates/turbo-tasks-macros/src/function_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,14 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
#inline_signature #inline_block

#[doc(hidden)]
pub(crate) static #native_function_ident: #native_function_ty = #native_function_def;
pub(crate) static #native_function_ident:
turbo_tasks::macro_helpers::Lazy<#native_function_ty> =
turbo_tasks::macro_helpers::Lazy::new(|| #native_function_def);

#[doc(hidden)]
pub(crate) static #native_function_id_ident: #native_function_id_ty = #native_function_id_def;
pub(crate) static #native_function_id_ident:
turbo_tasks::macro_helpers::Lazy<#native_function_id_ty> =
turbo_tasks::macro_helpers::Lazy::new(|| #native_function_id_def);

#(#errors)*
}
Expand Down
56 changes: 21 additions & 35 deletions turbopack/crates/turbo-tasks-macros/src/value_impl_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
let native_function_ident = get_inherent_impl_function_ident(ty_ident, ident);
let native_function_ty = native_fn.ty();
let native_function_def = native_fn.definition();

let native_function_id_ident = get_inherent_impl_function_id_ident(ty_ident, ident);
let native_function_id_ty = native_fn.id_ty();
let native_function_id_def = native_fn.id_definition(&parse_quote! {
#native_function_ident
});
let native_function_id_def =
native_fn.id_definition(&native_function_ident.clone().into());

let turbo_signature = turbo_fn.signature();
let turbo_block = turbo_fn.static_block(&native_function_id_ident);
Expand All @@ -157,26 +157,23 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
all_definitions.push(quote! {
#[doc(hidden)]
impl #ty {
// By declaring the native function's body within an `impl` block, we ensure that `Self` refers
// to `#ty`. This is necessary because the function's body is originally declared within an
// `impl` block already.
#[allow(declare_interior_mutable_const)]
#[doc(hidden)]
const #native_function_ident: #native_function_ty = #native_function_def;
#[allow(declare_interior_mutable_const)]
#[doc(hidden)]
const #native_function_id_ident: #native_function_id_ty = #native_function_id_def;

// By declaring the native function's body within an `impl` block, we ensure
// that `Self` refers to `#ty`. This is necessary because the function's
// body is originally declared within an `impl` block already.
#(#attrs)*
#[doc(hidden)]
#[deprecated(note = "This function is only exposed for use in macros. Do not call it directly.")]
pub(self) #inline_signature #inline_block
}

#[doc(hidden)]
pub(crate) static #native_function_ident: #native_function_ty = <#ty>::#native_function_ident;
pub(crate) static #native_function_ident:
turbo_tasks::macro_helpers::Lazy<#native_function_ty> =
turbo_tasks::macro_helpers::Lazy::new(|| #native_function_def);
#[doc(hidden)]
pub(crate) static #native_function_id_ident: #native_function_id_ty = <#ty>::#native_function_id_ident;
pub(crate) static #native_function_id_ident:
turbo_tasks::macro_helpers::Lazy<#native_function_id_ty> =
turbo_tasks::macro_helpers::Lazy::new(|| #native_function_id_def);
})
}
}
Expand Down Expand Up @@ -252,15 +249,14 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {

let native_function_ident =
get_trait_impl_function_ident(ty_ident, &trait_ident, ident);

let native_function_ty = native_fn.ty();
let native_function_def = native_fn.definition();

let native_function_id_ident =
get_trait_impl_function_id_ident(ty_ident, &trait_ident, ident);
let native_function_id_ty = native_fn.id_ty();
let native_function_id_def = native_fn.id_definition(&parse_quote! {
#native_function_ident
});
let native_function_id_def =
native_fn.id_definition(&native_function_ident.clone().into());

let turbo_signature = turbo_fn.signature();
let turbo_block = turbo_fn.static_block(&native_function_id_ident);
Expand All @@ -274,37 +270,27 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
#[doc(hidden)]
#[allow(non_camel_case_types)]
trait #inline_extension_trait_ident: std::marker::Send {
#[allow(declare_interior_mutable_const)]
#[doc(hidden)]
const #native_function_ident: #native_function_ty;
#[allow(declare_interior_mutable_const)]
#[doc(hidden)]
const #native_function_id_ident: #native_function_id_ty;

#(#attrs)*
#[doc(hidden)]
#inline_signature;
}

#[doc(hidden)]
impl #impl_generics #inline_extension_trait_ident for #ty #where_clause {
#[allow(declare_interior_mutable_const)]
#[doc(hidden)]
const #native_function_ident: #native_function_ty = #native_function_def;
#[allow(declare_interior_mutable_const)]
#[doc(hidden)]
const #native_function_id_ident: #native_function_id_ty = #native_function_id_def;

#(#attrs)*
#[doc(hidden)]
#[deprecated(note = "This function is only exposed for use in macros. Do not call it directly.")]
#inline_signature #inline_block
}

#[doc(hidden)]
pub(crate) static #native_function_ident: #native_function_ty = <#ty as #inline_extension_trait_ident>::#native_function_ident;
pub(crate) static #native_function_ident:
turbo_tasks::macro_helpers::Lazy<#native_function_ty> =
turbo_tasks::macro_helpers::Lazy::new(|| #native_function_def);
#[doc(hidden)]
pub(crate) static #native_function_id_ident: #native_function_id_ty = <#ty as #inline_extension_trait_ident>::#native_function_id_ident;
pub(crate) static #native_function_id_ident:
turbo_tasks::macro_helpers::Lazy<#native_function_id_ty> =
turbo_tasks::macro_helpers::Lazy::new(|| #native_function_id_def);
});

trait_registers.push(quote! {
Expand Down
25 changes: 10 additions & 15 deletions turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,12 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
let native_function_ident = get_trait_default_impl_function_ident(trait_ident, ident);
let native_function_ty = native_function.ty();
let native_function_def = native_function.definition();

let native_function_id_ident =
get_trait_default_impl_function_id_ident(trait_ident, ident);
let native_function_id_ty = native_function.id_ty();
let native_function_id_def = native_function.id_definition(&parse_quote! {
#native_function_ident
});
let native_function_id_def =
native_function.id_definition(&native_function_ident.clone().into());

trait_methods.push(quote! {
trait_type.register_default_trait_method::<(#(#arg_types,)*)>(stringify!(#ident).into(), *#native_function_id_ident);
Expand All @@ -146,11 +146,6 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
#[doc(hidden)]
#[allow(non_camel_case_types)]
trait #inline_extension_trait_ident: std::marker::Send {
#[allow(declare_interior_mutable_const)]
const #native_function_ident: #native_function_ty;
#[allow(declare_interior_mutable_const)]
const #native_function_id_ident: #native_function_id_ty;

#(#attrs)*
#inline_signature;
}
Expand All @@ -159,19 +154,19 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
// Needs to be explicit 'static here, otherwise we can get a lifetime error
// in the inline signature.
impl #inline_extension_trait_ident for Box<dyn #trait_ident> {
#[allow(declare_interior_mutable_const)]
const #native_function_ident: #native_function_ty = #native_function_def;
#[allow(declare_interior_mutable_const)]
const #native_function_id_ident: #native_function_id_ty = #native_function_id_def;

// put the function body here so that `Self` points to `Box<dyn ...>`
#(#attrs)*
#inline_signature #inline_block
}

#[doc(hidden)]
pub(crate) static #native_function_ident: #native_function_ty = <Box<dyn #trait_ident> as #inline_extension_trait_ident>::#native_function_ident;
pub(crate) static #native_function_ident:
turbo_tasks::macro_helpers::Lazy<#native_function_ty> =
turbo_tasks::macro_helpers::Lazy::new(|| #native_function_def);
#[doc(hidden)]
pub(crate) static #native_function_id_ident: #native_function_id_ty = <Box<dyn #trait_ident> as #inline_extension_trait_ident>::#native_function_id_ident;
pub(crate) static #native_function_id_ident:
turbo_tasks::macro_helpers::Lazy<#native_function_id_ty> =
turbo_tasks::macro_helpers::Lazy::new(|| #native_function_id_def);
});

Some(turbo_fn.static_block(&native_function_id_ident))
Expand Down

0 comments on commit 7ae9c79

Please sign in to comment.