Provide a way for custom derives to know if they were invoked via `#[derive_const]` #118580

fmease · 2023-12-04T01:18:36Z

cc @rust-lang/wg-macros
r? compiler and libs-api

fmease · 2023-12-04T01:19:20Z

library/proc_macro/src/lib.rs

+#[unstable(feature = "derive_const", issue = "none")]
+#[derive(Default, Clone)]
+#[non_exhaustive]
+pub struct DeriveExpansionOptions;


bikeshed name

fmease · 2023-12-04T01:19:33Z

library/proc_macro/src/lib.rs

+#[unstable(feature = "derive_const", issue = "none")]
+impl !Send for DeriveExpansionOptions {}
+#[unstable(feature = "derive_const", issue = "none")]
+impl !Sync for DeriveExpansionOptions {}


for forward compatibility

fmease · 2023-12-04T11:29:35Z

tests/ui/rfcs/rfc-2632-const-trait-impl/const_derives/signature-proc-macro-derive.stderr

+   | incorrect number of function parameters
+   | incorrect number of function parameters


This duplication makes me sad but it's annoying to fix. I'd need to modify note_type_err just for this.

fmease · 2023-12-04T15:51:29Z

Ah, I need to update r-a, too (proc-macro-srv/src/server.rs).

rustbot · 2023-12-04T18:06:00Z

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

fmease · 2023-12-04T18:06:23Z

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

Pretty sure that this in-tree change is unavoidable.

fmease · 2023-12-14T14:33:22Z

@rust-lang/libs-api, would you like to see an ACP for this first?

TL;DR: Allow derive-proc-macros to have the signature fn(TokenStream, DeriveExpansionOptions) -> TokenStream where DeriveExpansionOptions is an abstract type with the method fn is_const(&self) -> bool which returns whether the macro was invoked via #[derive_const] or #[derive]. Afaik, #[derive_const] isn't backed by an RFC just like the rest of const_trait_impl/effects. Just happy to receive feedback from you if you're roughly okay with the shape of this unstable API or not :)

petrochenkov · 2023-12-24T11:27:53Z

(I should get to this on new year holidays.)

bors · 2023-12-26T05:13:17Z

☔ The latest upstream changes (presumably #119146) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2024-01-04T10:09:32Z

☔ The latest upstream changes (presumably #119542) made this pull request unmergeable. Please resolve the merge conflicts.

fmease · 2024-01-05T14:47:27Z

compiler/rustc_passes/src/check_attr.rs

+                && tcx
+                    .features()
+                    .declared_lib_features
+                    .iter()
+                    .any(|&(feature, _)| feature == sym::derive_const)


I think I should actually be able to do:

Suggested change

&& tcx

.features()

.declared_lib_features

.iter()

.any(|&(feature, _)| feature == sym::derive_const)

&& tcx.features().declared(sym::derive_const)

or is it

Suggested change

&& tcx

.features()

.declared_lib_features

.iter()

.any(|&(feature, _)| feature == sym::derive_const)

&& tcx.features().active(sym::derive_const)

fmease · 2024-01-06T12:10:16Z

library/proc_macro/src/bridge/client.rs

        trait_name: &'static str,
        attributes: &'static [&'static str],
-        expand: impl Fn(crate::TokenStream) -> crate::TokenStream + Copy,
+        expand: impl ~const ExpandCustomDerive<Discriminant>,


It's unfortunate that I had to enable const_trait_impl and effects to make this work. If I hadn't done it, I wouldn't have been able to call into_fn in this const fn.

fmease · 2024-01-06T12:13:54Z

library/proc_macro/src/bridge/client.rs

@@ -506,3 +510,26 @@ impl ProcMacro {
        ProcMacro::Bang { name, client: Client::expand1(expand) }
    }
 }
+
+#[const_trait]
+pub trait ExpandCustomDerive<Discriminant> {


I know that adding a “discriminant” to this trait is slightly ugly but it's necessary to make the two Fn impls non-overlapping. Sadly, Fn(T) and Fn(T, U) are considered to be overlapping which obviously makes sense looking at their desugared form Fn<(T,)> and Fn<(T, U)>. Theoretically a single type can implement both of those trait refs (under #![feature(unboxed_closures)]).

fmease · 2024-01-06T12:18:20Z

library/proc_macro/src/bridge/client.rs

@@ -484,12 +484,16 @@ impl ProcMacro {
        }
    }

-    pub const fn custom_derive(
+    pub const fn custom_derive<Discriminant>(


I really hope that this type parameter doesn't “leak” in the sense that Discriminant doesn't lead to inference failures. See my other comment for why Discriminant is necessary.

By the way, I can't replace this type parameter with impl-Trait since nested impl-Traits aren't supported at the moment (here: impl ~const ExpandCustomDerive<impl Discriminant>). It would at least prevent users from being able to explicitly specify it. Of course, this function is not considered part of the public API since this entire module is #[doc(hidden)] if I remember correctly (hence no stability attributes anywhere in this file).

fmease · 2024-01-06T12:26:09Z

library/proc_macro/src/bridge/client.rs

+    F: Fn(crate::TokenStream, crate::DeriveExpansionOptions) -> crate::TokenStream + Copy,
+{
+    fn into_fn(self) -> impl Fn(crate::TokenStream) -> crate::TokenStream + Copy {
+        move |input| self(input, Default::default())


This feels a bit weird and it demonstrates that DeriveExpansionOptions is truly a “sham”. I fear that the way it's wired right now, this impl isn't extensible. Consider the hypothetical in which we do want to store something meaningful in DeriveExpansionOptions, we wouldn't be able to actually retrieve it, only receive it with an RPC, e.g. via FreeFunctions. Maybe this is a non-issue, I'm just rambling at this point ^^'

fmease · 2024-01-15T01:13:41Z

Nominating for t-libs-api

TL;DR: Allow derive-proc-macros to have the signature fn(TokenStream, DeriveExpansionOptions) -> TokenStream where DeriveExpansionOptions is an abstract type with the method fn is_const(&self) -> bool which returns whether the macro was invoked via #[derive_const] or #[derive]. Afaik, #[derive_const] isn't backed by an RFC just like the rest of const_trait_impl/effects. Just happy to receive feedback from you if you're roughly okay with the shape of this unstable API or not :)

m-ou-se · 2024-01-16T16:33:15Z

We briefly discussed this in the libs-api meeting. As an unstable feature in proc_macro, this is fine. Feel free to go ahead, as long as there is a tracking issue attached to these unstable APIs and there will be an libs-api FCP before stabilization.

petrochenkov · 2024-01-19T18:08:29Z

Both #[derive_const] itself, and this, are maximally ad hoc tools for addressing the issue.

How macros currently take their input - by a token stream, or two token streams if the macro has two inputs like #[attr(input1)] INPUT2.

This PR adds a third backdoor input tailored for a very specific use case.
The #[derive_const] added a whole new macro just to add one configuration bit to already existing #[derive] macro.

I would suggest keeping the current model and using already existing token-based methods for configuring macro behaviors.

Either helper attributes, like many derive macros, including derive_more, already do
Or giving derive macros the second input token stream - #[derive(Clone(input1))] INPUT2, that would also improve consistency with attributes.

Then the macros themselves can process the inputs as they want.
E.g. built-in macros could conventionally take a const token to produce a const impl - #[derive(Clone(const))] struct S;, others may accept more configuration.

If #[derive] needs some sugar to pass const to multiple derive macros that it applies at once, then it can grow such sugar itself, instead of delegating to a whole new special-purpose macro.
E.g. #[derive(const: Clone, PartialEq, PartialOrd)] struct S; or anything else, tokens inside the parentheses are an arbitrary token stream, I remind you, so #[derive] can create its own language for its input, like any other attribute macro.

petrochenkov · 2024-01-20T10:00:59Z

On a second thought, there's a third way to pass inputs - the "global data" in struct Rustc.

Proc macro methods without arguments like Span::call_site() take their return values from that global data.
I also planned to add some more data there to be available to proc macros, like the kind of brackets used by the macro.

If "derive expansion options" are placed into struct Rustc, then there's no need to add a second parameter to #[proc_macro_derive] functions for passing them.

petrochenkov · 2024-01-20T10:03:59Z

What other similar configuration values we may need in the future?

Will we need to pass them to derive macros only, or for attribute possibly for attribute or fn-like macros too?
With derive macros we have an intermediate layer, the #[derive] macro which has access to compiler magic to set such configuration values, but for attribute and fn-like macros we do not.

bors · 2024-02-16T10:12:40Z

☔ The latest upstream changes (presumably #120486) made this pull request unmergeable. Please resolve the merge conflicts.

Dylan-DPC · 2024-02-18T08:34:29Z

@fmease any updates on this? thanks

fmease · 2024-02-18T08:38:54Z

No updates yet, my plan is to discuss petrochenkov's very good feedback with the rest of project-const-traits as well as with wg-macros.
@rustbot blocked

fmease · 2024-07-01T22:25:56Z

I've updated #118304 to mention petrochenkov's valid criticism and I will close this PR to preserve history instead of keeping it open until we work out a redesign of this feature and implementing it here.

rustbot assigned petrochenkov Dec 4, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 4, 2023

fmease added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 4, 2023

fmease commented Dec 4, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

fmease force-pushed the libproc_macro-is_derive_const branch from 87da6b8 to 1e4af5e Compare December 4, 2023 01:32

This comment has been minimized.

Sign in to view

fmease force-pushed the libproc_macro-is_derive_const branch from 1e4af5e to 25c6726 Compare December 4, 2023 11:26

fmease marked this pull request as ready for review December 4, 2023 11:27

fmease commented Dec 4, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

fmease force-pushed the libproc_macro-is_derive_const branch from 25c6726 to 541dd8c Compare December 4, 2023 15:17

This comment has been minimized.

Sign in to view

fmease force-pushed the libproc_macro-is_derive_const branch from 541dd8c to f4480cd Compare December 4, 2023 18:05

fmease force-pushed the libproc_macro-is_derive_const branch from f4480cd to 882ab9b Compare January 3, 2024 09:30

Introduce proc_macro::DeriveExpansionOptions

3768c8f

fmease force-pushed the libproc_macro-is_derive_const branch from 882ab9b to 3768c8f Compare January 5, 2024 14:03

fmease commented Jan 5, 2024

View reviewed changes

fmease commented Jan 6, 2024

View reviewed changes

fmease added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jan 15, 2024

m-ou-se removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jan 16, 2024

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2024

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 18, 2024

fmease mentioned this pull request Jul 1, 2024

Provide a way for derives to know if they were invoked with #[derive_const] #118304

Open

fmease closed this Jul 1, 2024

fmease deleted the libproc_macro-is_derive_const branch July 1, 2024 22:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide a way for custom derives to know if they were invoked via `#[derive_const]` #118580

Provide a way for custom derives to know if they were invoked via `#[derive_const]` #118580

fmease commented Dec 4, 2023 •

edited

Loading

fmease Dec 4, 2023

fmease Dec 4, 2023

This comment has been minimized.

This comment has been minimized.

fmease Dec 4, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

fmease commented Dec 4, 2023

rustbot commented Dec 4, 2023

fmease commented Dec 4, 2023 •

edited

Loading

fmease commented Dec 14, 2023 •

edited

Loading

petrochenkov commented Dec 24, 2023

bors commented Dec 26, 2023

bors commented Jan 4, 2024

fmease Jan 5, 2024

fmease Jan 6, 2024 •

edited

Loading

fmease Jan 6, 2024 •

edited

Loading

fmease Jan 6, 2024 •

edited

Loading

fmease Jan 6, 2024 •

edited

Loading

fmease commented Jan 15, 2024

m-ou-se commented Jan 16, 2024

petrochenkov commented Jan 19, 2024

petrochenkov commented Jan 20, 2024

petrochenkov commented Jan 20, 2024

bors commented Feb 16, 2024

Dylan-DPC commented Feb 18, 2024

fmease commented Feb 18, 2024

fmease commented Jul 1, 2024 •

edited

Loading

		\| incorrect number of function parameters
		\| incorrect number of function parameters

Provide a way for custom derives to know if they were invoked via #[derive_const] #118580

Provide a way for custom derives to know if they were invoked via #[derive_const] #118580

Conversation

fmease commented Dec 4, 2023 • edited Loading

fmease Dec 4, 2023

Choose a reason for hiding this comment

fmease Dec 4, 2023

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

fmease Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

fmease commented Dec 4, 2023

rustbot commented Dec 4, 2023

fmease commented Dec 4, 2023 • edited Loading

fmease commented Dec 14, 2023 • edited Loading

petrochenkov commented Dec 24, 2023

bors commented Dec 26, 2023

bors commented Jan 4, 2024

fmease Jan 5, 2024

Choose a reason for hiding this comment

fmease Jan 6, 2024 • edited Loading

Choose a reason for hiding this comment

fmease Jan 6, 2024 • edited Loading

Choose a reason for hiding this comment

fmease Jan 6, 2024 • edited Loading

Choose a reason for hiding this comment

fmease Jan 6, 2024 • edited Loading

Choose a reason for hiding this comment

fmease commented Jan 15, 2024

m-ou-se commented Jan 16, 2024

petrochenkov commented Jan 19, 2024

petrochenkov commented Jan 20, 2024

petrochenkov commented Jan 20, 2024

bors commented Feb 16, 2024

Dylan-DPC commented Feb 18, 2024

fmease commented Feb 18, 2024

fmease commented Jul 1, 2024 • edited Loading

Provide a way for custom derives to know if they were invoked via `#[derive_const]` #118580

Provide a way for custom derives to know if they were invoked via `#[derive_const]` #118580

fmease commented Dec 4, 2023 •

edited

Loading

fmease Dec 4, 2023 •

edited

Loading

fmease commented Dec 4, 2023 •

edited

Loading

fmease commented Dec 14, 2023 •

edited

Loading

fmease Jan 6, 2024 •

edited

Loading

fmease Jan 6, 2024 •

edited

Loading

fmease Jan 6, 2024 •

edited

Loading

fmease Jan 6, 2024 •

edited

Loading

fmease commented Jul 1, 2024 •

edited

Loading