Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Proc macro tweaks #97004

Merged
merged 12 commits into from
May 27, 2022
44 changes: 17 additions & 27 deletions compiler/rustc_builtin_macros/src/proc_macro_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,16 @@ struct ProcMacroDerive {
attrs: Vec<Symbol>,
}

enum ProcMacroDefType {
Attr,
Bang,
}

struct ProcMacroDef {
id: NodeId,
function_name: Ident,
span: Span,
def_type: ProcMacroDefType,
}

enum ProcMacro {
Derive(ProcMacroDerive),
Def(ProcMacroDef),
Attr(ProcMacroDef),
Bang(ProcMacroDef),
}
nnethercote marked this conversation as resolved.
Show resolved Hide resolved

struct CollectProcMacros<'a> {
Expand Down Expand Up @@ -128,11 +123,10 @@ impl<'a> CollectProcMacros<'a> {

fn collect_attr_proc_macro(&mut self, item: &'a ast::Item) {
if self.in_root && item.vis.kind.is_pub() {
self.macros.push(ProcMacro::Def(ProcMacroDef {
self.macros.push(ProcMacro::Attr(ProcMacroDef {
id: item.id,
span: item.span,
function_name: item.ident,
def_type: ProcMacroDefType::Attr,
}));
} else {
let msg = if !self.in_root {
Expand All @@ -147,11 +141,10 @@ impl<'a> CollectProcMacros<'a> {

fn collect_bang_proc_macro(&mut self, item: &'a ast::Item) {
if self.in_root && item.vis.kind.is_pub() {
self.macros.push(ProcMacro::Def(ProcMacroDef {
self.macros.push(ProcMacro::Bang(ProcMacroDef {
id: item.id,
span: item.span,
function_name: item.ident,
def_type: ProcMacroDefType::Bang,
}));
} else {
let msg = if !self.in_root {
Expand Down Expand Up @@ -308,6 +301,17 @@ fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P<ast::Item> {
let proc_macro_ty_method_path = |cx: &ExtCtxt<'_>, method| {
cx.expr_path(cx.path(span, vec![proc_macro, bridge, client, proc_macro_ty, method]))
};
let attr_or_bang = |cx: &mut ExtCtxt<'_>, ca: &ProcMacroDef, ident| {
cx.resolver.declare_proc_macro(ca.id);
cx.expr_call(
span,
proc_macro_ty_method_path(cx, ident),
vec![
cx.expr_str(ca.span, ca.function_name.name),
local_path(cx, ca.span, ca.function_name),
],
)
};
macros
.iter()
.map(|m| match m {
Expand All @@ -329,22 +333,8 @@ fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P<ast::Item> {
],
)
}
ProcMacro::Def(ca) => {
cx.resolver.declare_proc_macro(ca.id);
let ident = match ca.def_type {
ProcMacroDefType::Attr => attr,
ProcMacroDefType::Bang => bang,
};

cx.expr_call(
span,
proc_macro_ty_method_path(cx, ident),
vec![
cx.expr_str(ca.span, ca.function_name.name),
local_path(cx, ca.span, ca.function_name),
],
)
}
ProcMacro::Attr(ca) => attr_or_bang(cx, &ca, attr),
ProcMacro::Bang(ca) => attr_or_bang(cx, &ca, bang),
nnethercote marked this conversation as resolved.
Show resolved Hide resolved
})
.collect()
};
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ where
}
}

pub trait ProcMacro {
pub trait BangProcMacro {
fn expand<'cx>(
&self,
ecx: &'cx mut ExtCtxt<'_>,
Expand All @@ -275,7 +275,7 @@ pub trait ProcMacro {
) -> Result<TokenStream, ErrorGuaranteed>;
}

impl<F> ProcMacro for F
impl<F> BangProcMacro for F
where
F: Fn(TokenStream) -> TokenStream,
{
Expand Down Expand Up @@ -640,7 +640,7 @@ pub enum SyntaxExtensionKind {
/// A token-based function-like macro.
Bang(
/// An expander with signature TokenStream -> TokenStream.
Box<dyn ProcMacro + sync::Sync + sync::Send>,
Box<dyn BangProcMacro + sync::Sync + sync::Send>,
),

/// An AST-based function-like macro.
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_expand/src/proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub struct BangProcMacro {
pub client: pm::bridge::client::Client<fn(pm::TokenStream) -> pm::TokenStream>,
}

impl base::ProcMacro for BangProcMacro {
impl base::BangProcMacro for BangProcMacro {
fn expand<'cx>(
&self,
ecx: &'cx mut ExtCtxt<'_>,
Expand Down Expand Up @@ -72,11 +72,11 @@ impl base::AttrProcMacro for AttrProcMacro {
}
}

pub struct ProcMacroDerive {
pub struct DeriveProcMacro {
pub client: pm::bridge::client::Client<fn(pm::TokenStream) -> pm::TokenStream>,
}

impl MultiItemModifier for ProcMacroDerive {
impl MultiItemModifier for DeriveProcMacro {
fn expand(
&self,
ecx: &mut ExtCtxt<'_>,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::{Lock, LockGuard, Lrc, OnceCell};
use rustc_data_structures::unhash::UnhashMap;
use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, ProcMacroDerive};
use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, DeriveProcMacro};
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::definitions::{DefKey, DefPath, DefPathData, DefPathHash};
Expand Down Expand Up @@ -837,7 +837,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
attributes.iter().cloned().map(Symbol::intern).collect::<Vec<_>>();
(
trait_name,
SyntaxExtensionKind::Derive(Box::new(ProcMacroDerive { client })),
SyntaxExtensionKind::Derive(Box::new(DeriveProcMacro { client })),
helper_attrs,
)
}
Expand Down
48 changes: 24 additions & 24 deletions library/proc_macro/src/bridge/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,37 @@ use std::ops::{Deref, DerefMut};
use std::slice;

#[repr(C)]
pub struct Buffer<T: Copy> {
data: *mut T,
pub struct Buffer {
data: *mut u8,
Comment on lines -9 to +10
Copy link
Member

Choose a reason for hiding this comment

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

For the record, I think the reason I made this generic was to avoid accidentally doing the wrong thing because the types happened to match up (especially since there's unsafe code here) but I'll review this carefully and hopefully this is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would any type other than u8 make sense here? Doesn't seem like it, especially given that Buffer<u8> was hardcoded in a bunch of places anyway.

Copy link
Member

Choose a reason for hiding this comment

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

That's not what I meant, but rather that writing parametric code can sometimes help avoid making mistakes (which can be catastrophic in unsafe code).

It doesn't make sense at all from the perspective of whether e.g. a C++ container might be templated or not, but in does in Rust because of the type-checking in the generic form.

Anyway it doesn't matter much, I was just explaining that Foo<T> can still make sense even if it's only ever used as a single Foo<Concrete> and the generic nature "isn't taken advantage of".

Hopefully we can remove this Buffer abstraction and move more to e.g. a model closer to read/write syscalls (or io::{Read,Write} traits I suppose, at a higher level).

len: usize,
capacity: usize,
reserve: extern "C" fn(Buffer<T>, usize) -> Buffer<T>,
drop: extern "C" fn(Buffer<T>),
reserve: extern "C" fn(Buffer, usize) -> Buffer,
drop: extern "C" fn(Buffer),
}

unsafe impl<T: Copy + Sync> Sync for Buffer<T> {}
unsafe impl<T: Copy + Send> Send for Buffer<T> {}
unsafe impl Sync for Buffer {}
unsafe impl Send for Buffer {}

impl<T: Copy> Default for Buffer<T> {
impl Default for Buffer {
fn default() -> Self {
Self::from(vec![])
}
}

impl<T: Copy> Deref for Buffer<T> {
type Target = [T];
fn deref(&self) -> &[T] {
unsafe { slice::from_raw_parts(self.data as *const T, self.len) }
impl Deref for Buffer {
type Target = [u8];
fn deref(&self) -> &[u8] {
unsafe { slice::from_raw_parts(self.data as *const u8, self.len) }
}
}

impl<T: Copy> DerefMut for Buffer<T> {
fn deref_mut(&mut self) -> &mut [T] {
impl DerefMut for Buffer {
fn deref_mut(&mut self) -> &mut [u8] {
unsafe { slice::from_raw_parts_mut(self.data, self.len) }
}
}

impl<T: Copy> Buffer<T> {
impl Buffer {
pub(super) fn new() -> Self {
Self::default()
}
Expand All @@ -53,7 +53,7 @@ impl<T: Copy> Buffer<T> {
// because in the case of small arrays, codegen can be more efficient
// (avoiding a memmove call). With extend_from_slice, LLVM at least
// currently is not able to make that optimization.
pub(super) fn extend_from_array<const N: usize>(&mut self, xs: &[T; N]) {
pub(super) fn extend_from_array<const N: usize>(&mut self, xs: &[u8; N]) {
if xs.len() > (self.capacity - self.len) {
let b = self.take();
*self = (b.reserve)(b, xs.len());
Expand All @@ -64,7 +64,7 @@ impl<T: Copy> Buffer<T> {
}
}

pub(super) fn extend_from_slice(&mut self, xs: &[T]) {
pub(super) fn extend_from_slice(&mut self, xs: &[u8]) {
if xs.len() > (self.capacity - self.len) {
let b = self.take();
*self = (b.reserve)(b, xs.len());
Expand All @@ -75,7 +75,7 @@ impl<T: Copy> Buffer<T> {
}
}

pub(super) fn push(&mut self, v: T) {
pub(super) fn push(&mut self, v: u8) {
// The code here is taken from Vec::push, and we know that reserve()
// will panic if we're exceeding isize::MAX bytes and so there's no need
// to check for overflow.
Expand All @@ -90,7 +90,7 @@ impl<T: Copy> Buffer<T> {
}
}

impl Write for Buffer<u8> {
impl Write for Buffer {
fn write(&mut self, xs: &[u8]) -> io::Result<usize> {
self.extend_from_slice(xs);
Ok(xs.len())
Expand All @@ -106,35 +106,35 @@ impl Write for Buffer<u8> {
}
}

impl<T: Copy> Drop for Buffer<T> {
impl Drop for Buffer {
fn drop(&mut self) {
let b = self.take();
(b.drop)(b);
}
}

impl<T: Copy> From<Vec<T>> for Buffer<T> {
fn from(mut v: Vec<T>) -> Self {
impl From<Vec<u8>> for Buffer {
fn from(mut v: Vec<u8>) -> Self {
let (data, len, capacity) = (v.as_mut_ptr(), v.len(), v.capacity());
mem::forget(v);

// This utility function is nested in here because it can *only*
// be safely called on `Buffer`s created by *this* `proc_macro`.
fn to_vec<T: Copy>(b: Buffer<T>) -> Vec<T> {
fn to_vec(b: Buffer) -> Vec<u8> {
unsafe {
let Buffer { data, len, capacity, .. } = b;
mem::forget(b);
Vec::from_raw_parts(data, len, capacity)
}
}

extern "C" fn reserve<T: Copy>(b: Buffer<T>, additional: usize) -> Buffer<T> {
extern "C" fn reserve(b: Buffer, additional: usize) -> Buffer {
let mut v = to_vec(b);
v.reserve(additional);
Buffer::from(v)
}

extern "C" fn drop<T: Copy>(b: Buffer<T>) {
extern "C" fn drop(b: Buffer) {
mem::drop(to_vec(b));
}

Expand Down
Loading