mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(lexer): macros place attributes on correct functions (#28689)
Fix how the `cpu_version` and `cpu_multiversion` macros handle attributes on the function. Apply those attributes either to the dispatcher function or the implementation function, depending on their type.
This commit is contained in:
1 parent
83163d0928
commit
60a1860667
5 files changed
+197
-28
No files matched your search
@@ -1,3 +1,5 @@
|
||||
use std::panic::Location;
|
||||
|
||||
use super::{BASELINE, Cpu, DynCpu, cpu_multiversion, cpu_select, cpu_version, to_dyn};
|
||||
|
||||
#[cfg(target_arch = "x86_64")]
|
||||
@@ -143,6 +145,29 @@ fn nested_items(x: u32, _cpu: impl Cpu) -> u32 {
|
||||
Inner::new(x).total()
|
||||
}
|
||||
|
||||
// Lint attributes go on `__impl`, where the lint fires
|
||||
#[cpu_multiversion]
|
||||
#[expect(unused_variables, reason = "tests lint attributes go on the function with the body")]
|
||||
fn unused_in_body(x: u32, _cpu: impl Cpu) -> u32 {
|
||||
let unused = x;
|
||||
x
|
||||
}
|
||||
|
||||
// `#[cold]` goes on the entry points, with `#[inline]`
|
||||
#[cpu_multiversion]
|
||||
#[cold]
|
||||
#[inline(never)]
|
||||
fn cold_add_one(x: u32, _cpu: impl Cpu) -> u32 {
|
||||
x + 1
|
||||
}
|
||||
|
||||
// `#[track_caller]` goes on every generated function, so the caller's location passes through
|
||||
#[cpu_multiversion]
|
||||
#[track_caller]
|
||||
fn caller_line(_cpu: impl Cpu) -> u32 {
|
||||
Location::caller().line()
|
||||
}
|
||||
|
||||
struct Counter(u32);
|
||||
|
||||
impl Counter {
|
||||
@@ -191,6 +216,37 @@ impl Counter {
|
||||
Self(n).0 * 2
|
||||
}
|
||||
|
||||
// Lint attributes go on `__plus_unused_impl`, where the lint fires.
|
||||
// On `plus_unused` the expectation would be unfulfilled, as its body only calls entry points.
|
||||
#[cpu_multiversion]
|
||||
#[expect(unused_variables, reason = "tests lint attributes go on the function with the body")]
|
||||
fn plus_unused(&self, x: u32, _cpu: impl Cpu) -> u32 {
|
||||
let unused = x;
|
||||
self.0
|
||||
}
|
||||
|
||||
// rustc evaluates `#[cfg]` before the macro runs, wherever it is in the list.
|
||||
// The macro doesn't have to remove the sibling functions, or this would fail to compile.
|
||||
#[cpu_multiversion]
|
||||
#[cfg(any())]
|
||||
fn removed(&self, _cpu: impl Cpu) -> Missing {
|
||||
Missing
|
||||
}
|
||||
|
||||
// rustc expands `#[cfg_attr]` before the macro runs too, so the macro sees `#[expect]`
|
||||
#[cpu_multiversion]
|
||||
#[cfg_attr(all(), expect(unused_variables, reason = "tests `cfg_attr` is expanded first"))]
|
||||
fn plus_unused_cfg_attr(&self, x: u32, _cpu: impl Cpu) -> u32 {
|
||||
let unused = x;
|
||||
self.0
|
||||
}
|
||||
|
||||
#[cpu_multiversion]
|
||||
#[track_caller]
|
||||
fn caller_line(&self, _cpu: impl Cpu) -> u32 {
|
||||
Location::caller().line()
|
||||
}
|
||||
|
||||
#[cpu_multiversion]
|
||||
#[expect(clippy::inline_always, reason = "tests an `#[inline(always)]` method")]
|
||||
#[inline(always)]
|
||||
@@ -311,6 +367,18 @@ fn nested_items_with_self() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn attributes() {
|
||||
for cpu in [DynCpu::detect(), BASELINE_DYN] {
|
||||
assert_eq!(unused_in_body(3, cpu), 3);
|
||||
assert_eq!(cold_add_one(1, cpu), 2);
|
||||
assert_eq!(caller_line(cpu), line!());
|
||||
assert_eq!(Counter(5).plus_unused(1, cpu), 5);
|
||||
assert_eq!(Counter(5).plus_unused_cfg_attr(1, cpu), 5);
|
||||
assert_eq!(Counter(5).caller_line(cpu), line!());
|
||||
}
|
||||
}
|
||||
|
||||
#[cpu_version(Avx2)]
|
||||
#[inline]
|
||||
fn double_avx2(x: u32, _cpu: Avx2) -> u32 {
|
||||
@@ -541,6 +609,32 @@ macro_rules! wrapped_tests {
|
||||
Self(n).0 * 2
|
||||
}
|
||||
|
||||
// Lint attributes go on `__plus_unused_impl`, where the lint fires
|
||||
#[cpu_version($cpu)]
|
||||
#[expect(
|
||||
unused_variables,
|
||||
reason = "tests lint attributes go on the function with the body"
|
||||
)]
|
||||
fn plus_unused(&self, x: u32, _cpu: $cpu) -> u32 {
|
||||
let unused = x;
|
||||
self.0
|
||||
}
|
||||
|
||||
// rustc evaluates `#[cfg]` before the macro runs, wherever it is in the list.
|
||||
// The macro doesn't have to remove `__removed_impl`, or this would fail to compile.
|
||||
#[cpu_version($cpu)]
|
||||
#[cfg(any())]
|
||||
fn removed(&self, _cpu: $cpu) -> Missing {
|
||||
Missing
|
||||
}
|
||||
|
||||
// `#[track_caller]` goes on `__caller_line_impl` too, once
|
||||
#[cpu_version($cpu)]
|
||||
#[track_caller]
|
||||
fn caller_line(&self, _cpu: $cpu) -> u32 {
|
||||
Location::caller().line()
|
||||
}
|
||||
|
||||
#[cpu_version($cpu)]
|
||||
fn into_incremented(mut self, _cpu: $cpu) -> u32 {
|
||||
self.0 += 1;
|
||||
@@ -593,6 +687,8 @@ macro_rules! wrapped_tests {
|
||||
assert_eq!(total.doubled(cpu).into_incremented(cpu), 7);
|
||||
assert_eq!(Total::of(5, cpu).0, 5);
|
||||
assert_eq!(Total::double_of(6, cpu), 12);
|
||||
assert_eq!(Total(2).plus_unused(1, cpu), 2);
|
||||
assert_eq!(Total(2).caller_line(cpu), line!());
|
||||
|
||||
assert_eq!(Wrapper(3u8).convert::<Vec<u8>>(cpu), vec![3]);
|
||||
}
|
||||
|
||||
@@ -18,10 +18,8 @@ use syn::{
|
||||
pub struct Func {
|
||||
/// The function's original tokens. Output when a macro leaves the function unchanged.
|
||||
pub original: TokenStream,
|
||||
/// All attributes except `#[inline]` ones. They go on the outer function.
|
||||
pub outer_attrs: Vec<Attribute>,
|
||||
/// `#[inline]` attributes. They go on the functions which have `#[target_feature]`.
|
||||
pub inline_attrs: Vec<Attribute>,
|
||||
/// The attributes, sorted by which of the generated functions they go on.
|
||||
pub attrs: Attrs,
|
||||
/// Visibility, e.g. `pub(crate)`. Empty if the function is private.
|
||||
pub vis: TokenStream,
|
||||
/// `const`, if the function is a `const fn`.
|
||||
@@ -50,7 +48,8 @@ pub struct Func {
|
||||
|
||||
impl Func {
|
||||
/// Parse a function. Error if it isn't one, or it's a kind the macros don't support.
|
||||
pub fn parse(original: TokenStream) -> Result<Func> {
|
||||
/// `mac` is the macro expanding it.
|
||||
pub fn parse(original: TokenStream, mac: Macro) -> Result<Func> {
|
||||
let FuncParts { attrs, vis, sig, body } = parse2(original.clone())?;
|
||||
|
||||
reject(sig.asyncness, "`async` functions")?;
|
||||
@@ -65,8 +64,7 @@ impl Func {
|
||||
}
|
||||
};
|
||||
|
||||
let (inline_attrs, outer_attrs) =
|
||||
attrs.into_iter().partition(|attr| attr.path().is_ident("inline"));
|
||||
let attrs = Attrs::new(attrs, mac)?;
|
||||
|
||||
let mut receiver = None;
|
||||
let mut params = Vec::new();
|
||||
@@ -80,8 +78,7 @@ impl Func {
|
||||
|
||||
Ok(Func {
|
||||
original,
|
||||
outer_attrs,
|
||||
inline_attrs,
|
||||
attrs,
|
||||
vis: call_site(&vis),
|
||||
constness: sig.constness.map(|_| <Token![const]>::default()),
|
||||
unsafety,
|
||||
@@ -97,9 +94,10 @@ impl Func {
|
||||
|
||||
/// `true` if one of the function's `#[inline]` attributes is `#[inline(always)]`.
|
||||
pub fn is_inline_always(&self) -> bool {
|
||||
self.inline_attrs
|
||||
.iter()
|
||||
.any(|attr| attr.parse_args::<Ident>().is_ok_and(|arg| arg == "always"))
|
||||
self.attrs.entry_points.iter().any(|attr| {
|
||||
attr.path().is_ident("inline")
|
||||
&& attr.parse_args::<Ident>().is_ok_and(|arg| arg == "always")
|
||||
})
|
||||
}
|
||||
|
||||
/// Name for a function which a macro generates for this one, e.g. `__impl`.
|
||||
@@ -166,6 +164,67 @@ impl Func {
|
||||
}
|
||||
}
|
||||
|
||||
/// The macro expanding the function. `#[track_caller]` goes on different functions for each.
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
pub enum Macro {
|
||||
/// `#[cpu_multiversion]`. Its entry points call `__impl`.
|
||||
Multiversion,
|
||||
/// `#[cpu_version]`. `__impl` is its entry point.
|
||||
Version,
|
||||
}
|
||||
|
||||
/// A function's attributes, separated by which of the generated functions they go on.
|
||||
///
|
||||
/// `#[track_caller]` goes on all of them, so the caller's location passes through each.
|
||||
///
|
||||
/// `#[cfg]` and `#[cfg_attr]` never reach the macros. rustc evaluates them before it runs
|
||||
/// an attribute macro, wherever they are in the list.
|
||||
///
|
||||
/// `#[target_feature]` is an error, because the macros add it.
|
||||
pub struct Attrs {
|
||||
/// On the outer function, which replaces the original: doc comments, `#[must_use]`,
|
||||
/// and anything not listed below.
|
||||
pub outer: Vec<Attribute>,
|
||||
/// On the entry points, the functions with `#[target_feature]` which get called:
|
||||
/// `#[inline]` and `#[cold]`.
|
||||
pub entry_points: Vec<Attribute>,
|
||||
/// On `__impl`, the function with the body: lint attributes, as that's where the lints fire.
|
||||
/// A lint attribute on a method wouldn't cover its sibling `__foo_impl`.
|
||||
pub impl_fn: Vec<Attribute>,
|
||||
}
|
||||
|
||||
impl Attrs {
|
||||
/// Separate attributes by which of the generated functions they go on.
|
||||
/// Error for attributes the macros don't support. `mac` is the macro expanding the function.
|
||||
fn new(attrs: Vec<Attribute>, mac: Macro) -> Result<Attrs> {
|
||||
let mut sorted = Attrs { outer: Vec::new(), entry_points: Vec::new(), impl_fn: Vec::new() };
|
||||
|
||||
for attr in attrs {
|
||||
// Comparing a path to a `&str` allocates a `String` each time, so convert it once.
|
||||
// `None` for a path with `::` in it, e.g. `rustfmt::skip`.
|
||||
let name = attr.path().get_ident().map(Ident::to_string);
|
||||
match name.as_deref() {
|
||||
Some("inline" | "cold") => sorted.entry_points.push(attr),
|
||||
Some("expect" | "allow" | "warn" | "deny" | "forbid") => sorted.impl_fn.push(attr),
|
||||
Some("track_caller") => {
|
||||
// `cpu_version`'s `__impl` is an entry point, and gets it from `impl_fn`
|
||||
sorted.outer.push(attr.clone());
|
||||
if mac == Macro::Multiversion {
|
||||
sorted.entry_points.push(attr.clone());
|
||||
}
|
||||
sorted.impl_fn.push(attr);
|
||||
}
|
||||
Some("target_feature") => {
|
||||
return Err(Error::new_spanned(attr, "`#[target_feature]` isn't supported"));
|
||||
}
|
||||
_ => sorted.outer.push(attr),
|
||||
}
|
||||
}
|
||||
|
||||
Ok(sorted)
|
||||
}
|
||||
}
|
||||
|
||||
/// A parameter, `name: Type` or `mut name: Type`.
|
||||
pub struct Param {
|
||||
/// `mut`, if the parameter is `mut name: Type`.
|
||||
|
||||
@@ -77,8 +77,16 @@ mod version;
|
||||
/// so `#[cfg]` (or `cpu_select!`, which uses it) would grey out the function's signature.
|
||||
/// Branches of `cfg_select!` are just dropped, so nothing is greyed out.
|
||||
///
|
||||
/// `#[inline]` and `#[inline(never)]` attributes go on the entry points.
|
||||
/// All other attributes (e.g. doc comments) go on `compress`.
|
||||
/// Attributes go on the generated function where they take effect:
|
||||
///
|
||||
/// - `#[inline]` and `#[cold]` go on the entry points. They're the functions which get called.
|
||||
/// - Lint attributes (`#[expect]`, `#[allow]`, etc.) go on `__impl`, where the lints fire.
|
||||
/// On a method, they wouldn't cover its sibling `__pipeline_impl`.
|
||||
/// - `#[track_caller]` goes on all of them, so the caller's location passes through.
|
||||
/// - All other attributes (doc comments, `#[must_use]`, etc.) go on `compress`.
|
||||
///
|
||||
/// `#[cfg]` and `#[cfg_attr]` are evaluated by rustc before the macro runs, wherever they are
|
||||
/// in the list. `#[target_feature]` is an error, because the macro adds it.
|
||||
///
|
||||
/// `#[inline(never)]` keeps the entry points as separate functions.
|
||||
/// Without it, they can be inlined into their callers in builds which enable AVX2 globally.
|
||||
@@ -286,8 +294,9 @@ pub fn cpu_multiversion(args: TokenStream, item: TokenStream) -> TokenStream {
|
||||
/// that a parameter's type `Avx2` is really `crate::cpu::Avx2`. It might be a type alias or
|
||||
/// a generic parameter with that name. The `let _` fails to compile if so, and generates no code.
|
||||
///
|
||||
/// Like [`cpu_multiversion`] `#[inline]` and `#[inline(never)]` attributes go on `__impl`,
|
||||
/// all other attributes go on `foo`, and `mut` only goes on `__impl`'s parameters.
|
||||
/// Attributes go where [`cpu_multiversion`] puts them. `__impl` is the entry point here, so it
|
||||
/// gets `#[inline]`, `#[cold]` and lint attributes. All other attributes go on `foo`, and `mut`
|
||||
/// only goes on `__impl`'s parameters.
|
||||
///
|
||||
/// A nested function can't use `self` or `Self`, so for a method, or an associated function
|
||||
/// which uses `Self`, `__impl` is a private method next to it, called `__foo_impl`. So it can't
|
||||
|
||||
@@ -20,7 +20,7 @@ use syn::{Error, Result};
|
||||
|
||||
use crate::{
|
||||
cpu::Cpu,
|
||||
func::{Func, Param, is_impl_cpu},
|
||||
func::{Func, Macro, Param, is_impl_cpu},
|
||||
};
|
||||
|
||||
/// The function, with its `cpu: impl Cpu` parameter split off.
|
||||
@@ -40,7 +40,7 @@ pub fn expand(args: TokenStream, item: TokenStream) -> Result<TokenStream> {
|
||||
return Err(Error::new_spanned(args, "`cpu_multiversion` takes no arguments"));
|
||||
}
|
||||
|
||||
let mut func = Func::parse(item)?;
|
||||
let mut func = Func::parse(item, Macro::Multiversion)?;
|
||||
let Some(cpu_param) = func.params.pop_if(|param| is_impl_cpu(¶m.ty)) else {
|
||||
return Err(Error::new_spanned(&func.name, "the last parameter must be `cpu: impl Cpu`"));
|
||||
};
|
||||
@@ -107,7 +107,7 @@ impl Multiversion<'_> {
|
||||
/// }
|
||||
/// ```
|
||||
fn expand_fn(&self) -> TokenStream {
|
||||
let outer_attrs = &*self.func.outer_attrs;
|
||||
let outer_attrs = &*self.func.attrs.outer;
|
||||
let signature = self.signature();
|
||||
let impl_fn = self.impl_fn();
|
||||
let generic_fn = self.entry_point(Cpu::Generic);
|
||||
@@ -198,7 +198,7 @@ impl Multiversion<'_> {
|
||||
/// }
|
||||
/// ```
|
||||
fn expand_method(&self) -> TokenStream {
|
||||
let outer_attrs = &*self.func.outer_attrs;
|
||||
let outer_attrs = &*self.func.attrs.outer;
|
||||
let signature = self.signature();
|
||||
let cpu_match = cfg_select(|simd| self.cpu_match(simd));
|
||||
let impl_fn = self.impl_fn();
|
||||
@@ -238,6 +238,7 @@ impl Multiversion<'_> {
|
||||
}
|
||||
|
||||
/// `__impl`: the original function, which is compiled as part of each entry point.
|
||||
/// Lint attributes go on it, as that's where the lints fire.
|
||||
///
|
||||
/// ```ignore
|
||||
/// #[inline(always)]
|
||||
@@ -246,6 +247,7 @@ impl Multiversion<'_> {
|
||||
/// }
|
||||
/// ```
|
||||
fn impl_fn(&self) -> TokenStream {
|
||||
let impl_attrs = &*self.func.attrs.impl_fn;
|
||||
let Func { constness, unsafety, generics, return_type, body, .. } = self.func;
|
||||
let name = self.func.generated_name("impl");
|
||||
let params = self.func.params_with_mut();
|
||||
@@ -253,6 +255,7 @@ impl Multiversion<'_> {
|
||||
let where_clause = &generics.where_clause;
|
||||
|
||||
quote! {
|
||||
#(#impl_attrs)*
|
||||
#[inline(always)]
|
||||
#constness #unsafety fn #name #generics (#params #cpu_name: #cpu_ty) #return_type
|
||||
#where_clause
|
||||
@@ -261,6 +264,7 @@ impl Multiversion<'_> {
|
||||
}
|
||||
|
||||
/// An entry point: calls `__impl`, compiled with one feature set's target features.
|
||||
/// `#[inline]` and `#[cold]` go on it, as it's the function which gets called.
|
||||
///
|
||||
/// ```ignore
|
||||
/// #[inline(never)]
|
||||
@@ -270,7 +274,8 @@ impl Multiversion<'_> {
|
||||
/// }
|
||||
/// ```
|
||||
fn entry_point(&self, cpu: Cpu) -> TokenStream {
|
||||
let Func { inline_attrs, constness, unsafety, generics, return_type, .. } = self.func;
|
||||
let entry_attrs = &*self.func.attrs.entry_points;
|
||||
let Func { constness, unsafety, generics, return_type, .. } = self.func;
|
||||
let target_feature_attr = cpu.target_feature_attr();
|
||||
let name = self.func.generated_name(cpu.lower_name());
|
||||
let params = self.func.params();
|
||||
@@ -280,7 +285,7 @@ impl Multiversion<'_> {
|
||||
let call = self.call("impl");
|
||||
|
||||
quote! {
|
||||
#(#inline_attrs)*
|
||||
#(#entry_attrs)*
|
||||
#target_feature_attr
|
||||
#constness #unsafety fn #name #generics (#params #cpu_name: #cpu_ty) #return_type
|
||||
#where_clause
|
||||
|
||||
@@ -20,7 +20,7 @@ use syn::{Error, GenericArgument, Ident, PathArguments, Result, Type, parse2, sp
|
||||
|
||||
use crate::{
|
||||
cpu::Cpu,
|
||||
func::{Func, is_impl_cpu, respan},
|
||||
func::{Attrs, Func, Macro, is_impl_cpu, respan},
|
||||
};
|
||||
|
||||
/// A parameter which proves that the CPU supports a feature set.
|
||||
@@ -40,7 +40,7 @@ struct Proof {
|
||||
/// `args` is the attribute's argument: `Avx2`, `Neon` or `Generic`. `item` is the function.
|
||||
pub fn expand(args: TokenStream, item: TokenStream) -> Result<TokenStream> {
|
||||
let cpu = parse_cpu(args)?;
|
||||
let func = Func::parse(item)?;
|
||||
let func = Func::parse(item, Macro::Version)?;
|
||||
|
||||
if cpu == Cpu::Generic {
|
||||
check_no_cpu_params(&func)?;
|
||||
@@ -88,9 +88,8 @@ pub fn expand(args: TokenStream, item: TokenStream) -> Result<TokenStream> {
|
||||
/// A nested function can't use `self` or `Self`, so a method's `__impl` is a method `__foo_impl`
|
||||
/// next to it, with the same `#[cfg]`, and the call is `Self::__foo_impl::<N, T>(self, arr, cpu)`.
|
||||
fn wrap(func: &Func, cpu: Cpu, proof: &Proof) -> TokenStream {
|
||||
let Func {
|
||||
outer_attrs, inline_attrs, vis, constness, name, generics, return_type, body, ..
|
||||
} = func;
|
||||
let Attrs { outer: outer_attrs, entry_points: entry_attrs, impl_fn: impl_attrs } = &func.attrs;
|
||||
let Func { vis, constness, name, generics, return_type, body, .. } = func;
|
||||
let cfg_attr = cpu.cfg_attr();
|
||||
|
||||
let params = func.params();
|
||||
@@ -104,7 +103,8 @@ fn wrap(func: &Func, cpu: Cpu, proof: &Proof) -> TokenStream {
|
||||
let impl_name = func.generated_name("impl");
|
||||
let params_with_mut = func.params_with_mut();
|
||||
let impl_fn = quote! {
|
||||
#(#inline_attrs)*
|
||||
#(#impl_attrs)*
|
||||
#(#entry_attrs)*
|
||||
#target_feature_attr
|
||||
#constness fn #impl_name #generics (#params_with_mut) #return_type
|
||||
#where_clause
|
||||
|
||||
Reference in new issue
Block a user