Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
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
11 changes: 7 additions & 4 deletions compiler/rustc_expand/src/proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl base::BangProcMacro for BangProcMacro {

let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace;
let strategy = exec_strategy(ecx);
let server = proc_macro_server::Rustc::new(ecx);
let server = proc_macro_server::Rustc::new(ecx, Default::default());
self.client.run(&strategy, server, input, proc_macro_backtrace).map_err(|e| {
ecx.dcx().emit_err(errors::ProcMacroPanicked {
span,
Expand Down Expand Up @@ -90,7 +90,7 @@ impl base::AttrProcMacro for AttrProcMacro {

let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace;
let strategy = exec_strategy(ecx);
let server = proc_macro_server::Rustc::new(ecx);
let server = proc_macro_server::Rustc::new(ecx, Default::default());
self.client.run(&strategy, server, annotation, annotated, proc_macro_backtrace).map_err(
|e| {
let mut err = ecx.dcx().struct_span_err(span, "custom attribute panicked");
Expand All @@ -114,7 +114,7 @@ impl MultiItemModifier for DeriveProcMacro {
span: Span,
_meta_item: &ast::MetaItem,
item: Annotatable,
_is_derive_const: bool,
is_derive_const: bool,
) -> ExpandResult<Vec<Annotatable>, Annotatable> {
// We need special handling for statement items
// (e.g. `fn foo() { #[derive(Debug)] struct Bar; }`)
Expand Down Expand Up @@ -142,7 +142,10 @@ impl MultiItemModifier for DeriveProcMacro {
});
let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace;
let strategy = exec_strategy(ecx);
let server = proc_macro_server::Rustc::new(ecx);
let server = proc_macro_server::Rustc::new(
ecx,
proc_macro_server::ExpansionOptions { is_derive_const },
);
match self.client.run(&strategy, server, input, proc_macro_backtrace) {
Ok(stream) => stream,
Err(e) => {
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_expand/src/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,17 +397,19 @@ pub(crate) struct Rustc<'a, 'b> {
mixed_site: Span,
krate: CrateNum,
rebased_spans: FxHashMap<usize, Span>,
options: ExpansionOptions,
}

impl<'a, 'b> Rustc<'a, 'b> {
pub fn new(ecx: &'a mut ExtCtxt<'b>) -> Self {
pub fn new(ecx: &'a mut ExtCtxt<'b>, options: ExpansionOptions) -> Self {
let expn_data = ecx.current_expansion.id.expn_data();
Rustc {
def_site: ecx.with_def_site_ctxt(expn_data.def_site),
call_site: ecx.with_call_site_ctxt(expn_data.call_site),
mixed_site: ecx.with_mixed_site_ctxt(expn_data.call_site),
krate: expn_data.macro_def_id.unwrap().krate,
rebased_spans: FxHashMap::default(),
options,
ecx,
}
}
Expand All @@ -417,6 +419,11 @@ impl<'a, 'b> Rustc<'a, 'b> {
}
}

#[derive(Default)]
pub(crate) struct ExpansionOptions {
pub(crate) is_derive_const: bool,
}

impl server::Types for Rustc<'_, '_> {
type FreeFunctions = FreeFunctions;
type TokenStream = TokenStream;
Expand Down Expand Up @@ -503,6 +510,10 @@ impl server::FreeFunctions for Rustc<'_, '_> {
}
self.sess().dcx.emit_diagnostic(diag);
}

fn is_derive_const(&mut self) -> bool {
self.options.is_derive_const
}
}

impl server::TokenStream for Rustc<'_, '_> {
Expand Down
68 changes: 53 additions & 15 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2293,10 +2293,17 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}

let tcx = self.tcx;
let Some(token_stream_def_id) = tcx.get_diagnostic_item(sym::TokenStream) else {

let Some(token_stream) = tcx
.get_diagnostic_item(sym::TokenStream)
.and_then(|did| tcx.type_of(did).no_bound_vars())
else {
return;
};
let Some(token_stream) = tcx.type_of(token_stream_def_id).no_bound_vars() else {
let Some(derive_expansion_options) = tcx
.get_diagnostic_item(sym::DeriveExpansionOptions)
.and_then(|did| tcx.type_of(did).no_bound_vars())
else {
return;
};

Expand Down Expand Up @@ -2332,8 +2339,24 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
Unsafety::Normal,
Abi::Rust,
);
let expected_options_sig = tcx.mk_fn_sig(
[token_stream, derive_expansion_options],
token_stream,
false,
Unsafety::Normal,
Abi::Rust,
);

if let Err(terr) = ocx.eq(&cause, param_env, expected_sig, sig) {
let mut result = infcx.probe(|_| ocx.eq(&cause, param_env, expected_sig, sig));
if result.is_err()
&& let ProcMacroKind::Derive = kind
{
if infcx.probe(|_| ocx.eq(&cause, param_env, expected_options_sig, sig)).is_ok() {
result = Ok(());
}
}

if let Err(terr) = result {
let mut diag = tcx.dcx().create_err(errors::ProcMacroBadSig { span, kind });

let hir_sig = tcx.hir().fn_sig_by_hir_id(hir_id);
Expand Down Expand Up @@ -2368,18 +2391,33 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

infcx.err_ctxt().note_type_err(
&mut diag,
&cause,
None,
Some(ValuePairs::PolySigs(ExpectedFound {
expected: ty::Binder::dummy(expected_sig),
found: ty::Binder::dummy(sig),
})),
terr,
false,
false,
);
let mut note_expected_found = |expected_sig| {
infcx.err_ctxt().note_type_err(
&mut diag,
&cause,
None,
Some(ValuePairs::PolySigs(ExpectedFound {
expected: ty::Binder::dummy(expected_sig),
found: ty::Binder::dummy(sig),
})),
terr,
false,
false,
)
};

note_expected_found(expected_sig);

if let ProcMacroKind::Derive = kind
&& tcx
.features()
.declared_lib_features
.iter()
.any(|&(feature, _)| feature == sym::derive_const)
Comment on lines +2412 to +2416
Copy link
Member Author

Choose a reason for hiding this comment

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

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)

{
note_expected_found(expected_options_sig);
}

diag.emit();
self.abort.set(true);
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ symbols! {
DecorateLint,
Default,
Deref,
DeriveExpansionOptions,
DiagnosticMessage,
DirBuilder,
Display,
Expand Down
33 changes: 30 additions & 3 deletions library/proc_macro/src/bridge/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,12 +484,16 @@ impl ProcMacro {
}
}

pub const fn custom_derive(
pub const fn custom_derive<Discriminant>(
Copy link
Member Author

@fmease fmease Jan 6, 2024

Choose a reason for hiding this comment

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

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).

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

@fmease fmease Jan 6, 2024

Choose a reason for hiding this comment

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

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.

) -> Self {
ProcMacro::CustomDerive { trait_name, attributes, client: Client::expand1(expand) }
ProcMacro::CustomDerive {
trait_name,
attributes,
client: Client::expand1(expand.into_fn()),
}
}

pub const fn attr(
Expand All @@ -506,3 +510,26 @@ impl ProcMacro {
ProcMacro::Bang { name, client: Client::expand1(expand) }
}
}

#[const_trait]
pub trait ExpandCustomDerive<Discriminant> {
Copy link
Member Author

@fmease fmease Jan 6, 2024

Choose a reason for hiding this comment

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

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)]).

fn into_fn(self) -> impl Fn(crate::TokenStream) -> crate::TokenStream + Copy;
}

impl<F> const ExpandCustomDerive<()> for F
where
F: Fn(crate::TokenStream) -> crate::TokenStream + Copy,
{
fn into_fn(self) -> impl Fn(crate::TokenStream) -> crate::TokenStream + Copy {
self
}
}

impl<F> const ExpandCustomDerive<crate::DeriveExpansionOptions> for F
where
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())
Copy link
Member Author

@fmease fmease Jan 6, 2024

Choose a reason for hiding this comment

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

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 ^^'

}
}
1 change: 1 addition & 0 deletions library/proc_macro/src/bridge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ macro_rules! with_api {
fn track_path(path: &str);
fn literal_from_str(s: &str) -> Result<Literal<$S::Span, $S::Symbol>, ()>;
fn emit_diagnostic(diagnostic: Diagnostic<$S::Span>);
fn is_derive_const() -> bool;
},
TokenStream {
fn drop($self: $S::TokenStream);
Expand Down
28 changes: 28 additions & 0 deletions library/proc_macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
#![feature(rustc_allow_const_fn_unstable)]
#![feature(staged_api)]
#![feature(allow_internal_unstable)]
#![feature(const_trait_impl)]
#![feature(decl_macro)]
#![feature(effects)]
#![feature(maybe_uninit_write_slice)]
#![feature(negative_impls)]
#![feature(new_uninit)]
Expand Down Expand Up @@ -85,6 +87,32 @@ impl !Send for TokenStream {}
#[stable(feature = "proc_macro_lib", since = "1.15.0")]
impl !Sync for TokenStream {}

/// Derive expansion options.
#[rustc_diagnostic_item = "DeriveExpansionOptions"]
#[unstable(feature = "derive_const", issue = "none")]
#[derive(Default, Clone)]
#[non_exhaustive]
pub struct DeriveExpansionOptions;
Copy link
Member Author

Choose a reason for hiding this comment

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

bikeshed name


impl DeriveExpansionOptions {
/// Returns the default options.
#[unstable(feature = "derive_const", issue = "none")]
pub fn new() -> Self {
Self::default()
}

/// Whether this is a `#[derive_const]` or a `#[derive]`.
#[unstable(feature = "derive_const", issue = "none")]
pub fn is_const(&self) -> bool {
bridge::client::FreeFunctions::is_derive_const()
}
}

#[unstable(feature = "derive_const", issue = "none")]
impl !Send for DeriveExpansionOptions {}
#[unstable(feature = "derive_const", issue = "none")]
impl !Sync for DeriveExpansionOptions {}
Comment on lines +111 to +114
Copy link
Member Author

Choose a reason for hiding this comment

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

for forward compatibility


/// Error returned from `TokenStream::from_str`.
#[stable(feature = "proc_macro_lib", since = "1.15.0")]
#[non_exhaustive]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ impl server::FreeFunctions for RaSpanServer {
fn emit_diagnostic(&mut self, _: bridge::Diagnostic<Self::Span>) {
// FIXME handle diagnostic
}

fn is_derive_const(&mut self) -> bool {
// FIXME: pass the correct information
false
}
}

impl server::TokenStream for RaSpanServer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ impl server::FreeFunctions for TokenIdServer {
}

fn emit_diagnostic(&mut self, _: bridge::Diagnostic<Self::Span>) {}

fn is_derive_const(&mut self) -> bool {
// FIXME: pass the correct information
false
}
}

impl server::TokenStream for TokenIdServer {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// force-host
// no-prefer-dynamic
#![crate_type = "proc-macro"]
#![feature(derive_const)]

extern crate proc_macro;

use proc_macro::{TokenStream, DeriveExpansionOptions};

#[proc_macro_derive(IsDeriveConst)]
pub fn is_derive_const(_: TokenStream, options: DeriveExpansionOptions) -> TokenStream {
format!("const IS_DERIVE_CONST: bool = {};", options.is_const()).parse().unwrap()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// check-pass
// edition: 2021
// aux-crate:is_derive_const=is-derive-const.rs
#![feature(derive_const)]

const _: () = {
#[derive(is_derive_const::IsDeriveConst)]
struct _Type;

assert!(!IS_DERIVE_CONST);
};

const _: () = {
#[derive_const(is_derive_const::IsDeriveConst)]
struct _Type;

assert!(IS_DERIVE_CONST);
};

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Check that we suggest *both* possible signatures of derive proc macros, namely
// fn(TokenStream) -> TokenStream
// and
// fn(TokenStream, DeriveExpansionOptions) -> TokenStream
// provided libs feature `derive_const` is enabled.

// force-host
// no-prefer-dynamic

#![crate_type = "proc-macro"]
#![feature(derive_const)]

extern crate proc_macro;

#[proc_macro_derive(Blah)]
pub fn bad_input() -> proc_macro::TokenStream {
//~^ ERROR derive proc macro has incorrect signature
Default::default()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: derive proc macro has incorrect signature
--> $DIR/signature-proc-macro-derive.rs:16:1
|
LL | pub fn bad_input() -> proc_macro::TokenStream {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| incorrect number of function parameters
| incorrect number of function parameters
Comment on lines +7 to +8
Copy link
Member Author

@fmease fmease Dec 4, 2023

Choose a reason for hiding this comment

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

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

|
= note: expected signature `fn(proc_macro::TokenStream) -> proc_macro::TokenStream`
found signature `fn() -> proc_macro::TokenStream`
= note: expected signature `fn(proc_macro::TokenStream, DeriveExpansionOptions) -> proc_macro::TokenStream`
found signature `fn() -> proc_macro::TokenStream`

error: aborting due to 1 previous error

Loading