chore(turbo-tasks-macros): Remove use of associated items for NativeFunction construction #73929

bgw · 2024-12-13T22:52:19Z

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)
});

Closes PACK-3659

…pressions with functions

bgw · 2024-12-13T22:52:36Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

ijjk · 2024-12-13T23:02:37Z

Stats from current PR

Default Build

General

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
buildDuration	17.2s	15.6s	N/A
buildDurationCached	14.7s	12.6s	N/A
nodeModulesSize	410 MB	410 MB	N/A
nextStartRea..uration (ms)	473ms	473ms	✓

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
1187-HASH.js gzip	50.9 kB	50.9 kB	N/A
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.36 kB	5.36 kB	N/A
bccd1874-HASH.js gzip	53 kB	53 kB	N/A
framework-HASH.js gzip	57.5 kB	57.5 kB	N/A
main-app-HASH.js gzip	232 B	236 B	N/A
main-HASH.js gzip	34.1 kB	34 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	0 B	0 B	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
polyfills-HASH.js gzip	39.4 kB	39.4 kB	✓
Overall change	39.4 kB	39.4 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	512 B	510 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	✓
edge-ssr-HASH.js gzip	265 B	265 B	✓
head-HASH.js gzip	363 B	362 B	N/A
hooks-HASH.js gzip	393 B	392 B	N/A
image-HASH.js gzip	4.49 kB	4.49 kB	N/A
index-HASH.js gzip	268 B	268 B	✓
link-HASH.js gzip	2.35 kB	2.34 kB	N/A
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	323 B	326 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	3.59 kB	3.59 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
_buildManifest.js gzip	749 B	746 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
index.html gzip	523 B	524 B	N/A
link.html gzip	538 B	538 B	✓
withRouter.html gzip	518 B	521 B	N/A
Overall change	538 B	538 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
edge-ssr.js gzip	128 kB	128 kB	N/A
page.js gzip	204 kB	204 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
middleware-b..fest.js gzip	670 B	668 B	N/A
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.2 kB	31.2 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	844 B	844 B	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
523-experime...dev.js gzip	322 B	322 B	✓
523.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	323 kB	323 kB	✓
app-page-exp..prod.js gzip	127 kB	127 kB	✓
app-page-tur..prod.js gzip	140 kB	140 kB	✓
app-page-tur..prod.js gzip	135 kB	135 kB	✓
app-page.run...dev.js gzip	313 kB	313 kB	✓
app-page.run..prod.js gzip	123 kB	123 kB	✓
app-route-ex...dev.js gzip	37.4 kB	37.4 kB	✓
app-route-ex..prod.js gzip	25.5 kB	25.5 kB	✓
app-route-tu..prod.js gzip	25.5 kB	25.5 kB	✓
app-route-tu..prod.js gzip	25.3 kB	25.3 kB	✓
app-route.ru...dev.js gzip	39 kB	39 kB	✓
app-route.ru..prod.js gzip	25.3 kB	25.3 kB	✓
pages-api-tu..prod.js gzip	9.69 kB	9.69 kB	✓
pages-api.ru...dev.js gzip	11.6 kB	11.6 kB	✓
pages-api.ru..prod.js gzip	9.68 kB	9.68 kB	✓
pages-turbo...prod.js gzip	21.7 kB	21.7 kB	✓
pages.runtim...dev.js gzip	27.4 kB	27.4 kB	✓
pages.runtim..prod.js gzip	21.7 kB	21.7 kB	✓
server.runti..prod.js gzip	916 kB	916 kB	✓
Overall change	2.36 MB	2.36 MB	✓

build cache

	vercel/next.js canary	vercel/next.js bgw/no-internally-mutable-consts	Change
0.pack gzip	2.06 MB	2.05 MB	N/A
index.pack gzip	71.7 kB	71.5 kB	N/A
Overall change	0 B	0 B	✓

Diff details

Diff for main-HASH.js

Diff too large to display

Commit: 81432e7

ijjk · 2024-12-13T23:07:59Z

Tests Passed

…n't depend on the value of Self

bgw · 2024-12-16T18:53:36Z

Merge activity

Dec 16, 1:53 PM EST: A user started a stack merge that includes this pull request via Graphite.
Dec 16, 1:54 PM EST: A user merged this pull request with Graphite.

chore(turbo-tasks-macros): Replace use of internally mutable const ex…

6b59610

…pressions with functions

ijjk added the created-by: Turbopack team PRs by the Turbopack team. label Dec 13, 2024

bgw requested review from mischnic and sokra December 13, 2024 22:54

bgw marked this pull request as ready for review December 13, 2024 22:54

Remove associated items entirely, turns out they're not needed, we do…

81432e7

…n't depend on the value of Self

bgw changed the title ~~chore(turbo-tasks-macros): Replace use of internally mutable const expressions with functions~~ chore(turbo-tasks-macros): Remove use of associated items for NativeFunction construction Dec 13, 2024

bgw mentioned this pull request Dec 14, 2024

refactor(turbo-tasks-macros): Strip (already hidden) doc attributes from inline function signatures #73931

Merged

mischnic approved these changes Dec 16, 2024

View reviewed changes

bgw merged commit 7ae9c79 into canary Dec 16, 2024
120 checks passed

bgw deleted the bgw/no-internally-mutable-consts branch December 16, 2024 18:54

github-actions bot added the locked label Dec 31, 2024

github-actions bot locked as resolved and limited conversation to collaborators Dec 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(turbo-tasks-macros): Remove use of associated items for NativeFunction construction #73929

chore(turbo-tasks-macros): Remove use of associated items for NativeFunction construction #73929

bgw commented Dec 13, 2024 •

edited by padmaia

Loading

bgw commented Dec 13, 2024 •

edited

Loading

ijjk commented Dec 13, 2024 •

edited

Loading

ijjk commented Dec 13, 2024 •

edited

Loading

bgw commented Dec 16, 2024 •

edited

Loading

chore(turbo-tasks-macros): Remove use of associated items for NativeFunction construction #73929

chore(turbo-tasks-macros): Remove use of associated items for NativeFunction construction #73929

Conversation

bgw commented Dec 13, 2024 • edited by padmaia Loading

Before

After

bgw commented Dec 13, 2024 • edited Loading

ijjk commented Dec 13, 2024 • edited Loading

Stats from current PR

ijjk commented Dec 13, 2024 • edited Loading

Tests Passed

bgw commented Dec 16, 2024 • edited Loading

Merge activity

bgw commented Dec 13, 2024 •

edited by padmaia

Loading

bgw commented Dec 13, 2024 •

edited

Loading

ijjk commented Dec 13, 2024 •

edited

Loading

ijjk commented Dec 13, 2024 •

edited

Loading

bgw commented Dec 16, 2024 •

edited

Loading