Auto merge of rust-lang#97004 - nnethercote:proc-macro-tweaks, r=eddyb

bors · bors · commit eaf5b6564cee · 2022-05-27T06:09:45.000Z
Proc macro tweaks

Various improvements I spotted while looking through the proc macro code.

r? `@eddyb`
diff --git a/proc_macro/src/bridge/buffer.rs b/proc_macro/src/bridge/buffer.rs
@@ -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,
     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()
     }
@@ -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());
@@ -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());
@@ -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.
@@ -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())
@@ -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));
         }
 
diff --git a/proc_macro/src/bridge/client.rs b/proc_macro/src/bridge/client.rs
@@ -49,7 +49,9 @@ macro_rules! define_handles {
             #[repr(C)]
             pub(crate) struct $oty {
                 handle: handle::Handle,
-                // Prevent Send and Sync impls
+                // Prevent Send and Sync impls. `!Send`/`!Sync` is the usual
+                // way of doing this, but that requires unstable features.
+                // rust-analyzer uses this code and avoids unstable features.
                 _marker: PhantomData<*mut ()>,
             }
 
@@ -133,7 +135,9 @@ macro_rules! define_handles {
             #[derive(Copy, Clone, PartialEq, Eq, Hash)]
             pub(crate) struct $ity {
                 handle: handle::Handle,
-                // Prevent Send and Sync impls
+                // Prevent Send and Sync impls. `!Send`/`!Sync` is the usual
+                // way of doing this, but that requires unstable features.
+                // rust-analyzer uses this code and avoids unstable features.
                 _marker: PhantomData<*mut ()>,
             }
 
@@ -191,7 +195,7 @@ define_handles! {
 // FIXME(eddyb) generate these impls by pattern-matching on the
 // names of methods - also could use the presence of `fn drop`
 // to distinguish between 'owned and 'interned, above.
-// Alternatively, special 'modes" could be listed of types in with_api
+// Alternatively, special "modes" could be listed of types in with_api
 // instead of pattern matching on methods, here and in server decl.
 
 impl Clone for TokenStream {
@@ -250,17 +254,17 @@ macro_rules! define_client_side {
         $(impl $name {
             $(pub(crate) fn $method($($arg: $arg_ty),*) $(-> $ret_ty)* {
                 Bridge::with(|bridge| {
-                    let mut b = bridge.cached_buffer.take();
+                    let mut buf = bridge.cached_buffer.take();
 
-                    b.clear();
-                    api_tags::Method::$name(api_tags::$name::$method).encode(&mut b, &mut ());
-                    reverse_encode!(b; $($arg),*);
+                    buf.clear();
+                    api_tags::Method::$name(api_tags::$name::$method).encode(&mut buf, &mut ());
+                    reverse_encode!(buf; $($arg),*);
 
-                    b = bridge.dispatch.call(b);
+                    buf = bridge.dispatch.call(buf);
 
-                    let r = Result::<_, PanicMessage>::decode(&mut &b[..], &mut ());
+                    let r = Result::<_, PanicMessage>::decode(&mut &buf[..], &mut ());
 
-                    bridge.cached_buffer = b;
+                    bridge.cached_buffer = buf;
 
                     r.unwrap_or_else(|e| panic::resume_unwind(e.into()))
                 })
@@ -367,7 +371,7 @@ pub struct Client<F> {
     // FIXME(eddyb) use a reference to the `static COUNTERS`, instead of
     // a wrapper `fn` pointer, once `const fn` can reference `static`s.
     pub(super) get_handle_counters: extern "C" fn() -> &'static HandleCounters,
-    pub(super) run: extern "C" fn(Bridge<'_>, F) -> Buffer<u8>,
+    pub(super) run: extern "C" fn(Bridge<'_>, F) -> Buffer,
     pub(super) f: F,
 }
 
@@ -377,22 +381,22 @@ pub struct Client<F> {
 fn run_client<A: for<'a, 's> DecodeMut<'a, 's, ()>, R: Encode<()>>(
     mut bridge: Bridge<'_>,
     f: impl FnOnce(A) -> R,
-) -> Buffer<u8> {
+) -> Buffer {
     // The initial `cached_buffer` contains the input.
-    let mut b = bridge.cached_buffer.take();
+    let mut buf = bridge.cached_buffer.take();
 
     panic::catch_unwind(panic::AssertUnwindSafe(|| {
         bridge.enter(|| {
-            let reader = &mut &b[..];
+            let reader = &mut &buf[..];
             let input = A::decode(reader, &mut ());
 
             // Put the `cached_buffer` back in the `Bridge`, for requests.
-            Bridge::with(|bridge| bridge.cached_buffer = b.take());
+            Bridge::with(|bridge| bridge.cached_buffer = buf.take());
 
             let output = f(input);
 
             // Take the `cached_buffer` back out, for the output value.
-            b = Bridge::with(|bridge| bridge.cached_buffer.take());
+            buf = Bridge::with(|bridge| bridge.cached_buffer.take());
 
             // HACK(eddyb) Separate encoding a success value (`Ok(output)`)
             // from encoding a panic (`Err(e: PanicMessage)`) to avoid
@@ -403,24 +407,24 @@ fn run_client<A: for<'a, 's> DecodeMut<'a, 's, ()>, R: Encode<()>>(
             // this is defensively trying to avoid any accidental panicking
             // reaching the `extern "C"` (which should `abort` but might not
             // at the moment, so this is also potentially preventing UB).
-            b.clear();
-            Ok::<_, ()>(output).encode(&mut b, &mut ());
+            buf.clear();
+            Ok::<_, ()>(output).encode(&mut buf, &mut ());
         })
     }))
     .map_err(PanicMessage::from)
     .unwrap_or_else(|e| {
-        b.clear();
-        Err::<(), _>(e).encode(&mut b, &mut ());
+        buf.clear();
+        Err::<(), _>(e).encode(&mut buf, &mut ());
     });
-    b
+    buf
 }
 
 impl Client<fn(crate::TokenStream) -> crate::TokenStream> {
     pub const fn expand1(f: fn(crate::TokenStream) -> crate::TokenStream) -> Self {
         extern "C" fn run(
             bridge: Bridge<'_>,
             f: impl FnOnce(crate::TokenStream) -> crate::TokenStream,
-        ) -> Buffer<u8> {
+        ) -> Buffer {
             run_client(bridge, |input| f(crate::TokenStream(input)).0)
         }
         Client { get_handle_counters: HandleCounters::get, run, f }
@@ -434,7 +438,7 @@ impl Client<fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream> {
         extern "C" fn run(
             bridge: Bridge<'_>,
             f: impl FnOnce(crate::TokenStream, crate::TokenStream) -> crate::TokenStream,
-        ) -> Buffer<u8> {
+        ) -> Buffer {
             run_client(bridge, |(input, input2)| {
                 f(crate::TokenStream(input), crate::TokenStream(input2)).0
             })
diff --git a/proc_macro/src/bridge/closure.rs b/proc_macro/src/bridge/closure.rs
@@ -6,7 +6,11 @@ use std::marker::PhantomData;
 pub struct Closure<'a, A, R> {
     call: unsafe extern "C" fn(*mut Env, A) -> R,
     env: *mut Env,
-    // Ensure Closure is !Send and !Sync
+    // Prevent Send and Sync impls. `!Send`/`!Sync` is the usual way of doing
+    // this, but that requires unstable features. rust-analyzer uses this code
+    // and avoids unstable features.
+    //
+    // The `'a` lifetime parameter represents the lifetime of `Env`.
     _marker: PhantomData<*mut &'a mut ()>,
 }
 
diff --git a/proc_macro/src/bridge/handle.rs b/proc_macro/src/bridge/handle.rs
@@ -8,6 +8,8 @@ use std::sync::atomic::{AtomicUsize, Ordering};
 
 pub(super) type Handle = NonZeroU32;
 
+/// A store that associates values of type `T` with numeric handles. A value can
+/// be looked up using its handle.
 pub(super) struct OwnedStore<T: 'static> {
     counter: &'static AtomicUsize,
     data: BTreeMap<Handle, T>,
@@ -49,6 +51,7 @@ impl<T> IndexMut<Handle> for OwnedStore<T> {
     }
 }
 
+/// Like `OwnedStore`, but avoids storing any value more than once.
 pub(super) struct InternedStore<T: 'static> {
     owned: OwnedStore<T>,
     interner: HashMap<T, Handle>,
diff --git a/proc_macro/src/bridge/mod.rs b/proc_macro/src/bridge/mod.rs
@@ -176,7 +176,7 @@ macro_rules! with_api {
 }
 
 // FIXME(eddyb) this calls `encode` for each argument, but in reverse,
-// to avoid borrow conflicts from borrows started by `&mut` arguments.
+// to match the ordering in `reverse_decode`.
 macro_rules! reverse_encode {
     ($writer:ident;) => {};
     ($writer:ident; $first:ident $(, $rest:ident)*) => {
@@ -224,15 +224,17 @@ use rpc::{Decode, DecodeMut, Encode, Reader, Writer};
 pub struct Bridge<'a> {
     /// Reusable buffer (only `clear`-ed, never shrunk), primarily
     /// used for making requests, but also for passing input to client.
-    cached_buffer: Buffer<u8>,
+    cached_buffer: Buffer,
 
     /// Server-side function that the client uses to make requests.
-    dispatch: closure::Closure<'a, Buffer<u8>, Buffer<u8>>,
+    dispatch: closure::Closure<'a, Buffer, Buffer>,
 
     /// If 'true', always invoke the default panic hook
     force_show_panics: bool,
 
-    // Prevent Send and Sync impls
+    // Prevent Send and Sync impls. `!Send`/`!Sync` is the usual way of doing
+    // this, but that requires unstable features. rust-analyzer uses this code
+    // and avoids unstable features.
     _marker: marker::PhantomData<*mut ()>,
 }
 
@@ -252,7 +254,6 @@ mod api_tags {
                 rpc_encode_decode!(enum $name { $($method),* });
             )*
 
-
             pub(super) enum Method {
                 $($name($name)),*
             }
diff --git a/proc_macro/src/bridge/rpc.rs b/proc_macro/src/bridge/rpc.rs
@@ -7,7 +7,7 @@ use std::num::NonZeroU32;
 use std::ops::Bound;
 use std::str;
 
-pub(super) type Writer = super::buffer::Buffer<u8>;
+pub(super) type Writer = super::buffer::Buffer;
 
 pub(super) trait Encode<S>: Sized {
     fn encode(self, w: &mut Writer, s: &mut S);
diff --git a/proc_macro/src/bridge/server.rs b/proc_macro/src/bridge/server.rs

Original file line number	Diff line number	Diff line change
`@@ -6,37 +6,37 @@ use std::ops::{Deref, DerefMut};`
`6`	`6`	`use std::slice;`
`7`	`7`
`8`	`8`	`#[repr(C)]`
`9`		`-pub struct Buffer<T: Copy> {`
`10`		`- data: *mut T,`
	`9`	`+pub struct Buffer {`
	`10`	`+ data: *mut u8,`
`11`	`11`	`len: usize,`
`12`	`12`	`capacity: usize,`
`13`		`- reserve: extern "C" fn(Buffer<T>, usize) -> Buffer<T>,`
`14`		`- drop: extern "C" fn(Buffer<T>),`
	`13`	`+ reserve: extern "C" fn(Buffer, usize) -> Buffer,`
	`14`	`+ drop: extern "C" fn(Buffer),`
`15`	`15`	`}`
`16`	`16`
`17`		`-unsafe impl<T: Copy + Sync> Sync for Buffer<T> {}`
`18`		`-unsafe impl<T: Copy + Send> Send for Buffer<T> {}`
	`17`	`+unsafe impl Sync for Buffer {}`
	`18`	`+unsafe impl Send for Buffer {}`
`19`	`19`
`20`		`-impl<T: Copy> Default for Buffer<T> {`
	`20`	`+impl Default for Buffer {`
`21`	`21`	`fn default() -> Self {`
`22`	`22`	`Self::from(vec![])`
`23`	`23`	`}`
`24`	`24`	`}`
`25`	`25`
`26`		`-impl<T: Copy> Deref for Buffer<T> {`
`27`		`- type Target = [T];`
`28`		`- fn deref(&self) -> &[T] {`
`29`		`- unsafe { slice::from_raw_parts(self.data as *const T, self.len) }`
	`26`	`+impl Deref for Buffer {`
	`27`	`+ type Target = [u8];`
	`28`	`+ fn deref(&self) -> &[u8] {`
	`29`	`+ unsafe { slice::from_raw_parts(self.data as *const u8, self.len) }`
`30`	`30`	`}`
`31`	`31`	`}`
`32`	`32`
`33`		`-impl<T: Copy> DerefMut for Buffer<T> {`
`34`		`- fn deref_mut(&mut self) -> &mut [T] {`
	`33`	`+impl DerefMut for Buffer {`
	`34`	`+ fn deref_mut(&mut self) -> &mut [u8] {`
`35`	`35`	`unsafe { slice::from_raw_parts_mut(self.data, self.len) }`
`36`	`36`	`}`
`37`	`37`	`}`
`38`	`38`
`39`		`-impl<T: Copy> Buffer<T> {`
	`39`	`+impl Buffer {`
`40`	`40`	`pub(super) fn new() -> Self {`
`41`	`41`	`Self::default()`
`42`	`42`	`}`
`@@ -53,7 +53,7 @@ impl<T: Copy> Buffer<T> {`
`53`	`53`	`// because in the case of small arrays, codegen can be more efficient`
`54`	`54`	`// (avoiding a memmove call). With extend_from_slice, LLVM at least`
`55`	`55`	`// currently is not able to make that optimization.`
`56`		`- pub(super) fn extend_from_array<const N: usize>(&mut self, xs: &[T; N]) {`
	`56`	`+ pub(super) fn extend_from_array<const N: usize>(&mut self, xs: &[u8; N]) {`
`57`	`57`	`if xs.len() > (self.capacity - self.len) {`
`58`	`58`	`let b = self.take();`
`59`	`59`	`*self = (b.reserve)(b, xs.len());`
`@@ -64,7 +64,7 @@ impl<T: Copy> Buffer<T> {`
`64`	`64`	`}`
`65`	`65`	`}`
`66`	`66`
`67`		`- pub(super) fn extend_from_slice(&mut self, xs: &[T]) {`
	`67`	`+ pub(super) fn extend_from_slice(&mut self, xs: &[u8]) {`
`68`	`68`	`if xs.len() > (self.capacity - self.len) {`
`69`	`69`	`let b = self.take();`
`70`	`70`	`*self = (b.reserve)(b, xs.len());`
`@@ -75,7 +75,7 @@ impl<T: Copy> Buffer<T> {`
`75`	`75`	`}`
`76`	`76`	`}`
`77`	`77`
`78`		`- pub(super) fn push(&mut self, v: T) {`
	`78`	`+ pub(super) fn push(&mut self, v: u8) {`
`79`	`79`	`// The code here is taken from Vec::push, and we know that reserve()`
`80`	`80`	`// will panic if we're exceeding isize::MAX bytes and so there's no need`
`81`	`81`	`// to check for overflow.`
`@@ -90,7 +90,7 @@ impl<T: Copy> Buffer<T> {`
`90`	`90`	`}`
`91`	`91`	`}`
`92`	`92`
`93`		`-impl Write for Buffer<u8> {`
	`93`	`+impl Write for Buffer {`
`94`	`94`	`fn write(&mut self, xs: &[u8]) -> io::Result<usize> {`
`95`	`95`	`self.extend_from_slice(xs);`
`96`	`96`	`Ok(xs.len())`
`@@ -106,35 +106,35 @@ impl Write for Buffer<u8> {`
`106`	`106`	`}`
`107`	`107`	`}`
`108`	`108`
`109`		`-impl<T: Copy> Drop for Buffer<T> {`
	`109`	`+impl Drop for Buffer {`
`110`	`110`	`fn drop(&mut self) {`
`111`	`111`	`let b = self.take();`
`112`	`112`	`(b.drop)(b);`
`113`	`113`	`}`
`114`	`114`	`}`
`115`	`115`
`116`		`-impl<T: Copy> From<Vec<T>> for Buffer<T> {`
`117`		`- fn from(mut v: Vec<T>) -> Self {`
	`116`	`+impl From<Vec<u8>> for Buffer {`
	`117`	`+ fn from(mut v: Vec<u8>) -> Self {`
`118`	`118`	`let (data, len, capacity) = (v.as_mut_ptr(), v.len(), v.capacity());`
`119`	`119`	`mem::forget(v);`
`120`	`120`
`121`	`121`	`// This utility function is nested in here because it can only`
`122`	`122`	// be safely called on `Buffer`s created by this `proc_macro`.
`123`		`- fn to_vec<T: Copy>(b: Buffer<T>) -> Vec<T> {`
	`123`	`+ fn to_vec(b: Buffer) -> Vec<u8> {`
`124`	`124`	`unsafe {`
`125`	`125`	`let Buffer { data, len, capacity, .. } = b;`
`126`	`126`	`mem::forget(b);`
`127`	`127`	`Vec::from_raw_parts(data, len, capacity)`
`128`	`128`	`}`
`129`	`129`	`}`
`130`	`130`
`131`		`- extern "C" fn reserve<T: Copy>(b: Buffer<T>, additional: usize) -> Buffer<T> {`
	`131`	`+ extern "C" fn reserve(b: Buffer, additional: usize) -> Buffer {`
`132`	`132`	`let mut v = to_vec(b);`
`133`	`133`	`v.reserve(additional);`
`134`	`134`	`Buffer::from(v)`
`135`	`135`	`}`
`136`	`136`
`137`		`- extern "C" fn drop<T: Copy>(b: Buffer<T>) {`
	`137`	`+ extern "C" fn drop(b: Buffer) {`
`138`	`138`	`mem::drop(to_vec(b));`
`139`	`139`	`}`
`140`	`140`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,8 @@ use std::sync::atomic::{AtomicUsize, Ordering};`
`8`	`8`
`9`	`9`	`pub(super) type Handle = NonZeroU32;`
`10`	`10`
	`11`	+/// A store that associates values of type `T` with numeric handles. A value can
	`12`	`+/// be looked up using its handle.`
`11`	`13`	`pub(super) struct OwnedStore<T: 'static> {`
`12`	`14`	`counter: &'static AtomicUsize,`
`13`	`15`	`data: BTreeMap<Handle, T>,`
`@@ -49,6 +51,7 @@ impl<T> IndexMut<Handle> for OwnedStore<T> {`
`49`	`51`	`}`
`50`	`52`	`}`
`51`	`53`
	`54`	+/// Like `OwnedStore`, but avoids storing any value more than once.
`52`	`55`	`pub(super) struct InternedStore<T: 'static> {`
`53`	`56`	`owned: OwnedStore<T>,`
`54`	`57`	`interner: HashMap<T, Handle>,`