From eccec746c4a948423675a2d76a98fc015b4d4557 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 11 Aug 2024 17:32:03 +0200 Subject: [PATCH 01/28] deps(rustc_expand): Add `rustc_middle` as a dep --- Cargo.lock | 1 + compiler/rustc_expand/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 7ac73a1c981d5..ea090b0d72ba3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3654,6 +3654,7 @@ dependencies = [ "rustc_lexer", "rustc_lint_defs", "rustc_macros", + "rustc_middle", "rustc_parse", "rustc_serialize", "rustc_session", diff --git a/compiler/rustc_expand/Cargo.toml b/compiler/rustc_expand/Cargo.toml index 0ba139ea5cc57..c603769cddab6 100644 --- a/compiler/rustc_expand/Cargo.toml +++ b/compiler/rustc_expand/Cargo.toml @@ -21,6 +21,7 @@ rustc_hir = { path = "../rustc_hir" } rustc_lexer = { path = "../rustc_lexer" } rustc_lint_defs = { path = "../rustc_lint_defs" } rustc_macros = { path = "../rustc_macros" } +rustc_middle = { path = "../rustc_middle" } rustc_parse = { path = "../rustc_parse" } rustc_serialize = { path = "../rustc_serialize" } rustc_session = { path = "../rustc_session" } From e0fe67eb05632ba4caaf0fb43370345e57986b9d Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 11 Aug 2024 17:32:29 +0200 Subject: [PATCH 02/28] refactor(rustc_expand): Take &SyntaxExtension instead of only &*Kind --- compiler/rustc_expand/src/expand.rs | 35 +++++++++++++++-------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 22da1179feb98..902cf49e69934 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -512,7 +512,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { self.cx.force_mode = force; let fragment_kind = invoc.fragment_kind; - match self.expand_invoc(invoc, &ext.kind) { + match self.expand_invoc(invoc, &ext) { ExpandResult::Ready(fragment) => { let mut derive_invocations = Vec::new(); let derive_placeholders = self @@ -674,7 +674,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { fn expand_invoc( &mut self, invoc: Invocation, - ext: &SyntaxExtensionKind, + ext: &Lrc, ) -> ExpandResult { let recursion_limit = match self.cx.reduced_recursion_limit { Some((limit, _)) => limit, @@ -695,7 +695,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { let (fragment_kind, span) = (invoc.fragment_kind, invoc.span()); ExpandResult::Ready(match invoc.kind { - InvocationKind::Bang { mac, span } => match ext { + InvocationKind::Bang { mac, span } => match &ext.kind { SyntaxExtensionKind::Bang(expander) => { match expander.expand(self.cx, span, mac.args.tokens.clone()) { Ok(tok_result) => { @@ -725,7 +725,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } _ => unreachable!(), }, - InvocationKind::Attr { attr, pos, mut item, derives } => match ext { + InvocationKind::Attr { attr, pos, mut item, derives } => match &ext.kind { SyntaxExtensionKind::Attr(expander) => { self.gate_proc_macro_input(&item); self.gate_proc_macro_attr_item(span, &item); @@ -804,10 +804,10 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } _ => unreachable!(), }, - InvocationKind::Derive { path, item, is_const } => match ext { + InvocationKind::Derive { path, item, is_const } => match &ext.kind { SyntaxExtensionKind::Derive(expander) | SyntaxExtensionKind::LegacyDerive(expander) => { - if let SyntaxExtensionKind::Derive(..) = ext { + if let SyntaxExtensionKind::Derive(..) = ext.kind { self.gate_proc_macro_input(&item); } // The `MetaItem` representing the trait to derive can't @@ -834,18 +834,19 @@ impl<'a, 'b> MacroExpander<'a, 'b> { }, InvocationKind::GlobDelegation { item, of_trait } => { let AssocItemKind::DelegationMac(deleg) = &item.kind else { unreachable!() }; - let suffixes = match ext { - SyntaxExtensionKind::GlobDelegation(expander) => match expander.expand(self.cx) - { - ExpandResult::Ready(suffixes) => suffixes, - ExpandResult::Retry(()) => { - // Reassemble the original invocation for retrying. - return ExpandResult::Retry(Invocation { - kind: InvocationKind::GlobDelegation { item, of_trait }, - ..invoc - }); + let suffixes = match &ext.kind { + SyntaxExtensionKind::GlobDelegation(expander) => { + match expander.expand(self.cx) { + ExpandResult::Ready(suffixes) => suffixes, + ExpandResult::Retry(()) => { + // Reassemble the original invocation for retrying. + return ExpandResult::Retry(Invocation { + kind: InvocationKind::GlobDelegation { item, of_trait }, + ..invoc + }); + } } - }, + } SyntaxExtensionKind::LegacyBang(..) => { let msg = "expanded a dummy glob delegation"; let guar = self.cx.dcx().span_delayed_bug(span, msg); From 70810909517998d912fd8f73e67dcc5647607dc5 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 03:24:07 +0200 Subject: [PATCH 03/28] wip: So yeah, tests pass, but still `eval_always` (not far from disk caching though I think) --- compiler/rustc_ast/src/tokenstream.rs | 100 +++++++++++++++- compiler/rustc_expand/src/base.rs | 3 + .../src/derive_macro_expansion.rs | 113 ++++++++++++++++++ compiler/rustc_expand/src/expand.rs | 5 + compiler/rustc_expand/src/lib.rs | 5 + compiler/rustc_expand/src/proc_macro.rs | 48 ++++---- compiler/rustc_interface/src/passes.rs | 1 + compiler/rustc_middle/src/arena.rs | 1 + compiler/rustc_middle/src/query/erase.rs | 5 + compiler/rustc_middle/src/query/keys.rs | 16 ++- compiler/rustc_middle/src/query/mod.rs | 9 +- compiler/rustc_resolve/src/macros.rs | 8 ++ compiler/rustc_span/src/hygiene.rs | 6 + 13 files changed, 287 insertions(+), 33 deletions(-) create mode 100644 compiler/rustc_expand/src/derive_macro_expansion.rs diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index bdd244be6d1cc..cd6653e647d4a 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -14,13 +14,15 @@ //! ownership of the original. use std::borrow::Cow; +use std::hash::Hash; use std::sync::Arc; use std::{cmp, fmt, iter}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::sync; use rustc_macros::{Decodable, Encodable, HashStable_Generic}; -use rustc_serialize::{Decodable, Encodable}; +use rustc_serialize::{Decodable, Encodable, Encoder}; +use rustc_span::def_id::{CrateNum, DefIndex}; use rustc_span::{DUMMY_SP, Span, SpanDecoder, SpanEncoder, Symbol, sym}; use crate::ast::AttrStyle; @@ -139,8 +141,10 @@ impl fmt::Debug for LazyAttrTokenStream { } impl Encodable for LazyAttrTokenStream { - fn encode(&self, _s: &mut S) { - panic!("Attempted to encode LazyAttrTokenStream"); + fn encode(&self, s: &mut S) { + // TODO: welp + // TODO: (also) `.flattened()` here? + self.to_attr_token_stream().encode(s) } } @@ -296,6 +300,96 @@ pub struct AttrsTarget { #[derive(Clone, Debug, Default, Encodable, Decodable)] pub struct TokenStream(pub(crate) Arc>); +struct HashEncoder { + hasher: H, +} + +impl Encoder for HashEncoder { + fn emit_usize(&mut self, v: usize) { + self.hasher.write_usize(v) + } + + fn emit_u128(&mut self, v: u128) { + self.hasher.write_u128(v) + } + + fn emit_u64(&mut self, v: u64) { + self.hasher.write_u64(v) + } + + fn emit_u32(&mut self, v: u32) { + self.hasher.write_u32(v) + } + + fn emit_u16(&mut self, v: u16) { + self.hasher.write_u16(v) + } + + fn emit_u8(&mut self, v: u8) { + self.hasher.write_u8(v) + } + + fn emit_isize(&mut self, v: isize) { + self.hasher.write_isize(v) + } + + fn emit_i128(&mut self, v: i128) { + self.hasher.write_i128(v) + } + + fn emit_i64(&mut self, v: i64) { + self.hasher.write_i64(v) + } + + fn emit_i32(&mut self, v: i32) { + self.hasher.write_i32(v) + } + + fn emit_i16(&mut self, v: i16) { + self.hasher.write_i16(v) + } + + fn emit_raw_bytes(&mut self, s: &[u8]) { + self.hasher.write(s) + } +} + +impl SpanEncoder for HashEncoder { + fn encode_span(&mut self, span: Span) { + span.hash(&mut self.hasher) + } + + fn encode_symbol(&mut self, symbol: Symbol) { + symbol.hash(&mut self.hasher) + } + + fn encode_expn_id(&mut self, expn_id: rustc_span::ExpnId) { + expn_id.hash(&mut self.hasher) + } + + fn encode_syntax_context(&mut self, syntax_context: rustc_span::SyntaxContext) { + syntax_context.hash(&mut self.hasher) + } + + fn encode_crate_num(&mut self, crate_num: CrateNum) { + crate_num.hash(&mut self.hasher) + } + + fn encode_def_index(&mut self, def_index: DefIndex) { + def_index.hash(&mut self.hasher) + } + + fn encode_def_id(&mut self, def_id: rustc_span::def_id::DefId) { + def_id.hash(&mut self.hasher) + } +} + +impl Hash for TokenStream { + fn hash(&self, state: &mut H) { + Encodable::encode(self, &mut HashEncoder { hasher: state }); + } +} + /// Indicates whether a token can join with the following token to form a /// compound token. Used for conversions to `proc_macro::Spacing`. Also used to /// guide pretty-printing, which is where the `JointHidden` value (which isn't diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 990d0f2e028aa..46dd49f29fc6c 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -1116,6 +1116,9 @@ pub trait ResolverExpand { trait_def_id: DefId, impl_def_id: LocalDefId, ) -> Result)>, Indeterminate>; + + fn register_proc_macro_invoc(&mut self, invoc_id: LocalExpnId, ext: Lrc); + fn unregister_proc_macro_invoc(&mut self, invoc_id: LocalExpnId); } pub trait LintStoreExpand { diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs new file mode 100644 index 0000000000000..0404c254e80e8 --- /dev/null +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -0,0 +1,113 @@ +// TODO: remove +#![allow(dead_code)] + +use std::cell::Cell; +use std::ptr; + +use rustc_ast::tokenstream::TokenStream; +use rustc_middle::ty::TyCtxt; +use rustc_span::LocalExpnId; +use rustc_span::profiling::SpannedEventArgRecorder; + +use crate::base::ExtCtxt; +use crate::errors; + +pub(super) fn expand<'tcx>( + tcx: TyCtxt<'tcx>, + key: (LocalExpnId, &'tcx TokenStream), +) -> Result<&'tcx TokenStream, ()> { + let (invoc_id, input) = key; + + let res = with_context(|(ecx, client)| { + let span = invoc_id.expn_data().call_site; + let _timer = + ecx.sess.prof.generic_activity_with_arg_recorder("expand_proc_macro", |recorder| { + recorder.record_arg_with_span(ecx.sess.source_map(), ecx.expansion_descr(), span); + }); + let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; + let strategy = crate::proc_macro::exec_strategy(ecx); + let server = crate::proc_macro_server::Rustc::new(ecx); + let res = match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { + // TODO: without flattened some (weird) tests fail, but no idea if it's correct/enough + Ok(stream) => Ok(tcx.arena.alloc(stream.flattened()) as &TokenStream), + Err(e) => { + ecx.dcx().emit_err({ + errors::ProcMacroDerivePanicked { + span, + message: e.as_str().map(|message| errors::ProcMacroDerivePanickedHelp { + message: message.into(), + }), + } + }); + Err(()) + } + }; + res + }); + + res +} + +type CLIENT = pm::bridge::client::Client; + +// based on rust/compiler/rustc_middle/src/ty/context/tls.rs +// #[cfg(not(parallel_compiler))] +thread_local! { + /// A thread local variable that stores a pointer to the current `CONTEXT`. + static TLV: Cell<(*mut (), Option)> = const { Cell::new((ptr::null_mut(), None)) }; +} + +#[inline] +fn erase(context: &mut ExtCtxt<'_>) -> *mut () { + context as *mut _ as *mut () +} + +#[inline] +unsafe fn downcast<'a>(context: *mut ()) -> &'a mut ExtCtxt<'a> { + unsafe { &mut *(context as *mut ExtCtxt<'a>) } +} + +/// Sets `context` as the new current `CONTEXT` for the duration of the function `f`. +#[inline] +pub fn enter_context<'a, F, R>(context: (&mut ExtCtxt<'a>, CLIENT), f: F) -> R +where + F: FnOnce() -> R, +{ + let (ectx, client) = context; + let erased = (erase(ectx), Some(client)); + TLV.with(|tlv| { + let old = tlv.replace(erased); + let _reset = rustc_data_structures::defer(move || tlv.set(old)); + f() + }) +} + +/// Allows access to the current `CONTEXT` in a closure if one is available. +#[inline] +#[track_caller] +pub fn with_context_opt(f: F) -> R +where + F: for<'a, 'b> FnOnce(Option<&'b mut (&mut ExtCtxt<'a>, CLIENT)>) -> R, +{ + let (ectx, client_opt) = TLV.get(); + if ectx.is_null() { + f(None) + } else { + // We could get an `CONTEXT` pointer from another thread. + // Ensure that `CONTEXT` is `DynSync`. + // TODO: we should not be able to? + // sync::assert_dyn_sync::>(); + + unsafe { f(Some(&mut (downcast(ectx), client_opt.unwrap()))) } + } +} + +/// Allows access to the current `CONTEXT`. +/// Panics if there is no `CONTEXT` available. +#[inline] +pub fn with_context(f: F) -> R +where + F: for<'a, 'b> FnOnce(&'b mut (&mut ExtCtxt<'a>, CLIENT)) -> R, +{ + with_context_opt(|opt_context| f(opt_context.expect("no CONTEXT stored in tls"))) +} diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 902cf49e69934..77f1706ece742 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -818,9 +818,14 @@ impl<'a, 'b> MacroExpander<'a, 'b> { span, path, }; + self.cx + .resolver + .register_proc_macro_invoc(invoc.expansion_data.id, ext.clone()); + invoc.expansion_data.id.expn_data(); let items = match expander.expand(self.cx, span, &meta, item, is_const) { ExpandResult::Ready(items) => items, ExpandResult::Retry(item) => { + self.cx.resolver.unregister_proc_macro_invoc(invoc.expansion_data.id); // Reassemble the original invocation for retrying. return ExpandResult::Retry(Invocation { kind: InvocationKind::Derive { path: meta.path, item, is_const }, diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index 4222c9fe90616..6f1747529c8a0 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -28,10 +28,15 @@ mod proc_macro_server; pub use mbe::macro_rules::compile_declarative_macro; pub mod base; pub mod config; +pub(crate) mod derive_macro_expansion; pub mod expand; pub mod module; // FIXME(Nilstrieb) Translate proc_macro diagnostics #[allow(rustc::untranslatable_diagnostic)] pub mod proc_macro; +pub fn provide(providers: &mut rustc_middle::util::Providers) { + providers.derive_macro_expansion = derive_macro_expansion::expand; +} + rustc_fluent_macro::fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index d5af9849e759f..200ecd613d88f 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -2,6 +2,7 @@ use rustc_ast as ast; use rustc_ast::ptr::P; use rustc_ast::tokenstream::TokenStream; use rustc_errors::ErrorGuaranteed; +use rustc_middle::ty; use rustc_parse::parser::{ForceCollect, Parser}; use rustc_session::config::ProcMacroExecutionStrategy; use rustc_span::Span; @@ -31,7 +32,7 @@ impl pm::bridge::server::MessagePipe for MessagePipe { } } -fn exec_strategy(ecx: &ExtCtxt<'_>) -> impl pm::bridge::server::ExecutionStrategy + 'static { +pub fn exec_strategy(ecx: &ExtCtxt<'_>) -> impl pm::bridge::server::ExecutionStrategy + 'static { pm::bridge::server::MaybeCrossThread::>::new( ecx.sess.opts.unstable_opts.proc_macro_execution_strategy == ProcMacroExecutionStrategy::CrossThread, @@ -124,36 +125,27 @@ impl MultiItemModifier for DeriveProcMacro { // altogether. See #73345. crate::base::ann_pretty_printing_compatibility_hack(&item, &ecx.sess.psess); let input = item.to_tokens(); - let stream = { - let _timer = - ecx.sess.prof.generic_activity_with_arg_recorder("expand_proc_macro", |recorder| { - recorder.record_arg_with_span( - ecx.sess.source_map(), - ecx.expansion_descr(), - span, - ); - }); - let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; - let strategy = exec_strategy(ecx); - let server = proc_macro_server::Rustc::new(ecx); - match self.client.run(&strategy, server, input, proc_macro_backtrace) { - Ok(stream) => stream, - Err(e) => { - ecx.dcx().emit_err({ - errors::ProcMacroDerivePanicked { - span, - message: e.as_str().map(|message| { - errors::ProcMacroDerivePanickedHelp { message: message.into() } - }), - } - }); - return ExpandResult::Ready(vec![]); - } - } + let res = ty::tls::with(|tcx| { + // TODO: without flattened some (weird) tests fail, but no idea if it's correct/enough + let input = tcx.arena.alloc(input.flattened()) as &TokenStream; + let invoc_id = ecx.current_expansion.id; + + assert_eq!(invoc_id.expn_data().call_site, span); + + let res = crate::derive_macro_expansion::enter_context((ecx, self.client), move || { + let res = tcx.derive_macro_expansion((invoc_id, input)).cloned(); + res + }); + + res + }); + let Ok(output) = res else { + // error will already have been emitted + return ExpandResult::Ready(vec![]); }; let error_count_before = ecx.dcx().err_count(); - let mut parser = Parser::new(&ecx.sess.psess, stream, Some("proc-macro derive")); + let mut parser = Parser::new(&ecx.sess.psess, output, Some("proc-macro derive")); let mut items = vec![]; loop { diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 2440f0639c8a6..12a952e258273 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -753,6 +753,7 @@ pub static DEFAULT_QUERY_PROVIDERS: LazyLock = LazyLock::new(|| { providers.env_var_os = env_var_os; limits::provide(providers); proc_macro_decls::provide(providers); + rustc_expand::provide(providers); rustc_const_eval::provide(providers); rustc_middle::hir::provide(providers); rustc_borrowck::provide(providers); diff --git a/compiler/rustc_middle/src/arena.rs b/compiler/rustc_middle/src/arena.rs index aef56ea46e957..c9f72d0f28c6f 100644 --- a/compiler/rustc_middle/src/arena.rs +++ b/compiler/rustc_middle/src/arena.rs @@ -116,6 +116,7 @@ macro_rules! arena_types { [decode] specialization_graph: rustc_middle::traits::specialization_graph::Graph, [] crate_inherent_impls: rustc_middle::ty::CrateInherentImpls, [] hir_owner_nodes: rustc_hir::OwnerNodes<'tcx>, + [] token_stream: rustc_ast::tokenstream::TokenStream, ]); ) } diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index 6c6b9a5510c69..28e103be49d61 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -2,6 +2,7 @@ use std::ffi::OsStr; use std::intrinsics::transmute_unchecked; use std::mem::MaybeUninit; +use rustc_ast::tokenstream::TokenStream; use rustc_span::ErrorGuaranteed; use crate::query::CyclePlaceholder; @@ -171,6 +172,10 @@ impl EraseType for Result>, CyclePlaceholder> { type Result = [u8; size_of::>, CyclePlaceholder>>()]; } +impl EraseType for Result<&'_ TokenStream, ()> { + type Result = [u8; size_of::>()]; +} + impl EraseType for Option<&'_ T> { type Result = [u8; size_of::>()]; } diff --git a/compiler/rustc_middle/src/query/keys.rs b/compiler/rustc_middle/src/query/keys.rs index c382bcd726ffa..71eaeaffa5bc9 100644 --- a/compiler/rustc_middle/src/query/keys.rs +++ b/compiler/rustc_middle/src/query/keys.rs @@ -2,11 +2,12 @@ use std::ffi::OsStr; +use rustc_ast::tokenstream::TokenStream; use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId, LocalModDefId, ModDefId}; use rustc_hir::hir_id::{HirId, OwnerId}; use rustc_query_system::dep_graph::DepNodeIndex; use rustc_query_system::query::{DefIdCache, DefaultCache, SingleCache, VecCache}; -use rustc_span::{DUMMY_SP, Ident, Span, Symbol}; +use rustc_span::{DUMMY_SP, Ident, LocalExpnId, Span, Symbol}; use crate::infer::canonical::CanonicalQueryInput; use crate::mir::mono::CollectionMode; @@ -608,6 +609,19 @@ impl Key for (LocalDefId, HirId) { } } +impl<'tcx> Key for (LocalExpnId, &'tcx TokenStream) { + type Cache = DefaultCache; + + fn default_span(&self, _tcx: TyCtxt<'_>) -> Span { + self.0.expn_data().call_site + } + + #[inline(always)] + fn key_as_def_id(&self) -> Option { + None + } +} + impl<'tcx> Key for (ValidityRequirement, ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>) { type Cache = DefaultCache; diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index d7ed703f4ae30..558c02959664f 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -14,6 +14,7 @@ use std::sync::Arc; use rustc_arena::TypedArena; use rustc_ast::expand::StrippedCfgItem; use rustc_ast::expand::allocator::AllocatorKind; +use rustc_ast::tokenstream::TokenStream; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; use rustc_data_structures::sorted_map::SortedMap; @@ -42,7 +43,7 @@ use rustc_session::cstore::{ use rustc_session::lint::LintExpectationId; use rustc_span::def_id::LOCAL_CRATE; use rustc_span::source_map::Spanned; -use rustc_span::{DUMMY_SP, Span, Symbol}; +use rustc_span::{DUMMY_SP, LocalExpnId, Span, Symbol}; use rustc_target::spec::PanicStrategy; use {rustc_abi as abi, rustc_ast as ast, rustc_attr_data_structures as attr, rustc_hir as hir}; @@ -107,6 +108,12 @@ pub use plumbing::{IntoQueryParam, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk}; // Queries marked with `fatal_cycle` do not need the latter implementation, // as they will raise an fatal error on query cycles instead. rustc_queries! { + query derive_macro_expansion(key: (LocalExpnId, &'tcx TokenStream)) -> Result<&'tcx TokenStream, ()> { + eval_always + no_hash + desc { "expanding a derive (proc) macro" } + } + /// This exists purely for testing the interactions between delayed bugs and incremental. query trigger_delayed_bug(key: DefId) { desc { "triggering a delayed bug for testing incremental" } diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index d0ffef798cf6c..915b21843baa0 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -508,6 +508,14 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { }); Ok(idents) } + + fn register_proc_macro_invoc(&mut self, _invoc_id: LocalExpnId, _ext: Lrc) { + // TODO: dunno if need this yet + } + + fn unregister_proc_macro_invoc(&mut self, _invoc_id: LocalExpnId) { + // TODO: dunno if need this yet + } } impl<'ra, 'tcx> Resolver<'ra, 'tcx> { diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs index 4390085cd049a..e99d41f338b94 100644 --- a/compiler/rustc_span/src/hygiene.rs +++ b/compiler/rustc_span/src/hygiene.rs @@ -1627,3 +1627,9 @@ impl HashStable for ExpnId { hash.hash_stable(ctx, hasher); } } + +impl HashStable for LocalExpnId { + fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) { + self.to_expn_id().hash_stable(hcx, hasher); + } +} From 377075a4df9c4293febf87b4c9c97c1b153faa7e Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 14:09:19 +0200 Subject: [PATCH 04/28] fix: Prevent double-enter with same `&mut ExtCtxt` (which would be UB) --- .../src/derive_macro_expansion.rs | 26 ++++++++++++++----- compiler/rustc_expand/src/lib.rs | 2 +- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index 0404c254e80e8..73a29c7d11052 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -12,7 +12,7 @@ use rustc_span::profiling::SpannedEventArgRecorder; use crate::base::ExtCtxt; use crate::errors; -pub(super) fn expand<'tcx>( +pub(super) fn provide_derive_macro_expansion<'tcx>( tcx: TyCtxt<'tcx>, key: (LocalExpnId, &'tcx TokenStream), ) -> Result<&'tcx TokenStream, ()> { @@ -51,7 +51,6 @@ pub(super) fn expand<'tcx>( type CLIENT = pm::bridge::client::Client; // based on rust/compiler/rustc_middle/src/ty/context/tls.rs -// #[cfg(not(parallel_compiler))] thread_local! { /// A thread local variable that stores a pointer to the current `CONTEXT`. static TLV: Cell<(*mut (), Option)> = const { Cell::new((ptr::null_mut(), None)) }; @@ -67,14 +66,11 @@ unsafe fn downcast<'a>(context: *mut ()) -> &'a mut ExtCtxt<'a> { unsafe { &mut *(context as *mut ExtCtxt<'a>) } } -/// Sets `context` as the new current `CONTEXT` for the duration of the function `f`. #[inline] -pub fn enter_context<'a, F, R>(context: (&mut ExtCtxt<'a>, CLIENT), f: F) -> R +fn enter_context_erased(erased: (*mut (), Option), f: F) -> R where F: FnOnce() -> R, { - let (ectx, client) = context; - let erased = (erase(ectx), Some(client)); TLV.with(|tlv| { let old = tlv.replace(erased); let _reset = rustc_data_structures::defer(move || tlv.set(old)); @@ -82,6 +78,17 @@ where }) } +/// Sets `context` as the new current `CONTEXT` for the duration of the function `f`. +#[inline] +pub fn enter_context<'a, F, R>(context: (&mut ExtCtxt<'a>, CLIENT), f: F) -> R +where + F: FnOnce() -> R, +{ + let (ectx, client) = context; + let erased = (erase(ectx), Some(client)); + enter_context_erased(erased, f) +} + /// Allows access to the current `CONTEXT` in a closure if one is available. #[inline] #[track_caller] @@ -98,7 +105,12 @@ where // TODO: we should not be able to? // sync::assert_dyn_sync::>(); - unsafe { f(Some(&mut (downcast(ectx), client_opt.unwrap()))) } + // prevent double entering, as that would allow creating two `&mut ExtCtxt`s + // TODO: probably use a RefCell instead (which checks this properly)? + enter_context_erased((ptr::null_mut(), None), || unsafe { + let ectx = downcast(ectx); + f(Some(&mut (ectx, client_opt.unwrap()))) + }) } } diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index 6f1747529c8a0..5be846a279422 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -36,7 +36,7 @@ pub mod module; pub mod proc_macro; pub fn provide(providers: &mut rustc_middle::util::Providers) { - providers.derive_macro_expansion = derive_macro_expansion::expand; + providers.derive_macro_expansion = derive_macro_expansion::provide_derive_macro_expansion; } rustc_fluent_macro::fluent_messages! { "../messages.ftl" } From fdc7d639d7837fa67fd60152727b718e80b6775f Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 14:11:41 +0200 Subject: [PATCH 05/28] chore: Remove old `allow` --- compiler/rustc_expand/src/derive_macro_expansion.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index 73a29c7d11052..ea0a3792662d5 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -1,6 +1,3 @@ -// TODO: remove -#![allow(dead_code)] - use std::cell::Cell; use std::ptr; From efb8f717ac21c1ad8997f7289a2d7ca6083c5305 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 14:48:43 +0200 Subject: [PATCH 06/28] chore: Remove unneeded code, adapt TODOs for pr-time --- compiler/rustc_ast/src/tokenstream.rs | 4 ++-- compiler/rustc_expand/src/base.rs | 3 --- compiler/rustc_expand/src/derive_macro_expansion.rs | 6 +++--- compiler/rustc_expand/src/expand.rs | 4 ---- compiler/rustc_expand/src/proc_macro.rs | 2 +- compiler/rustc_resolve/src/macros.rs | 8 -------- 6 files changed, 6 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index cd6653e647d4a..c22b92d98818c 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -142,8 +142,8 @@ impl fmt::Debug for LazyAttrTokenStream { impl Encodable for LazyAttrTokenStream { fn encode(&self, s: &mut S) { - // TODO: welp - // TODO: (also) `.flattened()` here? + // TODO(pr-time): welp, do we really want this impl? maybe newtype wrapper? + // TODO(pr-time): (also) `.flattened()` here? self.to_attr_token_stream().encode(s) } } diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 46dd49f29fc6c..990d0f2e028aa 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -1116,9 +1116,6 @@ pub trait ResolverExpand { trait_def_id: DefId, impl_def_id: LocalDefId, ) -> Result)>, Indeterminate>; - - fn register_proc_macro_invoc(&mut self, invoc_id: LocalExpnId, ext: Lrc); - fn unregister_proc_macro_invoc(&mut self, invoc_id: LocalExpnId); } pub trait LintStoreExpand { diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index ea0a3792662d5..20a82380bea9a 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -25,7 +25,7 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( let strategy = crate::proc_macro::exec_strategy(ecx); let server = crate::proc_macro_server::Rustc::new(ecx); let res = match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { - // TODO: without flattened some (weird) tests fail, but no idea if it's correct/enough + // TODO(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough Ok(stream) => Ok(tcx.arena.alloc(stream.flattened()) as &TokenStream), Err(e) => { ecx.dcx().emit_err({ @@ -99,11 +99,11 @@ where } else { // We could get an `CONTEXT` pointer from another thread. // Ensure that `CONTEXT` is `DynSync`. - // TODO: we should not be able to? + // TODO(pr-time): we should not be able to? // sync::assert_dyn_sync::>(); // prevent double entering, as that would allow creating two `&mut ExtCtxt`s - // TODO: probably use a RefCell instead (which checks this properly)? + // TODO(pr-time): probably use a RefCell instead (which checks this properly)? enter_context_erased((ptr::null_mut(), None), || unsafe { let ectx = downcast(ectx); f(Some(&mut (ectx, client_opt.unwrap()))) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 77f1706ece742..45c62dcfc5b9c 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -818,14 +818,10 @@ impl<'a, 'b> MacroExpander<'a, 'b> { span, path, }; - self.cx - .resolver - .register_proc_macro_invoc(invoc.expansion_data.id, ext.clone()); invoc.expansion_data.id.expn_data(); let items = match expander.expand(self.cx, span, &meta, item, is_const) { ExpandResult::Ready(items) => items, ExpandResult::Retry(item) => { - self.cx.resolver.unregister_proc_macro_invoc(invoc.expansion_data.id); // Reassemble the original invocation for retrying. return ExpandResult::Retry(Invocation { kind: InvocationKind::Derive { path: meta.path, item, is_const }, diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 200ecd613d88f..9656e8a77391b 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -126,7 +126,7 @@ impl MultiItemModifier for DeriveProcMacro { crate::base::ann_pretty_printing_compatibility_hack(&item, &ecx.sess.psess); let input = item.to_tokens(); let res = ty::tls::with(|tcx| { - // TODO: without flattened some (weird) tests fail, but no idea if it's correct/enough + // TODO(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough let input = tcx.arena.alloc(input.flattened()) as &TokenStream; let invoc_id = ecx.current_expansion.id; diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 915b21843baa0..d0ffef798cf6c 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -508,14 +508,6 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { }); Ok(idents) } - - fn register_proc_macro_invoc(&mut self, _invoc_id: LocalExpnId, _ext: Lrc) { - // TODO: dunno if need this yet - } - - fn unregister_proc_macro_invoc(&mut self, _invoc_id: LocalExpnId) { - // TODO: dunno if need this yet - } } impl<'ra, 'tcx> Resolver<'ra, 'tcx> { From 9175cccd5c0c4aa452608f360a2733b5e80cd7cc Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 16:27:11 +0200 Subject: [PATCH 07/28] wip(test): Run with `-- --nocapture --verbose` and `invoked` should not appear in 2nd output --- .../auxiliary/derive_nothing.rs | 24 ++++++++++ .../derive_macro_expansion/item_changed.rs | 48 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs create mode 100644 tests/incremental/derive_macro_expansion/item_changed.rs diff --git a/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs b/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs new file mode 100644 index 0000000000000..cb572220aa197 --- /dev/null +++ b/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs @@ -0,0 +1,24 @@ +//@ force-host +//@ no-prefer-dynamic + +#![crate_type = "proc-macro"] + +extern crate proc_macro; +use proc_macro::TokenStream; + +#[proc_macro_derive(Nothing)] +pub fn derive(input: TokenStream) -> TokenStream { + eprintln!("invoked"); + + r#" + pub mod nothing_mod { + // #[cfg(cfail1)] + pub fn nothing() { + eprintln!("nothing"); + } + + // #[cfg(cfail2)] + // fn nothingx() {} + } + "#.parse().unwrap() +} diff --git a/tests/incremental/derive_macro_expansion/item_changed.rs b/tests/incremental/derive_macro_expansion/item_changed.rs new file mode 100644 index 0000000000000..c89a3d1c24d33 --- /dev/null +++ b/tests/incremental/derive_macro_expansion/item_changed.rs @@ -0,0 +1,48 @@ +//@ aux-build:derive_nothing.rs +//@ revisions:cfail1 cfail2 +//@ compile-flags: -Z query-dep-graph +//@ check-pass (FIXME(62277): could be check-pass?) + +// TODO(pr-time): do these revisions make sense? only "check" required? + +#![feature(rustc_attrs)] +#![feature(stmt_expr_attributes)] +#![allow(dead_code)] +#![crate_type = "rlib"] + +#![rustc_partition_codegened(module="item_changed-foo", cfg="cfail1")] +// #![rustc_partition_reused(module="item_changed-foo", cfg="cfail2")] +#![rustc_partition_reused(module="item_changed-foo-nothing_mod", cfg="cfail2")] + + #[macro_use] + extern crate derive_nothing; + +pub mod foo { + // #[rustc_clean(cfg="cfail2")] + #[derive(Nothing)] + pub struct Foo; + + #[cfg(cfail2)] + pub fn second_fn() { + eprintln!("just forcing codegen"); + } + + pub fn use_foo(_f: Foo) { + // #[cfg(cfail1)] + nothing_mod::nothing(); + + // #[cfg(cfail2)] + // nothingx(); + + eprintln!("foo used"); + } +} + +// fn main() { +// Foo; +// +// nothing(); +// +// #[cfg(rpass2)] +// Bar; +// } From b92601aca1e28bf9a3e89a2c72409a8d1d1a6cde Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 18:20:49 +0200 Subject: [PATCH 08/28] wip: Activate, test (and fix) on_disk caching for proc-macro expansions! Also add manual test. --- compiler/rustc_ast/src/tokenstream.rs | 11 +++-- .../src/derive_macro_expansion.rs | 5 +- compiler/rustc_expand/src/proc_macro.rs | 10 +++- compiler/rustc_middle/src/arena.rs | 2 +- compiler/rustc_middle/src/query/keys.rs | 3 +- compiler/rustc_middle/src/query/mod.rs | 5 +- .../rustc_middle/src/query/on_disk_cache.rs | 7 +++ .../auxiliary/derive_nothing.rs | 8 +--- .../derive_macro_expansion/item_changed.rs | 48 ------------------- .../proc_macro_unchanged.rs | 37 ++++++++++++++ 10 files changed, 71 insertions(+), 65 deletions(-) delete mode 100644 tests/incremental/derive_macro_expansion/item_changed.rs create mode 100644 tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index c22b92d98818c..3b20216254acc 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -141,10 +141,13 @@ impl fmt::Debug for LazyAttrTokenStream { } impl Encodable for LazyAttrTokenStream { - fn encode(&self, s: &mut S) { - // TODO(pr-time): welp, do we really want this impl? maybe newtype wrapper? - // TODO(pr-time): (also) `.flattened()` here? - self.to_attr_token_stream().encode(s) + fn encode(&self, _s: &mut S) { + // TODO(pr-time): Just a reminder that this exists/was tried out, + // but probably not necessary anymore (see below). + // self.to_attr_token_stream().encode(s) + // We should not need to anymore, now that we `flatten`? + // Yep, that seems to be true! :) + panic!("Attempted to encode LazyAttrTokenStream"); } } diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index 20a82380bea9a..c86a2a10e43cf 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -2,6 +2,7 @@ use std::cell::Cell; use std::ptr; use rustc_ast::tokenstream::TokenStream; +use rustc_data_structures::svh::Svh; use rustc_middle::ty::TyCtxt; use rustc_span::LocalExpnId; use rustc_span::profiling::SpannedEventArgRecorder; @@ -11,9 +12,9 @@ use crate::errors; pub(super) fn provide_derive_macro_expansion<'tcx>( tcx: TyCtxt<'tcx>, - key: (LocalExpnId, &'tcx TokenStream), + key: (LocalExpnId, Svh, &'tcx TokenStream), ) -> Result<&'tcx TokenStream, ()> { - let (invoc_id, input) = key; + let (invoc_id, _macro_crate_hash, input) = key; let res = with_context(|(ecx, client)| { let span = invoc_id.expn_data().call_site; diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 9656e8a77391b..7a85b63359d8e 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -130,10 +130,18 @@ impl MultiItemModifier for DeriveProcMacro { let input = tcx.arena.alloc(input.flattened()) as &TokenStream; let invoc_id = ecx.current_expansion.id; + // TODO(pr-time): Just using the crate hash to notice when the proc-macro code has + // changed. How to *correctly* depend on exactly the macro definition? + // I.e., depending on the crate hash is just a HACK (and leaves garbage in the + // incremental compilation dir). + let macro_def_id = invoc_id.expn_data().macro_def_id.unwrap(); + let proc_macro_crate_hash = tcx.crate_hash(macro_def_id.krate); + assert_eq!(invoc_id.expn_data().call_site, span); let res = crate::derive_macro_expansion::enter_context((ecx, self.client), move || { - let res = tcx.derive_macro_expansion((invoc_id, input)).cloned(); + let res = + tcx.derive_macro_expansion((invoc_id, proc_macro_crate_hash, input)).cloned(); res }); diff --git a/compiler/rustc_middle/src/arena.rs b/compiler/rustc_middle/src/arena.rs index c9f72d0f28c6f..fcad27c2d3842 100644 --- a/compiler/rustc_middle/src/arena.rs +++ b/compiler/rustc_middle/src/arena.rs @@ -116,7 +116,7 @@ macro_rules! arena_types { [decode] specialization_graph: rustc_middle::traits::specialization_graph::Graph, [] crate_inherent_impls: rustc_middle::ty::CrateInherentImpls, [] hir_owner_nodes: rustc_hir::OwnerNodes<'tcx>, - [] token_stream: rustc_ast::tokenstream::TokenStream, + [decode] token_stream: rustc_ast::tokenstream::TokenStream, ]); ) } diff --git a/compiler/rustc_middle/src/query/keys.rs b/compiler/rustc_middle/src/query/keys.rs index 71eaeaffa5bc9..fa530e3274792 100644 --- a/compiler/rustc_middle/src/query/keys.rs +++ b/compiler/rustc_middle/src/query/keys.rs @@ -3,6 +3,7 @@ use std::ffi::OsStr; use rustc_ast::tokenstream::TokenStream; +use rustc_data_structures::svh::Svh; use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId, LocalModDefId, ModDefId}; use rustc_hir::hir_id::{HirId, OwnerId}; use rustc_query_system::dep_graph::DepNodeIndex; @@ -609,7 +610,7 @@ impl Key for (LocalDefId, HirId) { } } -impl<'tcx> Key for (LocalExpnId, &'tcx TokenStream) { +impl<'tcx> Key for (LocalExpnId, Svh, &'tcx TokenStream) { type Cache = DefaultCache; fn default_span(&self, _tcx: TyCtxt<'_>) -> Span { diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 558c02959664f..b4b9e4aee6793 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -108,10 +108,11 @@ pub use plumbing::{IntoQueryParam, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk}; // Queries marked with `fatal_cycle` do not need the latter implementation, // as they will raise an fatal error on query cycles instead. rustc_queries! { - query derive_macro_expansion(key: (LocalExpnId, &'tcx TokenStream)) -> Result<&'tcx TokenStream, ()> { - eval_always + query derive_macro_expansion(key: (LocalExpnId, Svh, &'tcx TokenStream)) -> Result<&'tcx TokenStream, ()> { + // eval_always no_hash desc { "expanding a derive (proc) macro" } + cache_on_disk_if { true } } /// This exists purely for testing the interactions between delayed bugs and incremental. diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs index 14e3ce8bef6b2..2148f2a8bad80 100644 --- a/compiler/rustc_middle/src/query/on_disk_cache.rs +++ b/compiler/rustc_middle/src/query/on_disk_cache.rs @@ -773,6 +773,13 @@ impl<'a, 'tcx> Decodable> } } +impl<'a, 'tcx> Decodable> for &'tcx rustc_ast::tokenstream::TokenStream { + #[inline] + fn decode(d: &mut CacheDecoder<'a, 'tcx>) -> Self { + RefDecodable::decode(d) + } +} + macro_rules! impl_ref_decoder { (<$tcx:tt> $($ty:ty,)*) => { $(impl<'a, $tcx> Decodable> for &$tcx [$ty] { diff --git a/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs b/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs index cb572220aa197..a0fcadc463b8c 100644 --- a/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs +++ b/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs @@ -10,15 +10,11 @@ use proc_macro::TokenStream; pub fn derive(input: TokenStream) -> TokenStream { eprintln!("invoked"); - r#" + return r#" pub mod nothing_mod { - // #[cfg(cfail1)] pub fn nothing() { eprintln!("nothing"); } - - // #[cfg(cfail2)] - // fn nothingx() {} } - "#.parse().unwrap() + "#.parse().unwrap(); } diff --git a/tests/incremental/derive_macro_expansion/item_changed.rs b/tests/incremental/derive_macro_expansion/item_changed.rs deleted file mode 100644 index c89a3d1c24d33..0000000000000 --- a/tests/incremental/derive_macro_expansion/item_changed.rs +++ /dev/null @@ -1,48 +0,0 @@ -//@ aux-build:derive_nothing.rs -//@ revisions:cfail1 cfail2 -//@ compile-flags: -Z query-dep-graph -//@ check-pass (FIXME(62277): could be check-pass?) - -// TODO(pr-time): do these revisions make sense? only "check" required? - -#![feature(rustc_attrs)] -#![feature(stmt_expr_attributes)] -#![allow(dead_code)] -#![crate_type = "rlib"] - -#![rustc_partition_codegened(module="item_changed-foo", cfg="cfail1")] -// #![rustc_partition_reused(module="item_changed-foo", cfg="cfail2")] -#![rustc_partition_reused(module="item_changed-foo-nothing_mod", cfg="cfail2")] - - #[macro_use] - extern crate derive_nothing; - -pub mod foo { - // #[rustc_clean(cfg="cfail2")] - #[derive(Nothing)] - pub struct Foo; - - #[cfg(cfail2)] - pub fn second_fn() { - eprintln!("just forcing codegen"); - } - - pub fn use_foo(_f: Foo) { - // #[cfg(cfail1)] - nothing_mod::nothing(); - - // #[cfg(cfail2)] - // nothingx(); - - eprintln!("foo used"); - } -} - -// fn main() { -// Foo; -// -// nothing(); -// -// #[cfg(rpass2)] -// Bar; -// } diff --git a/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs b/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs new file mode 100644 index 0000000000000..151c9af29c2b5 --- /dev/null +++ b/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs @@ -0,0 +1,37 @@ +// This test tests that derive-macro execution is cached. +// HOWEVER, this test can currently only be checked manually, +// by running it (through compiletest) with `-- --nocapture --verbose`. +// The proc-macro (for `Nothing`) prints a message to stderr when invoked, +// and this message should only be present during the second invocation +// (which has `cfail2` set via cfg). +// TODO(pr-time): Properly have the test check this, but how? UI-test that tests for `.stderr`? + +//@ aux-build:derive_nothing.rs +//@ revisions:cfail1 cfail2 +//@ compile-flags: -Z query-dep-graph +//@ build-pass + +#![feature(rustc_attrs)] +#![feature(stmt_expr_attributes)] +#![allow(dead_code)] +#![crate_type = "rlib"] + +#![rustc_partition_codegened(module="proc_macro_unchanged-foo", cfg="cfail1")] +#![rustc_partition_codegened(module="proc_macro_unchanged-foo", cfg="cfail2")] + +// `foo::nothing_mod` is created by the derive macro and doesn't change +#![rustc_partition_reused(module="proc_macro_unchanged-foo", cfg="cfail2")] + + #[macro_use] + extern crate derive_nothing; + +pub mod foo { + #[derive(Nothing)] + pub struct Foo; + + pub fn use_foo(_f: Foo) { + nothing_mod::nothing(); + + eprintln!("foo used"); + } +} From f9ec979df6ffa2e67d510cb2f7fd0154a525f8a8 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 22:23:18 +0200 Subject: [PATCH 09/28] chore: Adjust timing outputs --- compiler/rustc_expand/src/derive_macro_expansion.rs | 8 +++++--- compiler/rustc_expand/src/proc_macro.rs | 7 +++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index c86a2a10e43cf..b9a8828bd3aea 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -18,10 +18,12 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( let res = with_context(|(ecx, client)| { let span = invoc_id.expn_data().call_site; - let _timer = - ecx.sess.prof.generic_activity_with_arg_recorder("expand_proc_macro", |recorder| { + let _timer = ecx.sess.prof.generic_activity_with_arg_recorder( + "expand_derive_proc_macro_inner", + |recorder| { recorder.record_arg_with_span(ecx.sess.source_map(), ecx.expansion_descr(), span); - }); + }, + ); let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; let strategy = crate::proc_macro::exec_strategy(ecx); let server = crate::proc_macro_server::Rustc::new(ecx); diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 7a85b63359d8e..35a482f1a0f33 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -115,6 +115,13 @@ impl MultiItemModifier for DeriveProcMacro { item: Annotatable, _is_derive_const: bool, ) -> ExpandResult, Annotatable> { + let _timer = ecx.sess.prof.generic_activity_with_arg_recorder( + "expand_derive_proc_macro_outer", + |recorder| { + recorder.record_arg_with_span(ecx.sess.source_map(), ecx.expansion_descr(), span); + }, + ); + // We need special handling for statement items // (e.g. `fn foo() { #[derive(Debug)] struct Bar; }`) let is_stmt = matches!(item, Annotatable::Stmt(..)); From f0e0006434eced8f2c30054a24ce702aa9c48e8c Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 22:23:39 +0200 Subject: [PATCH 10/28] feat: Add `-Zcache-all-derive-macros` to use/bypass cache --- compiler/rustc_expand/src/proc_macro.rs | 9 ++++++--- compiler/rustc_session/src/options.rs | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 35a482f1a0f33..b8f5326390d93 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -147,9 +147,12 @@ impl MultiItemModifier for DeriveProcMacro { assert_eq!(invoc_id.expn_data().call_site, span); let res = crate::derive_macro_expansion::enter_context((ecx, self.client), move || { - let res = - tcx.derive_macro_expansion((invoc_id, proc_macro_crate_hash, input)).cloned(); - res + let key = (invoc_id, proc_macro_crate_hash, input); + if tcx.sess.opts.unstable_opts.cache_all_derive_macros { + tcx.derive_macro_expansion(key).cloned() + } else { + crate::derive_macro_expansion::provide_derive_macro_expansion(tcx, key).cloned() + } }); res diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 4cc666b3e37d2..fab5c5dc2ee66 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -2104,6 +2104,8 @@ options! { "emit noalias metadata for box (default: yes)"), branch_protection: Option = (None, parse_branch_protection, [TRACKED], "set options for branch target identification and pointer authentication on AArch64"), + cache_all_derive_macros: bool = (false, parse_bool, [UNTRACKED], + "cache the results of ALL derive macro invocations (potentially unsound!) (default: no)"), cf_protection: CFProtection = (CFProtection::None, parse_cfprotection, [TRACKED], "instrument control-flow architecture protection"), check_cfg_all_expected: bool = (false, parse_bool, [UNTRACKED], From 74b93656aceeece048546d616989604c804dafe0 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 12 Aug 2024 23:43:41 +0200 Subject: [PATCH 11/28] wip: try also hashing query output --- compiler/rustc_middle/src/query/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index b4b9e4aee6793..02f107a4ad511 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -110,7 +110,7 @@ pub use plumbing::{IntoQueryParam, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk}; rustc_queries! { query derive_macro_expansion(key: (LocalExpnId, Svh, &'tcx TokenStream)) -> Result<&'tcx TokenStream, ()> { // eval_always - no_hash + // no_hash desc { "expanding a derive (proc) macro" } cache_on_disk_if { true } } From df518dbb378dc722d4534a95d1f5354f2c79e5ec Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Wed, 14 Aug 2024 18:49:21 +0200 Subject: [PATCH 12/28] prepare for PR (tidy should pass, caching=yes by default for rustc-perf) --- compiler/rustc_ast/src/tokenstream.rs | 2 +- compiler/rustc_expand/src/derive_macro_expansion.rs | 6 +++--- compiler/rustc_expand/src/proc_macro.rs | 4 ++-- compiler/rustc_session/src/options.rs | 4 ++-- .../derive_macro_expansion/proc_macro_unchanged.rs | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index 3b20216254acc..530796c67313f 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -142,7 +142,7 @@ impl fmt::Debug for LazyAttrTokenStream { impl Encodable for LazyAttrTokenStream { fn encode(&self, _s: &mut S) { - // TODO(pr-time): Just a reminder that this exists/was tried out, + // FIXME(pr-time): Just a reminder that this exists/was tried out, // but probably not necessary anymore (see below). // self.to_attr_token_stream().encode(s) // We should not need to anymore, now that we `flatten`? diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index b9a8828bd3aea..c0c820d6803a4 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -28,7 +28,7 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( let strategy = crate::proc_macro::exec_strategy(ecx); let server = crate::proc_macro_server::Rustc::new(ecx); let res = match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { - // TODO(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough + // FIXME(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough Ok(stream) => Ok(tcx.arena.alloc(stream.flattened()) as &TokenStream), Err(e) => { ecx.dcx().emit_err({ @@ -102,11 +102,11 @@ where } else { // We could get an `CONTEXT` pointer from another thread. // Ensure that `CONTEXT` is `DynSync`. - // TODO(pr-time): we should not be able to? + // FIXME(pr-time): we should not be able to? // sync::assert_dyn_sync::>(); // prevent double entering, as that would allow creating two `&mut ExtCtxt`s - // TODO(pr-time): probably use a RefCell instead (which checks this properly)? + // FIXME(pr-time): probably use a RefCell instead (which checks this properly)? enter_context_erased((ptr::null_mut(), None), || unsafe { let ectx = downcast(ectx); f(Some(&mut (ectx, client_opt.unwrap()))) diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index b8f5326390d93..fb71a79568a42 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -133,11 +133,11 @@ impl MultiItemModifier for DeriveProcMacro { crate::base::ann_pretty_printing_compatibility_hack(&item, &ecx.sess.psess); let input = item.to_tokens(); let res = ty::tls::with(|tcx| { - // TODO(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough + // FIXME(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough let input = tcx.arena.alloc(input.flattened()) as &TokenStream; let invoc_id = ecx.current_expansion.id; - // TODO(pr-time): Just using the crate hash to notice when the proc-macro code has + // FIXME(pr-time): Just using the crate hash to notice when the proc-macro code has // changed. How to *correctly* depend on exactly the macro definition? // I.e., depending on the crate hash is just a HACK (and leaves garbage in the // incremental compilation dir). diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index fab5c5dc2ee66..a61ab991749f6 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -2104,8 +2104,8 @@ options! { "emit noalias metadata for box (default: yes)"), branch_protection: Option = (None, parse_branch_protection, [TRACKED], "set options for branch target identification and pointer authentication on AArch64"), - cache_all_derive_macros: bool = (false, parse_bool, [UNTRACKED], - "cache the results of ALL derive macro invocations (potentially unsound!) (default: no)"), + cache_all_derive_macros: bool = (true, parse_bool, [UNTRACKED], + "cache the results of ALL derive macro invocations (potentially unsound!) (default: YES -- for rustc-perf)"), cf_protection: CFProtection = (CFProtection::None, parse_cfprotection, [TRACKED], "instrument control-flow architecture protection"), check_cfg_all_expected: bool = (false, parse_bool, [UNTRACKED], diff --git a/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs b/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs index 151c9af29c2b5..3b1505b171443 100644 --- a/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs +++ b/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs @@ -4,7 +4,7 @@ // The proc-macro (for `Nothing`) prints a message to stderr when invoked, // and this message should only be present during the second invocation // (which has `cfail2` set via cfg). -// TODO(pr-time): Properly have the test check this, but how? UI-test that tests for `.stderr`? +// FIXME(pr-time): Properly have the test check this, but how? UI-test that tests for `.stderr`? //@ aux-build:derive_nothing.rs //@ revisions:cfail1 cfail2 From 12c6d3e998fa0634aa1db60c3089c2c23e4f6b05 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 19 Aug 2024 14:53:04 +0200 Subject: [PATCH 13/28] wip: Undo unnecessary refactoring --- compiler/rustc_expand/src/expand.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 45c62dcfc5b9c..8ab18b920b193 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -512,7 +512,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { self.cx.force_mode = force; let fragment_kind = invoc.fragment_kind; - match self.expand_invoc(invoc, &ext) { + match self.expand_invoc(invoc, &ext.kind) { ExpandResult::Ready(fragment) => { let mut derive_invocations = Vec::new(); let derive_placeholders = self @@ -674,7 +674,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { fn expand_invoc( &mut self, invoc: Invocation, - ext: &Lrc, + ext: &SyntaxExtensionKind, ) -> ExpandResult { let recursion_limit = match self.cx.reduced_recursion_limit { Some((limit, _)) => limit, @@ -695,7 +695,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { let (fragment_kind, span) = (invoc.fragment_kind, invoc.span()); ExpandResult::Ready(match invoc.kind { - InvocationKind::Bang { mac, span } => match &ext.kind { + InvocationKind::Bang { mac, span } => match ext { SyntaxExtensionKind::Bang(expander) => { match expander.expand(self.cx, span, mac.args.tokens.clone()) { Ok(tok_result) => { @@ -725,7 +725,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } _ => unreachable!(), }, - InvocationKind::Attr { attr, pos, mut item, derives } => match &ext.kind { + InvocationKind::Attr { attr, pos, mut item, derives } => match ext { SyntaxExtensionKind::Attr(expander) => { self.gate_proc_macro_input(&item); self.gate_proc_macro_attr_item(span, &item); @@ -804,10 +804,10 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } _ => unreachable!(), }, - InvocationKind::Derive { path, item, is_const } => match &ext.kind { + InvocationKind::Derive { path, item, is_const } => match ext { SyntaxExtensionKind::Derive(expander) | SyntaxExtensionKind::LegacyDerive(expander) => { - if let SyntaxExtensionKind::Derive(..) = ext.kind { + if let SyntaxExtensionKind::Derive(..) = ext { self.gate_proc_macro_input(&item); } // The `MetaItem` representing the trait to derive can't @@ -818,7 +818,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> { span, path, }; - invoc.expansion_data.id.expn_data(); let items = match expander.expand(self.cx, span, &meta, item, is_const) { ExpandResult::Ready(items) => items, ExpandResult::Retry(item) => { From 5439803a9d22989cec2cc7d485578222286bcc58 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Mon, 9 Sep 2024 14:56:27 +0200 Subject: [PATCH 14/28] rebase + fix unreachable pub-items, inline some CONTEXT-things --- .../src/derive_macro_expansion.rs | 75 +++++++------------ compiler/rustc_expand/src/lib.rs | 2 +- 2 files changed, 27 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index c0c820d6803a4..e9c1d3b4ad9b4 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -1,5 +1,5 @@ use std::cell::Cell; -use std::ptr; +use std::ptr::{self, NonNull}; use rustc_ast::tokenstream::TokenStream; use rustc_data_structures::svh::Svh; @@ -56,21 +56,14 @@ thread_local! { static TLV: Cell<(*mut (), Option)> = const { Cell::new((ptr::null_mut(), None)) }; } +/// Sets `context` as the new current `CONTEXT` for the duration of the function `f`. #[inline] -fn erase(context: &mut ExtCtxt<'_>) -> *mut () { - context as *mut _ as *mut () -} - -#[inline] -unsafe fn downcast<'a>(context: *mut ()) -> &'a mut ExtCtxt<'a> { - unsafe { &mut *(context as *mut ExtCtxt<'a>) } -} - -#[inline] -fn enter_context_erased(erased: (*mut (), Option), f: F) -> R +pub(crate) fn enter_context<'a, F, R>(context: (&mut ExtCtxt<'a>, CLIENT), f: F) -> R where F: FnOnce() -> R, { + let (ectx, client) = context; + let erased = (ectx as *mut _ as *mut (), Some(client)); TLV.with(|tlv| { let old = tlv.replace(erased); let _reset = rustc_data_structures::defer(move || tlv.set(old)); @@ -78,48 +71,32 @@ where }) } -/// Sets `context` as the new current `CONTEXT` for the duration of the function `f`. -#[inline] -pub fn enter_context<'a, F, R>(context: (&mut ExtCtxt<'a>, CLIENT), f: F) -> R -where - F: FnOnce() -> R, -{ - let (ectx, client) = context; - let erased = (erase(ectx), Some(client)); - enter_context_erased(erased, f) -} - -/// Allows access to the current `CONTEXT` in a closure if one is available. +/// Allows access to the current `CONTEXT`. +/// Panics if there is no `CONTEXT` available. #[inline] #[track_caller] -pub fn with_context_opt(f: F) -> R +fn with_context(f: F) -> R where - F: for<'a, 'b> FnOnce(Option<&'b mut (&mut ExtCtxt<'a>, CLIENT)>) -> R, + F: for<'a, 'b> FnOnce(&'b mut (&mut ExtCtxt<'a>, CLIENT)) -> R, { let (ectx, client_opt) = TLV.get(); - if ectx.is_null() { - f(None) - } else { - // We could get an `CONTEXT` pointer from another thread. - // Ensure that `CONTEXT` is `DynSync`. - // FIXME(pr-time): we should not be able to? - // sync::assert_dyn_sync::>(); + let ectx = NonNull::new(ectx).expect("no CONTEXT stored in tls"); - // prevent double entering, as that would allow creating two `&mut ExtCtxt`s - // FIXME(pr-time): probably use a RefCell instead (which checks this properly)? - enter_context_erased((ptr::null_mut(), None), || unsafe { - let ectx = downcast(ectx); - f(Some(&mut (ectx, client_opt.unwrap()))) - }) - } -} + // We could get an `CONTEXT` pointer from another thread. + // Ensure that `CONTEXT` is `DynSync`. + // FIXME(pr-time): we should not be able to? + // sync::assert_dyn_sync::>(); -/// Allows access to the current `CONTEXT`. -/// Panics if there is no `CONTEXT` available. -#[inline] -pub fn with_context(f: F) -> R -where - F: for<'a, 'b> FnOnce(&'b mut (&mut ExtCtxt<'a>, CLIENT)) -> R, -{ - with_context_opt(|opt_context| f(opt_context.expect("no CONTEXT stored in tls"))) + // prevent double entering, as that would allow creating two `&mut ExtCtxt`s + // FIXME(pr-time): probably use a RefCell instead (which checks this properly)? + TLV.with(|tlv| { + let old = tlv.replace((ptr::null_mut(), None)); + let _reset = rustc_data_structures::defer(move || tlv.set(old)); + let ectx = { + let mut casted = ectx.cast::>(); + unsafe { casted.as_mut() } + }; + + f(&mut (ectx, client_opt.unwrap())) + }) } diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index 5be846a279422..b062bcfe01bef 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -28,7 +28,7 @@ mod proc_macro_server; pub use mbe::macro_rules::compile_declarative_macro; pub mod base; pub mod config; -pub(crate) mod derive_macro_expansion; +mod derive_macro_expansion; pub mod expand; pub mod module; // FIXME(Nilstrieb) Translate proc_macro diagnostics From 8610ec6b70f8e37a4484aa4807f98f722d235704 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 16 Nov 2024 16:06:48 +0100 Subject: [PATCH 15/28] fix: comment + unused warning --- .../auxiliary/derive_nothing.rs | 2 +- .../derive_macro_expansion/proc_macro_unchanged.rs | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs b/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs index a0fcadc463b8c..ee0fe7ea10023 100644 --- a/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs +++ b/tests/incremental/derive_macro_expansion/auxiliary/derive_nothing.rs @@ -7,7 +7,7 @@ extern crate proc_macro; use proc_macro::TokenStream; #[proc_macro_derive(Nothing)] -pub fn derive(input: TokenStream) -> TokenStream { +pub fn derive(_input: TokenStream) -> TokenStream { eprintln!("invoked"); return r#" diff --git a/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs b/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs index 3b1505b171443..1a37cdb2e6925 100644 --- a/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs +++ b/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs @@ -2,13 +2,13 @@ // HOWEVER, this test can currently only be checked manually, // by running it (through compiletest) with `-- --nocapture --verbose`. // The proc-macro (for `Nothing`) prints a message to stderr when invoked, -// and this message should only be present during the second invocation -// (which has `cfail2` set via cfg). +// and this message should only be present during the first invocation, +// because the cached result should be used for the second invocation. // FIXME(pr-time): Properly have the test check this, but how? UI-test that tests for `.stderr`? //@ aux-build:derive_nothing.rs //@ revisions:cfail1 cfail2 -//@ compile-flags: -Z query-dep-graph +//@ compile-flags: -Z query-dep-graph -Zcache-all-derive-macros=true //@ build-pass #![feature(rustc_attrs)] @@ -17,10 +17,12 @@ #![crate_type = "rlib"] #![rustc_partition_codegened(module="proc_macro_unchanged-foo", cfg="cfail1")] -#![rustc_partition_codegened(module="proc_macro_unchanged-foo", cfg="cfail2")] +// #![rustc_partition_codegened(module="proc_macro_unchanged-foo", cfg="cfail2")] // `foo::nothing_mod` is created by the derive macro and doesn't change -#![rustc_partition_reused(module="proc_macro_unchanged-foo", cfg="cfail2")] +// BUG: this yields the same result with `-Zcache-all-derive-macros=false` (i.e., uncached), +// not sure how to do this correctly. +#![rustc_partition_reused(module="proc_macro_unchanged-foo-nothing_mod", cfg="cfail2")] #[macro_use] extern crate derive_nothing; From 1bad277197df5f0296d18ecfb147d161dbc1515b Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 16 Nov 2024 16:46:18 +0100 Subject: [PATCH 16/28] style: move import to correct place --- compiler/rustc_expand/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index b062bcfe01bef..25904848abba8 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -18,6 +18,7 @@ extern crate proc_macro as pm; mod build; +mod derive_macro_expansion; mod errors; // FIXME(Nilstrieb) Translate macro_rules diagnostics #[allow(rustc::untranslatable_diagnostic)] @@ -28,7 +29,6 @@ mod proc_macro_server; pub use mbe::macro_rules::compile_declarative_macro; pub mod base; pub mod config; -mod derive_macro_expansion; pub mod expand; pub mod module; // FIXME(Nilstrieb) Translate proc_macro diagnostics From bc58b22dd88a4b787bb874298b2880e2e09b7a89 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 16 Nov 2024 16:48:21 +0100 Subject: [PATCH 17/28] chore: remove old comment --- compiler/rustc_ast/src/tokenstream.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index 530796c67313f..22683aba9dd37 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -142,11 +142,6 @@ impl fmt::Debug for LazyAttrTokenStream { impl Encodable for LazyAttrTokenStream { fn encode(&self, _s: &mut S) { - // FIXME(pr-time): Just a reminder that this exists/was tried out, - // but probably not necessary anymore (see below). - // self.to_attr_token_stream().encode(s) - // We should not need to anymore, now that we `flatten`? - // Yep, that seems to be true! :) panic!("Attempted to encode LazyAttrTokenStream"); } } From 3e5e84bfd0deee824613744c7ddd55e5387f1b9f Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 16 Nov 2024 18:16:50 +0100 Subject: [PATCH 18/28] don't flatten proc macro output, let's see how it goes --- compiler/rustc_expand/src/derive_macro_expansion.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index e9c1d3b4ad9b4..6678812302732 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -29,7 +29,8 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( let server = crate::proc_macro_server::Rustc::new(ecx); let res = match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { // FIXME(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough - Ok(stream) => Ok(tcx.arena.alloc(stream.flattened()) as &TokenStream), + // -> removed the flattened for now, gonna see what CI says. + Ok(stream) => Ok(tcx.arena.alloc(stream) as &TokenStream), Err(e) => { ecx.dcx().emit_err({ errors::ProcMacroDerivePanicked { From 46524736eeb501cb61d20ca8ef08cf4420cc757d Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 16 Nov 2024 18:20:50 +0100 Subject: [PATCH 19/28] only retrieve expn_data once --- compiler/rustc_expand/src/proc_macro.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index fb71a79568a42..504de1903093d 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -136,15 +136,16 @@ impl MultiItemModifier for DeriveProcMacro { // FIXME(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough let input = tcx.arena.alloc(input.flattened()) as &TokenStream; let invoc_id = ecx.current_expansion.id; + let invoc_expn_data = invoc_id.expn_data(); // FIXME(pr-time): Just using the crate hash to notice when the proc-macro code has // changed. How to *correctly* depend on exactly the macro definition? // I.e., depending on the crate hash is just a HACK (and leaves garbage in the // incremental compilation dir). - let macro_def_id = invoc_id.expn_data().macro_def_id.unwrap(); + let macro_def_id = invoc_expn_data.macro_def_id.unwrap(); let proc_macro_crate_hash = tcx.crate_hash(macro_def_id.krate); - assert_eq!(invoc_id.expn_data().call_site, span); + assert_eq!(invoc_expn_data.call_site, span); let res = crate::derive_macro_expansion::enter_context((ecx, self.client), move || { let key = (invoc_id, proc_macro_crate_hash, input); From 1cd9a4f78ca9937b334840df203956abf9703c84 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 8 Dec 2024 16:14:48 +0100 Subject: [PATCH 20/28] fix: Ensure incremental compilation is also enabled --- compiler/rustc_expand/src/proc_macro.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 504de1903093d..3cf33f2e201e7 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -149,7 +149,11 @@ impl MultiItemModifier for DeriveProcMacro { let res = crate::derive_macro_expansion::enter_context((ecx, self.client), move || { let key = (invoc_id, proc_macro_crate_hash, input); - if tcx.sess.opts.unstable_opts.cache_all_derive_macros { + // FIXME(pr-time): Is this the correct way to check for incremental compilation (as + // well)? + if tcx.sess.opts.incremental.is_some() + && tcx.sess.opts.unstable_opts.cache_all_derive_macros + { tcx.derive_macro_expansion(key).cloned() } else { crate::derive_macro_expansion::provide_derive_macro_expansion(tcx, key).cloned() From db56376d901d2e1d2019fca5d31bd414c1ee8d3a Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 28 Dec 2024 16:44:18 +0100 Subject: [PATCH 21/28] refactor!: Rename `cache-all-derive-macros` option to `cache-proc-macros` --- compiler/rustc_expand/src/proc_macro.rs | 2 +- compiler/rustc_session/src/options.rs | 4 ++-- .../derive_macro_expansion/proc_macro_unchanged.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 3cf33f2e201e7..6f15f64c090a8 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -152,7 +152,7 @@ impl MultiItemModifier for DeriveProcMacro { // FIXME(pr-time): Is this the correct way to check for incremental compilation (as // well)? if tcx.sess.opts.incremental.is_some() - && tcx.sess.opts.unstable_opts.cache_all_derive_macros + && tcx.sess.opts.unstable_opts.cache_proc_macros { tcx.derive_macro_expansion(key).cloned() } else { diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index a61ab991749f6..bd8514dd4d370 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -2104,8 +2104,8 @@ options! { "emit noalias metadata for box (default: yes)"), branch_protection: Option = (None, parse_branch_protection, [TRACKED], "set options for branch target identification and pointer authentication on AArch64"), - cache_all_derive_macros: bool = (true, parse_bool, [UNTRACKED], - "cache the results of ALL derive macro invocations (potentially unsound!) (default: YES -- for rustc-perf)"), + cache_proc_macros: bool = (true, parse_bool, [UNTRACKED], + "cache the results of ALL proc macro invocations (potentially unsound!) (default: YES -- for rustc-perf)"), cf_protection: CFProtection = (CFProtection::None, parse_cfprotection, [TRACKED], "instrument control-flow architecture protection"), check_cfg_all_expected: bool = (false, parse_bool, [UNTRACKED], diff --git a/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs b/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs index 1a37cdb2e6925..ad98c9f789f47 100644 --- a/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs +++ b/tests/incremental/derive_macro_expansion/proc_macro_unchanged.rs @@ -8,7 +8,7 @@ //@ aux-build:derive_nothing.rs //@ revisions:cfail1 cfail2 -//@ compile-flags: -Z query-dep-graph -Zcache-all-derive-macros=true +//@ compile-flags: -Z query-dep-graph -Zcache-proc-macros=true //@ build-pass #![feature(rustc_attrs)] @@ -20,7 +20,7 @@ // #![rustc_partition_codegened(module="proc_macro_unchanged-foo", cfg="cfail2")] // `foo::nothing_mod` is created by the derive macro and doesn't change -// BUG: this yields the same result with `-Zcache-all-derive-macros=false` (i.e., uncached), +// BUG: this yields the same result with `-Zcache-proc-macros=false` (i.e., uncached), // not sure how to do this correctly. #![rustc_partition_reused(module="proc_macro_unchanged-foo-nothing_mod", cfg="cfail2")] From 70460d4402d81a057a44e123fa6ff4f95a780d35 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 28 Dec 2024 16:46:22 +0100 Subject: [PATCH 22/28] nit: remove comment --- compiler/rustc_expand/src/derive_macro_expansion.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index 6678812302732..6e3712a108889 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -28,8 +28,6 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( let strategy = crate::proc_macro::exec_strategy(ecx); let server = crate::proc_macro_server::Rustc::new(ecx); let res = match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { - // FIXME(pr-time): without flattened some (weird) tests fail, but no idea if it's correct/enough - // -> removed the flattened for now, gonna see what CI says. Ok(stream) => Ok(tcx.arena.alloc(stream) as &TokenStream), Err(e) => { ecx.dcx().emit_err({ From 93526788f2b06dbe5f3f13ada7d2c18671bbb122 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 28 Dec 2024 17:05:57 +0100 Subject: [PATCH 23/28] refactor: Require `ecx` less --- compiler/rustc_expand/src/derive_macro_expansion.rs | 12 ++++++++---- compiler/rustc_expand/src/proc_macro.rs | 9 +++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index 6e3712a108889..7f6c84595cff1 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -18,19 +18,23 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( let res = with_context(|(ecx, client)| { let span = invoc_id.expn_data().call_site; - let _timer = ecx.sess.prof.generic_activity_with_arg_recorder( + let _timer = tcx.sess.prof.generic_activity_with_arg_recorder( "expand_derive_proc_macro_inner", |recorder| { - recorder.record_arg_with_span(ecx.sess.source_map(), ecx.expansion_descr(), span); + recorder.record_arg_with_span( + tcx.sess.source_map(), + invoc_id.expn_data().kind.descr(), + span, + ); }, ); let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; - let strategy = crate::proc_macro::exec_strategy(ecx); + let strategy = crate::proc_macro::exec_strategy(tcx.sess); let server = crate::proc_macro_server::Rustc::new(ecx); let res = match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { Ok(stream) => Ok(tcx.arena.alloc(stream) as &TokenStream), Err(e) => { - ecx.dcx().emit_err({ + tcx.dcx().emit_err({ errors::ProcMacroDerivePanicked { span, message: e.as_str().map(|message| errors::ProcMacroDerivePanickedHelp { diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 6f15f64c090a8..50be86b4ac954 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -4,6 +4,7 @@ use rustc_ast::tokenstream::TokenStream; use rustc_errors::ErrorGuaranteed; use rustc_middle::ty; use rustc_parse::parser::{ForceCollect, Parser}; +use rustc_session::Session; use rustc_session::config::ProcMacroExecutionStrategy; use rustc_span::Span; use rustc_span::profiling::SpannedEventArgRecorder; @@ -32,9 +33,9 @@ impl pm::bridge::server::MessagePipe for MessagePipe { } } -pub fn exec_strategy(ecx: &ExtCtxt<'_>) -> impl pm::bridge::server::ExecutionStrategy + 'static { +pub fn exec_strategy(sess: &Session) -> impl pm::bridge::server::ExecutionStrategy + 'static { pm::bridge::server::MaybeCrossThread::>::new( - ecx.sess.opts.unstable_opts.proc_macro_execution_strategy + sess.opts.unstable_opts.proc_macro_execution_strategy == ProcMacroExecutionStrategy::CrossThread, ) } @@ -56,7 +57,7 @@ impl base::BangProcMacro for BangProcMacro { }); let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; - let strategy = exec_strategy(ecx); + let strategy = exec_strategy(ecx.sess); let server = proc_macro_server::Rustc::new(ecx); self.client.run(&strategy, server, input, proc_macro_backtrace).map_err(|e| { ecx.dcx().emit_err(errors::ProcMacroPanicked { @@ -87,7 +88,7 @@ impl base::AttrProcMacro for AttrProcMacro { }); let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; - let strategy = exec_strategy(ecx); + let strategy = exec_strategy(ecx.sess); let server = proc_macro_server::Rustc::new(ecx); self.client.run(&strategy, server, annotation, annotated, proc_macro_backtrace).map_err( |e| { From 6a62f9b718f2b72880260dc30a4ed23802a6773c Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 28 Dec 2024 17:40:58 +0100 Subject: [PATCH 24/28] refactor: Only call invoc_id.expn_data() once, inline some `res` --- .../src/derive_macro_expansion.rs | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs index 7f6c84595cff1..6e0a8ad5d039c 100644 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ b/compiler/rustc_expand/src/derive_macro_expansion.rs @@ -16,22 +16,22 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( ) -> Result<&'tcx TokenStream, ()> { let (invoc_id, _macro_crate_hash, input) = key; - let res = with_context(|(ecx, client)| { - let span = invoc_id.expn_data().call_site; + with_context(|(ecx, client)| { + let invoc_expn_data = invoc_id.expn_data(); + let span = invoc_expn_data.call_site; + let event_arg = invoc_expn_data.kind.descr(); let _timer = tcx.sess.prof.generic_activity_with_arg_recorder( "expand_derive_proc_macro_inner", |recorder| { - recorder.record_arg_with_span( - tcx.sess.source_map(), - invoc_id.expn_data().kind.descr(), - span, - ); + recorder.record_arg_with_span(tcx.sess.source_map(), event_arg.clone(), span); }, ); + let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; let strategy = crate::proc_macro::exec_strategy(tcx.sess); let server = crate::proc_macro_server::Rustc::new(ecx); - let res = match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { + + match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { Ok(stream) => Ok(tcx.arena.alloc(stream) as &TokenStream), Err(e) => { tcx.dcx().emit_err({ @@ -44,11 +44,8 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( }); Err(()) } - }; - res - }); - - res + } + }) } type CLIENT = pm::bridge::client::Client; From 4a3ed82a619fee0e0ee196b6d79cf82d1ef928d4 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 28 Dec 2024 17:53:19 +0100 Subject: [PATCH 25/28] refactor: Don't enter context if not caching (don't need it then) --- compiler/rustc_expand/src/proc_macro.rs | 28 ++++++++++++------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 50be86b4ac954..4d8322559dc6f 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -141,27 +141,25 @@ impl MultiItemModifier for DeriveProcMacro { // FIXME(pr-time): Just using the crate hash to notice when the proc-macro code has // changed. How to *correctly* depend on exactly the macro definition? - // I.e., depending on the crate hash is just a HACK (and leaves garbage in the - // incremental compilation dir). + // I.e., depending on the crate hash is just a HACK, and ideally the dependency would be + // more narrow. let macro_def_id = invoc_expn_data.macro_def_id.unwrap(); let proc_macro_crate_hash = tcx.crate_hash(macro_def_id.krate); assert_eq!(invoc_expn_data.call_site, span); - let res = crate::derive_macro_expansion::enter_context((ecx, self.client), move || { - let key = (invoc_id, proc_macro_crate_hash, input); - // FIXME(pr-time): Is this the correct way to check for incremental compilation (as - // well)? - if tcx.sess.opts.incremental.is_some() - && tcx.sess.opts.unstable_opts.cache_proc_macros - { - tcx.derive_macro_expansion(key).cloned() - } else { - crate::derive_macro_expansion::provide_derive_macro_expansion(tcx, key).cloned() - } - }); + let key = (invoc_id, proc_macro_crate_hash, input); - res + // FIXME(pr-time): Is this the correct way to check for incremental compilation (as + // well)? + if tcx.sess.opts.incremental.is_some() && tcx.sess.opts.unstable_opts.cache_proc_macros + { + crate::derive_macro_expansion::enter_context((ecx, self.client), move || { + tcx.derive_macro_expansion(key).cloned() + }) + } else { + crate::derive_macro_expansion::provide_derive_macro_expansion(tcx, key).cloned() + } }); let Ok(output) = res else { // error will already have been emitted From 26cf691e03714f95d13784efbe4077c63ded76cd Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 28 Dec 2024 18:02:47 +0100 Subject: [PATCH 26/28] refactor: Move derive_macro_expansion.rs to end of proc_macro.rs --- .../src/derive_macro_expansion.rs | 102 ----------------- compiler/rustc_expand/src/lib.rs | 3 +- compiler/rustc_expand/src/proc_macro.rs | 105 +++++++++++++++++- 3 files changed, 100 insertions(+), 110 deletions(-) delete mode 100644 compiler/rustc_expand/src/derive_macro_expansion.rs diff --git a/compiler/rustc_expand/src/derive_macro_expansion.rs b/compiler/rustc_expand/src/derive_macro_expansion.rs deleted file mode 100644 index 6e0a8ad5d039c..0000000000000 --- a/compiler/rustc_expand/src/derive_macro_expansion.rs +++ /dev/null @@ -1,102 +0,0 @@ -use std::cell::Cell; -use std::ptr::{self, NonNull}; - -use rustc_ast::tokenstream::TokenStream; -use rustc_data_structures::svh::Svh; -use rustc_middle::ty::TyCtxt; -use rustc_span::LocalExpnId; -use rustc_span::profiling::SpannedEventArgRecorder; - -use crate::base::ExtCtxt; -use crate::errors; - -pub(super) fn provide_derive_macro_expansion<'tcx>( - tcx: TyCtxt<'tcx>, - key: (LocalExpnId, Svh, &'tcx TokenStream), -) -> Result<&'tcx TokenStream, ()> { - let (invoc_id, _macro_crate_hash, input) = key; - - with_context(|(ecx, client)| { - let invoc_expn_data = invoc_id.expn_data(); - let span = invoc_expn_data.call_site; - let event_arg = invoc_expn_data.kind.descr(); - let _timer = tcx.sess.prof.generic_activity_with_arg_recorder( - "expand_derive_proc_macro_inner", - |recorder| { - recorder.record_arg_with_span(tcx.sess.source_map(), event_arg.clone(), span); - }, - ); - - let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; - let strategy = crate::proc_macro::exec_strategy(tcx.sess); - let server = crate::proc_macro_server::Rustc::new(ecx); - - match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { - Ok(stream) => Ok(tcx.arena.alloc(stream) as &TokenStream), - Err(e) => { - tcx.dcx().emit_err({ - errors::ProcMacroDerivePanicked { - span, - message: e.as_str().map(|message| errors::ProcMacroDerivePanickedHelp { - message: message.into(), - }), - } - }); - Err(()) - } - } - }) -} - -type CLIENT = pm::bridge::client::Client; - -// based on rust/compiler/rustc_middle/src/ty/context/tls.rs -thread_local! { - /// A thread local variable that stores a pointer to the current `CONTEXT`. - static TLV: Cell<(*mut (), Option)> = const { Cell::new((ptr::null_mut(), None)) }; -} - -/// Sets `context` as the new current `CONTEXT` for the duration of the function `f`. -#[inline] -pub(crate) fn enter_context<'a, F, R>(context: (&mut ExtCtxt<'a>, CLIENT), f: F) -> R -where - F: FnOnce() -> R, -{ - let (ectx, client) = context; - let erased = (ectx as *mut _ as *mut (), Some(client)); - TLV.with(|tlv| { - let old = tlv.replace(erased); - let _reset = rustc_data_structures::defer(move || tlv.set(old)); - f() - }) -} - -/// Allows access to the current `CONTEXT`. -/// Panics if there is no `CONTEXT` available. -#[inline] -#[track_caller] -fn with_context(f: F) -> R -where - F: for<'a, 'b> FnOnce(&'b mut (&mut ExtCtxt<'a>, CLIENT)) -> R, -{ - let (ectx, client_opt) = TLV.get(); - let ectx = NonNull::new(ectx).expect("no CONTEXT stored in tls"); - - // We could get an `CONTEXT` pointer from another thread. - // Ensure that `CONTEXT` is `DynSync`. - // FIXME(pr-time): we should not be able to? - // sync::assert_dyn_sync::>(); - - // prevent double entering, as that would allow creating two `&mut ExtCtxt`s - // FIXME(pr-time): probably use a RefCell instead (which checks this properly)? - TLV.with(|tlv| { - let old = tlv.replace((ptr::null_mut(), None)); - let _reset = rustc_data_structures::defer(move || tlv.set(old)); - let ectx = { - let mut casted = ectx.cast::>(); - unsafe { casted.as_mut() } - }; - - f(&mut (ectx, client_opt.unwrap())) - }) -} diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index 25904848abba8..ba3838531c3e3 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -18,7 +18,6 @@ extern crate proc_macro as pm; mod build; -mod derive_macro_expansion; mod errors; // FIXME(Nilstrieb) Translate macro_rules diagnostics #[allow(rustc::untranslatable_diagnostic)] @@ -36,7 +35,7 @@ pub mod module; pub mod proc_macro; pub fn provide(providers: &mut rustc_middle::util::Providers) { - providers.derive_macro_expansion = derive_macro_expansion::provide_derive_macro_expansion; + providers.derive_macro_expansion = proc_macro::provide_derive_macro_expansion; } rustc_fluent_macro::fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 4d8322559dc6f..1b7c3ecf6861a 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -1,13 +1,17 @@ +use std::cell::Cell; +use std::ptr::{self, NonNull}; + use rustc_ast as ast; use rustc_ast::ptr::P; use rustc_ast::tokenstream::TokenStream; +use rustc_data_structures::svh::Svh; use rustc_errors::ErrorGuaranteed; -use rustc_middle::ty; +use rustc_middle::ty::{self, TyCtxt}; use rustc_parse::parser::{ForceCollect, Parser}; use rustc_session::Session; use rustc_session::config::ProcMacroExecutionStrategy; -use rustc_span::Span; use rustc_span::profiling::SpannedEventArgRecorder; +use rustc_span::{LocalExpnId, Span}; use crate::base::{self, *}; use crate::{errors, proc_macro_server}; @@ -154,11 +158,9 @@ impl MultiItemModifier for DeriveProcMacro { // well)? if tcx.sess.opts.incremental.is_some() && tcx.sess.opts.unstable_opts.cache_proc_macros { - crate::derive_macro_expansion::enter_context((ecx, self.client), move || { - tcx.derive_macro_expansion(key).cloned() - }) + enter_context((ecx, self.client), move || tcx.derive_macro_expansion(key).cloned()) } else { - crate::derive_macro_expansion::provide_derive_macro_expansion(tcx, key).cloned() + provide_derive_macro_expansion(tcx, key).cloned() } }); let Ok(output) = res else { @@ -195,3 +197,94 @@ impl MultiItemModifier for DeriveProcMacro { ExpandResult::Ready(items) } } + +pub(super) fn provide_derive_macro_expansion<'tcx>( + tcx: TyCtxt<'tcx>, + key: (LocalExpnId, Svh, &'tcx TokenStream), +) -> Result<&'tcx TokenStream, ()> { + let (invoc_id, _macro_crate_hash, input) = key; + + with_context(|(ecx, client)| { + let invoc_expn_data = invoc_id.expn_data(); + let span = invoc_expn_data.call_site; + let event_arg = invoc_expn_data.kind.descr(); + let _timer = tcx.sess.prof.generic_activity_with_arg_recorder( + "expand_derive_proc_macro_inner", + |recorder| { + recorder.record_arg_with_span(tcx.sess.source_map(), event_arg.clone(), span); + }, + ); + + let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; + let strategy = crate::proc_macro::exec_strategy(tcx.sess); + let server = crate::proc_macro_server::Rustc::new(ecx); + + match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { + Ok(stream) => Ok(tcx.arena.alloc(stream) as &TokenStream), + Err(e) => { + tcx.dcx().emit_err({ + errors::ProcMacroDerivePanicked { + span, + message: e.as_str().map(|message| errors::ProcMacroDerivePanickedHelp { + message: message.into(), + }), + } + }); + Err(()) + } + } + }) +} + +type CLIENT = pm::bridge::client::Client; + +// based on rust/compiler/rustc_middle/src/ty/context/tls.rs +thread_local! { + /// A thread local variable that stores a pointer to the current `CONTEXT`. + static TLV: Cell<(*mut (), Option)> = const { Cell::new((ptr::null_mut(), None)) }; +} + +/// Sets `context` as the new current `CONTEXT` for the duration of the function `f`. +#[inline] +pub(crate) fn enter_context<'a, F, R>(context: (&mut ExtCtxt<'a>, CLIENT), f: F) -> R +where + F: FnOnce() -> R, +{ + let (ectx, client) = context; + let erased = (ectx as *mut _ as *mut (), Some(client)); + TLV.with(|tlv| { + let old = tlv.replace(erased); + let _reset = rustc_data_structures::defer(move || tlv.set(old)); + f() + }) +} + +/// Allows access to the current `CONTEXT`. +/// Panics if there is no `CONTEXT` available. +#[inline] +#[track_caller] +fn with_context(f: F) -> R +where + F: for<'a, 'b> FnOnce(&'b mut (&mut ExtCtxt<'a>, CLIENT)) -> R, +{ + let (ectx, client_opt) = TLV.get(); + let ectx = NonNull::new(ectx).expect("no CONTEXT stored in tls"); + + // We could get an `CONTEXT` pointer from another thread. + // Ensure that `CONTEXT` is `DynSync`. + // FIXME(pr-time): we should not be able to? + // sync::assert_dyn_sync::>(); + + // prevent double entering, as that would allow creating two `&mut ExtCtxt`s + // FIXME(pr-time): probably use a RefCell instead (which checks this properly)? + TLV.with(|tlv| { + let old = tlv.replace((ptr::null_mut(), None)); + let _reset = rustc_data_structures::defer(move || tlv.set(old)); + let ectx = { + let mut casted = ectx.cast::>(); + unsafe { casted.as_mut() } + }; + + f(&mut (ectx, client_opt.unwrap())) + }) +} From 39002fbeb550605c020c0262436fa4b30134b757 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sat, 28 Dec 2024 18:35:08 +0100 Subject: [PATCH 27/28] fix: Make non-context expansion actually work --- compiler/rustc_expand/src/proc_macro.rs | 91 ++++++++++++++----------- 1 file changed, 50 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 1b7c3ecf6861a..4933b533f1956 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -143,26 +143,27 @@ impl MultiItemModifier for DeriveProcMacro { let invoc_id = ecx.current_expansion.id; let invoc_expn_data = invoc_id.expn_data(); - // FIXME(pr-time): Just using the crate hash to notice when the proc-macro code has - // changed. How to *correctly* depend on exactly the macro definition? - // I.e., depending on the crate hash is just a HACK, and ideally the dependency would be - // more narrow. - let macro_def_id = invoc_expn_data.macro_def_id.unwrap(); - let proc_macro_crate_hash = tcx.crate_hash(macro_def_id.krate); - assert_eq!(invoc_expn_data.call_site, span); - let key = (invoc_id, proc_macro_crate_hash, input); - // FIXME(pr-time): Is this the correct way to check for incremental compilation (as - // well)? + // well as for `cache_proc_macros`)? if tcx.sess.opts.incremental.is_some() && tcx.sess.opts.unstable_opts.cache_proc_macros { + // FIXME(pr-time): Just using the crate hash to notice when the proc-macro code has + // changed. How to *correctly* depend on exactly the macro definition? + // I.e., depending on the crate hash is just a HACK, and ideally the dependency would be + // more narrow. + let macro_def_id = invoc_expn_data.macro_def_id.unwrap(); + let proc_macro_crate_hash = tcx.crate_hash(macro_def_id.krate); + + let key = (invoc_id, proc_macro_crate_hash, input); + enter_context((ecx, self.client), move || tcx.derive_macro_expansion(key).cloned()) } else { - provide_derive_macro_expansion(tcx, key).cloned() + expand_derive_macro(tcx, invoc_id, input, ecx, self.client).cloned() } }); + let Ok(output) = res else { // error will already have been emitted return ExpandResult::Ready(vec![]); @@ -204,40 +205,48 @@ pub(super) fn provide_derive_macro_expansion<'tcx>( ) -> Result<&'tcx TokenStream, ()> { let (invoc_id, _macro_crate_hash, input) = key; - with_context(|(ecx, client)| { - let invoc_expn_data = invoc_id.expn_data(); - let span = invoc_expn_data.call_site; - let event_arg = invoc_expn_data.kind.descr(); - let _timer = tcx.sess.prof.generic_activity_with_arg_recorder( - "expand_derive_proc_macro_inner", - |recorder| { - recorder.record_arg_with_span(tcx.sess.source_map(), event_arg.clone(), span); - }, - ); - - let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; - let strategy = crate::proc_macro::exec_strategy(tcx.sess); - let server = crate::proc_macro_server::Rustc::new(ecx); - - match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { - Ok(stream) => Ok(tcx.arena.alloc(stream) as &TokenStream), - Err(e) => { - tcx.dcx().emit_err({ - errors::ProcMacroDerivePanicked { - span, - message: e.as_str().map(|message| errors::ProcMacroDerivePanickedHelp { - message: message.into(), - }), - } - }); - Err(()) - } - } - }) + with_context(|(ecx, client)| expand_derive_macro(tcx, invoc_id, input, ecx, *client)) } type CLIENT = pm::bridge::client::Client; +fn expand_derive_macro<'tcx>( + tcx: TyCtxt<'tcx>, + invoc_id: LocalExpnId, + input: &'tcx TokenStream, + ecx: &mut ExtCtxt<'_>, + client: CLIENT, +) -> Result<&'tcx TokenStream, ()> { + let invoc_expn_data = invoc_id.expn_data(); + let span = invoc_expn_data.call_site; + let event_arg = invoc_expn_data.kind.descr(); + let _timer = tcx.sess.prof.generic_activity_with_arg_recorder( + "expand_derive_proc_macro_inner", + |recorder| { + recorder.record_arg_with_span(tcx.sess.source_map(), event_arg.clone(), span); + }, + ); + + let proc_macro_backtrace = ecx.ecfg.proc_macro_backtrace; + let strategy = crate::proc_macro::exec_strategy(tcx.sess); + let server = crate::proc_macro_server::Rustc::new(ecx); + + match client.run(&strategy, server, input.clone(), proc_macro_backtrace) { + Ok(stream) => Ok(tcx.arena.alloc(stream) as &TokenStream), + Err(e) => { + tcx.dcx().emit_err({ + errors::ProcMacroDerivePanicked { + span, + message: e.as_str().map(|message| errors::ProcMacroDerivePanickedHelp { + message: message.into(), + }), + } + }); + Err(()) + } + } +} + // based on rust/compiler/rustc_middle/src/ty/context/tls.rs thread_local! { /// A thread local variable that stores a pointer to the current `CONTEXT`. From 0e125a55f3456a697058be2f2b138a90c14dd79e Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Fri, 28 Mar 2025 23:21:02 +0100 Subject: [PATCH 28/28] fix: Build was broken, probably should squash all commits now or sth. --- compiler/rustc_expand/src/expand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 8ab18b920b193..49c6112a0470a 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -834,7 +834,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { }, InvocationKind::GlobDelegation { item, of_trait } => { let AssocItemKind::DelegationMac(deleg) = &item.kind else { unreachable!() }; - let suffixes = match &ext.kind { + let suffixes = match ext { SyntaxExtensionKind::GlobDelegation(expander) => { match expander.expand(self.cx) { ExpandResult::Ready(suffixes) => suffixes,