Skip to content
This repository was archived by the owner on Apr 28, 2025. It is now read-only.

Commit 8abd2f9

Browse files
gnzlbggnzlbg
committed
Nitpicks
1 parent 5e85499 commit 8abd2f9

File tree

3 files changed

+151
-148
lines changed

3 files changed

+151
-148
lines changed

‎ci/run.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,3 @@ if [ "$TARGET" = "x86_64-unknown-linux-gnu" ]; then
2727
$CMD_ --target $TARGET --features 'stable checked system_libm'
2828
$CMD_ --target $TARGET --release --features 'stable checked system_libm'
2929
fi
30-

‎crates/libm-analyze/src/lib.rs

Lines changed: 147 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
//! A proc macro to analyze the libm APIs We want to exhaustively match all
2+
// fields here to create a compilation error if new fields are added.
3+
#![allow(clippy::unneeded_field_pattern)]
4+
15
extern crate proc_macro;
26
use self::proc_macro::TokenStream;
37
use quote::quote;
@@ -10,7 +14,7 @@ use syn::parse_macro_input;
1014
#[proc_macro]
1115
pub fn for_each_api(input: TokenStream) -> TokenStream {
1216
let files = get_libm_files();
13-
let functions = get_functions(files);
17+
let functions = get_functions(&files);
1418
let input = parse_macro_input!(input as syn::Ident);
1519
let mut tokens = proc_macro2::TokenStream::new();
1620
for function in functions {
@@ -45,7 +49,7 @@ fn get_libm_files() -> Vec<syn::File> {
4549
let mut files = Vec::new();
4650
for entry in walkdir::WalkDir::new(libm_src_dir)
4751
.into_iter()
48-
.filter_map(|e| e.ok())
52+
.filter_map(Result::ok)
4953
{
5054
use std::io::Read;
5155
let file_path = entry.path();
@@ -97,160 +101,158 @@ macro_rules! syn_to_str {
97101

98102
/// Extracts all public functions from the libm files while
99103
/// doing some sanity checks on the function signatures.
100-
fn get_functions(files: Vec<syn::File>) -> Vec<FnSig> {
104+
fn get_functions(files: &[syn::File]) -> Vec<FnSig> {
101105
let mut error = false;
102106
let mut functions = Vec::new();
103107
// Traverse all files matching function items
104108
for item in files.iter().flat_map(|f| f.items.iter()) {
105109
let mut e = false;
106-
match item {
107-
syn::Item::Fn(syn::ItemFn {
108-
vis: syn::Visibility::Public(_),
109-
ident,
110-
constness,
111-
asyncness,
112-
unsafety,
113-
attrs,
114-
abi,
115-
decl,
116-
block: _,
117-
}) => {
118-
// Build a function signature while doing some sanity checks
119-
let mut fn_sig = FnSig {
120-
ident: ident.clone(),
121-
c_abi: false,
122-
arg_tys: Vec::new(),
123-
ret_ty: None,
124-
};
125-
macro_rules! err {
126-
($msg:expr) => {{
127-
#[cfg(feature = "analyze")]
128-
{
129-
eprintln!("[error]: Function \"{}\" {}", fn_sig.name(), $msg);
130-
}
131-
#[allow(unused_assignments)]
132-
{
133-
e = true;
134-
}
135-
()
136-
}};
137-
}
138-
if let Some(syn::Abi {
139-
name: Some(l),
140-
extern_token: _,
141-
}) = abi
142-
{
143-
if l.value() == "C" {
144-
fn_sig.c_abi = true;
110+
if let syn::Item::Fn(syn::ItemFn {
111+
vis: syn::Visibility::Public(_),
112+
ident,
113+
constness,
114+
asyncness,
115+
unsafety,
116+
attrs,
117+
abi,
118+
decl,
119+
block: _,
120+
}) = item
121+
{
122+
// Build a function signature while doing some sanity checks
123+
let mut fn_sig = FnSig {
124+
ident: ident.clone(),
125+
c_abi: false,
126+
arg_tys: Vec::new(),
127+
ret_ty: None,
128+
};
129+
macro_rules! err {
130+
($msg:expr) => {{
131+
#[cfg(feature = "analyze")]
132+
{
133+
eprintln!("[error]: Function \"{}\" {}", fn_sig.name(), $msg);
145134
}
146-
}
147-
// If the function signature isn't extern "C", we aren't ABI compatible
148-
// with libm.
149-
if !fn_sig.c_abi {
150-
// FIXME: we should error here, but right that would break everything,
151-
// so we disable erroring.
152-
let e2 = e;
153-
err!("not `extern \"C\"`");
154-
e = e2;
155-
}
156-
// Right now there are no const fn functions. We might add them
157-
// in the future, and at that point, we should tune this here.
158-
// In the mean time, error if somebody tries.
159-
if let Some(_) = constness {
160-
err!("is const");
161-
}
162-
// No functions should be async fn
163-
if let Some(_) = asyncness {
164-
err!("is async");
165-
}
166-
// FIXME: Math functions are not unsafe. Some functions in the
167-
// libm C API take pointers, but in our API take repr(Rust)
168-
// tuples (for some reason). Once we fix those to have the same
169-
// API as C libm, we should use references on their signature
170-
// instead, and make them safe.
171-
if let Some(_) = unsafety {
172-
let e2 = e;
173-
err!("is unsafe");
174-
e = e2;
175-
}
176-
let syn::FnDecl {
177-
fn_token: _,
178-
generics,
179-
paren_token: _,
180-
inputs,
181-
variadic,
182-
output,
183-
} = (**decl).clone();
184-
185-
// Forbid generic parameters, lifetimes, and consts in public APIs:
186-
if variadic.is_some() {
187-
err!(format!(
188-
"contains variadic arguments \"{}\"",
189-
syn_to_str!(variadic.unwrap())
190-
));
191-
}
192-
if generics.type_params().into_iter().count() != 0 {
193-
err!(format!(
194-
"contains generic parameters \"{}\"",
195-
syn_to_str!(generics.clone())
196-
));
197-
}
198-
if generics.lifetimes().into_iter().count() != 0 {
199-
err!(format!(
200-
"contains lifetime parameters \"{}\"",
201-
syn_to_str!(generics.clone())
202-
));
203-
}
204-
if generics.const_params().into_iter().count() != 0 {
205-
err!(format!(
206-
"contains const parameters \"{}\"",
207-
syn_to_str!(generics.clone())
208-
));
209-
}
210-
// FIXME: we can do better here, but right now, we should
211-
// error if inline and no_panic are not used, which is the
212-
// case if the public API has no attributes.
213-
//
214-
// We might also want to check other attributes as well.
215-
if attrs.is_empty() {
216-
let e2 = e;
217-
err!(format!(
218-
"missing `#[inline]` and `#[no_panic]` attributes {}",
219-
attrs
220-
.iter()
221-
.map(|a| syn_to_str!(a))
222-
.collect::<Vec<_>>()
223-
.join(",")
224-
));
225-
e = e2;
226-
}
227-
// Validate and parse output parameters and function arguments:
228-
match output {
229-
syn::ReturnType::Default => (),
230-
syn::ReturnType::Type(_, ref b) if valid_ty(&b) => {
231-
fn_sig.ret_ty = Some(*b.clone())
135+
#[allow(unused_assignments)]
136+
{
137+
e = true;
232138
}
233-
other => err!(format!("returns unsupported type {}", syn_to_str!(other))),
139+
()
140+
}};
141+
}
142+
if let Some(syn::Abi {
143+
name: Some(l),
144+
extern_token: _,
145+
}) = abi
146+
{
147+
if l.value() == "C" {
148+
fn_sig.c_abi = true;
234149
}
235-
for input in inputs {
236-
match input {
237-
syn::FnArg::Captured(ref c) if valid_ty(&c.ty) => {
238-
fn_sig.arg_tys.push(c.ty.clone())
239-
}
240-
other => err!(format!(
241-
"takes unsupported argument type {}",
242-
syn_to_str!(other)
243-
)),
150+
}
151+
// If the function signature isn't extern "C", we aren't ABI compatible
152+
// with libm.
153+
if !fn_sig.c_abi {
154+
// FIXME: we should error here, but right that would break everything,
155+
// so we disable erroring.
156+
let e2 = e;
157+
err!("not `extern \"C\"`");
158+
e = e2;
159+
}
160+
// Right now there are no const fn functions. We might add them
161+
// in the future, and at that point, we should tune this here.
162+
// In the mean time, error if somebody tries.
163+
if constness.is_some() {
164+
err!("is const");
165+
}
166+
// No functions should be async fn
167+
if asyncness.is_some() {
168+
err!("is async");
169+
}
170+
// FIXME: Math functions are not unsafe. Some functions in the
171+
// libm C API take pointers, but in our API take repr(Rust)
172+
// tuples (for some reason). Once we fix those to have the same
173+
// API as C libm, we should use references on their signature
174+
// instead, and make them safe.
175+
if unsafety.is_some() {
176+
let e2 = e;
177+
err!("is unsafe");
178+
e = e2;
179+
}
180+
let syn::FnDecl {
181+
fn_token: _,
182+
generics,
183+
paren_token: _,
184+
inputs,
185+
variadic,
186+
output,
187+
} = (**decl).clone();
188+
189+
// Forbid generic parameters, lifetimes, and consts in public APIs:
190+
if variadic.is_some() {
191+
err!(format!(
192+
"contains variadic arguments \"{}\"",
193+
syn_to_str!(variadic.unwrap())
194+
));
195+
}
196+
if generics.type_params().count() != 0 {
197+
err!(format!(
198+
"contains generic parameters \"{}\"",
199+
syn_to_str!(generics.clone())
200+
));
201+
}
202+
if generics.lifetimes().count() != 0 {
203+
err!(format!(
204+
"contains lifetime parameters \"{}\"",
205+
syn_to_str!(generics.clone())
206+
));
207+
}
208+
if generics.const_params().count() != 0 {
209+
err!(format!(
210+
"contains const parameters \"{}\"",
211+
syn_to_str!(generics.clone())
212+
));
213+
}
214+
// FIXME: we can do better here, but right now, we should
215+
// error if inline and no_panic are not used, which is the
216+
// case if the public API has no attributes.
217+
//
218+
// We might also want to check other attributes as well.
219+
if attrs.is_empty() {
220+
let e2 = e;
221+
err!(format!(
222+
"missing `#[inline]` and `#[no_panic]` attributes {}",
223+
attrs
224+
.iter()
225+
.map(|a| syn_to_str!(a))
226+
.collect::<Vec<_>>()
227+
.join(",")
228+
));
229+
e = e2;
230+
}
231+
// Validate and parse output parameters and function arguments:
232+
match output {
233+
syn::ReturnType::Default => (),
234+
syn::ReturnType::Type(_, ref b) if valid_ty(&b) => fn_sig.ret_ty = Some(*b.clone()),
235+
other => err!(format!("returns unsupported type {}", syn_to_str!(other))),
236+
}
237+
for input in inputs {
238+
match input {
239+
syn::FnArg::Captured(ref c) if valid_ty(&c.ty) => {
240+
fn_sig.arg_tys.push(c.ty.clone())
244241
}
242+
other => err!(format!(
243+
"takes unsupported argument type {}",
244+
syn_to_str!(other)
245+
)),
245246
}
246-
// If there was an error, we skip the function:
247-
if !e {
248-
functions.push(fn_sig);
249-
} else {
250-
error = true;
251-
}
252247
}
253-
_ => (),
248+
// If there was an error, we skip the function.
249+
// Otherwise, the user macro is expanded with
250+
// the function:
251+
if e {
252+
error = true;
253+
} else {
254+
functions.push(fn_sig);
255+
}
254256
}
255257
}
256258
if error {

‎crates/libm-test/src/lib.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ pub trait WithinUlps {
1212
macro_rules! impl_within_ulps_f {
1313
($f_ty:ty, $i_ty:ty) => {
1414
impl WithinUlps for $f_ty {
15-
fn within_ulps(self, y: $f_ty, ulp_tol: usize) -> bool {
15+
#[allow(clippy::cast_possible_truncation)]
16+
fn within_ulps(self, y: Self, ulp_tol: usize) -> bool {
1617
let x = self;
1718
if x.is_nan() != y.is_nan() {
1819
// one is nan but the other is not
@@ -43,7 +44,8 @@ impl_within_ulps_f!(f32, i32);
4344
impl_within_ulps_f!(f64, i64);
4445

4546
impl WithinUlps for i32 {
46-
fn within_ulps(self, y: i32, ulp_tol: usize) -> bool {
47+
#[allow(clippy::cast_possible_truncation, clippy::cast_possible_wrap)]
48+
fn within_ulps(self, y: Self, ulp_tol: usize) -> bool {
4749
let x = self;
4850
let ulps = (x - y).abs();
4951
ulps <= ulp_tol as _

0 commit comments

Comments
 (0)