From eee632ee1b75f2fcc6ee679d340169a7386a0604 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 5 Feb 2025 15:03:34 +0000 Subject: [PATCH] Add checks via annotation that lists are sorted or exhaustive This crate has a handful of lists that need to list all API and can't easily be verified. Additionally, some longer lists should be kept sorted so they are easier to look through. Resolve both of these by adding a check in `update-api-list.py` that looks for annotations and verifies the contents are as expected. Annotations are `verify-apilist-start`, `verify-apilist-end`, `verify-sorted-start`, and `verify-sorted-end`. This includes fixes for anything that did not meet the criteria. --- .../libm/crates/libm-test/benches/icount.rs | 9 +- .../libm/crates/libm-test/src/mpfloat.rs | 2 + .../libm-test/tests/compare_built_musl.rs | 2 + .../libm/etc/update-api-list.py | 143 ++++++++++++++++-- .../compiler-builtins/libm/src/libm_helper.rs | 44 +++++- .../compiler-builtins/libm/src/math/mod.rs | 8 + 6 files changed, 187 insertions(+), 21 deletions(-) diff --git a/library/compiler-builtins/libm/crates/libm-test/benches/icount.rs b/library/compiler-builtins/libm/crates/libm-test/benches/icount.rs index 13de799c77f7..53ecb5a37c4b 100644 --- a/library/compiler-builtins/libm/crates/libm-test/benches/icount.rs +++ b/library/compiler-builtins/libm/crates/libm-test/benches/icount.rs @@ -52,7 +52,10 @@ libm_macros::for_each_function! { } main!( - library_benchmark_groups = icount_bench_acos_group, + library_benchmark_groups = + // verify-apilist-start + // verify-sorted-start + icount_bench_acos_group, icount_bench_acosf_group, icount_bench_acosh_group, icount_bench_acoshf_group, @@ -169,6 +172,8 @@ main!( icount_bench_scalbnf16_group, icount_bench_scalbnf_group, icount_bench_sin_group, + icount_bench_sincos_group, + icount_bench_sincosf_group, icount_bench_sinf_group, icount_bench_sinh_group, icount_bench_sinhf_group, @@ -192,4 +197,6 @@ main!( icount_bench_y1f_group, icount_bench_yn_group, icount_bench_ynf_group, + // verify-sorted-end + // verify-apilist-end ); diff --git a/library/compiler-builtins/libm/crates/libm-test/src/mpfloat.rs b/library/compiler-builtins/libm/crates/libm-test/src/mpfloat.rs index e3211b913794..ab77d541c812 100644 --- a/library/compiler-builtins/libm/crates/libm-test/src/mpfloat.rs +++ b/library/compiler-builtins/libm/crates/libm-test/src/mpfloat.rs @@ -132,6 +132,7 @@ libm_macros::for_each_function! { emit_types: [RustFn], skip: [ // Most of these need a manual implementation + // verify-sorted-start ceil, ceilf, ceilf128, @@ -188,6 +189,7 @@ libm_macros::for_each_function! { truncf128, truncf16,yn, ynf, + // verify-sorted-end ], fn_extra: match MACRO_FN_NAME { // Remap function names that are different between mpfr and libm diff --git a/library/compiler-builtins/libm/crates/libm-test/tests/compare_built_musl.rs b/library/compiler-builtins/libm/crates/libm-test/tests/compare_built_musl.rs index 191c7e69de39..0b0a9f097216 100644 --- a/library/compiler-builtins/libm/crates/libm-test/tests/compare_built_musl.rs +++ b/library/compiler-builtins/libm/crates/libm-test/tests/compare_built_musl.rs @@ -79,6 +79,7 @@ libm_macros::for_each_function! { ynf, // Not provided by musl + // verify-sorted-start ceilf128, ceilf16, copysignf128, @@ -107,5 +108,6 @@ libm_macros::for_each_function! { sqrtf16, truncf128, truncf16, + // verify-sorted-end ], } diff --git a/library/compiler-builtins/libm/etc/update-api-list.py b/library/compiler-builtins/libm/etc/update-api-list.py index 54da13257958..9cf625554223 100755 --- a/library/compiler-builtins/libm/etc/update-api-list.py +++ b/library/compiler-builtins/libm/etc/update-api-list.py @@ -8,16 +8,21 @@ needed, or that lists are sorted. import difflib import json +import re import subprocess as sp import sys from dataclasses import dataclass -from glob import glob +from glob import glob, iglob from pathlib import Path -from typing import Any, TypeAlias +from typing import Any, Callable, TypeAlias -ETC_DIR = Path(__file__).parent +SELF_PATH = Path(__file__) +ETC_DIR = SELF_PATH.parent ROOT_DIR = ETC_DIR.parent +# Loose approximation of what gets checked in to git, without needing `git ls-files`. +DIRECTORIES = [".github", "ci", "crates", "etc", "src"] + # These files do not trigger a retest. IGNORED_SOURCES = ["src/libm_helper.rs"] @@ -25,6 +30,11 @@ IndexTy: TypeAlias = dict[str, dict[str, Any]] """Type of the `index` item in rustdoc's JSON output""" +def eprint(*args, **kwargs): + """Print to stderr.""" + print(*args, file=sys.stderr, **kwargs) + + @dataclass class Crate: """Representation of public interfaces and function defintion locations in @@ -146,7 +156,7 @@ class Crate: if check: with open(out_file, "r") as f: current = f.read() - diff_and_exit(current, output) + diff_and_exit(current, output, "function list") else: with open(out_file, "w") as f: f.write(output) @@ -171,18 +181,115 @@ class Crate: if check: with open(out_file, "r") as f: current = f.read() - diff_and_exit(current, output) + diff_and_exit(current, output, "source list") else: with open(out_file, "w") as f: f.write(output) + def tidy_lists(self) -> None: + """In each file, check annotations indicating blocks of code should be sorted or should + include all public API. + """ + for dirname in DIRECTORIES: + dir = ROOT_DIR.joinpath(dirname) + for fname in iglob("**", root_dir=dir, recursive=True): + fpath = dir.joinpath(fname) + if fpath.is_dir() or fpath == SELF_PATH: + continue -def diff_and_exit(actual: str, expected: str): + lines = fpath.read_text().splitlines() + + validate_delimited_block( + fpath, + lines, + "verify-sorted-start", + "verify-sorted-end", + ensure_sorted, + ) + + validate_delimited_block( + fpath, + lines, + "verify-apilist-start", + "verify-apilist-end", + lambda p, n, lines: self.ensure_contains_api(p, n, lines), + ) + + def ensure_contains_api(self, fpath: Path, line_num: int, lines: list[str]): + """Given a list of strings, ensure that each public function we have is named + somewhere. + """ + not_found = [] + for func in self.public_functions: + # The function name may be on its own or somewhere in a snake case string. + pat = re.compile(rf"(\b|_){func}(\b|_)") + found = next((line for line in lines if pat.search(line)), None) + + if found is None: + not_found.append(func) + + if len(not_found) == 0: + return + + relpath = fpath.relative_to(ROOT_DIR) + eprint(f"functions not found at {relpath}:{line_num}: {not_found}") + exit(1) + + +def validate_delimited_block( + fpath: Path, + lines: list[str], + start: str, + end: str, + validate: Callable[[Path, int, list[str]], None], +) -> None: + """Identify blocks of code wrapped within `start` and `end`, collect their contents + to a list of strings, and call `validate` for each of those lists. + """ + relpath = fpath.relative_to(ROOT_DIR) + block_lines = [] + block_start_line: None | int = None + for line_num, line in enumerate(lines): + line_num += 1 + + if start in line: + block_start_line = line_num + continue + + if end in line: + if block_start_line is None: + eprint(f"`{end}` without `{start}` at {relpath}:{line_num}") + exit(1) + + validate(fpath, block_start_line, block_lines) + block_lines = [] + block_start_line = None + continue + + if block_start_line is not None: + block_lines.append(line) + + if block_start_line is not None: + eprint(f"`{start}` without `{end}` at {relpath}:{block_start_line}") + exit(1) + + +def ensure_sorted(fpath: Path, block_start_line: int, lines: list[str]) -> None: + """Ensure that a list of lines is sorted, otherwise print a diff and exit.""" + relpath = fpath.relative_to(ROOT_DIR) + diff_and_exit( + "".join(lines), + "".join(sorted(lines)), + f"sorted block at {relpath}:{block_start_line}", + ) + + +def diff_and_exit(actual: str, expected: str, name: str): """If the two strings are different, print a diff between them and then exit with an error. """ if actual == expected: - print("output matches expected; success") + print(f"{name} output matches expected; success") return a = [f"{line}\n" for line in actual.splitlines()] @@ -190,7 +297,7 @@ def diff_and_exit(actual: str, expected: str): diff = difflib.unified_diff(a, b, "actual", "expected") sys.stdout.writelines(diff) - print("mismatched function list") + print(f"mismatched {name}") exit(1) @@ -223,23 +330,31 @@ def base_name(name: str) -> tuple[str, str]: return (name, "f64") +def ensure_updated_list(check: bool) -> None: + """Runner to update the function list and JSON, or check that it is already up + to date. + """ + crate = Crate() + crate.write_function_list(check) + crate.write_function_defs(check) + + if check: + crate.tidy_lists() + + def main(): """By default overwrite the file. If `--check` is passed, print a diff instead and error if the files are different. """ match sys.argv: case [_]: - check = False + ensure_updated_list(False) case [_, "--check"]: - check = True + ensure_updated_list(True) case _: print("unrecognized arguments") exit(1) - crate = Crate() - crate.write_function_list(check) - crate.write_function_defs(check) - if __name__ == "__main__": main() diff --git a/library/compiler-builtins/libm/src/libm_helper.rs b/library/compiler-builtins/libm/src/libm_helper.rs index 73bae456761b..0768839c7799 100644 --- a/library/compiler-builtins/libm/src/libm_helper.rs +++ b/library/compiler-builtins/libm/src/libm_helper.rs @@ -44,9 +44,11 @@ macro_rules! libm_helper { }; } +// verify-apilist-start libm_helper! { f32, funcs: { + // verify-sorted-start (fn acos(x: f32) -> (f32); => acosf); (fn acosh(x: f32) -> (f32); => acoshf); (fn asin(x: f32) -> (f32); => asinf); @@ -62,8 +64,8 @@ libm_helper! { (fn erf(x: f32) -> (f32); => erff); (fn erfc(x: f32) -> (f32); => erfcf); (fn exp(x: f32) -> (f32); => expf); - (fn exp2(x: f32) -> (f32); => exp2f); (fn exp10(x: f32) -> (f32); => exp10f); + (fn exp2(x: f32) -> (f32); => exp2f); (fn expm1(x: f32) -> (f32); => expm1f); (fn fabs(x: f32) -> (f32); => fabsf); (fn fdim(x: f32, y: f32) -> (f32); => fdimf); @@ -79,12 +81,12 @@ libm_helper! { (fn j1(x: f32) -> (f32); => j1f); (fn jn(n: i32, x: f32) -> (f32); => jnf); (fn ldexp(x: f32, n: i32) -> (f32); => ldexpf); - (fn lgamma_r(x: f32) -> (f32, i32); => lgammaf_r); (fn lgamma(x: f32) -> (f32); => lgammaf); + (fn lgamma_r(x: f32) -> (f32, i32); => lgammaf_r); (fn log(x: f32) -> (f32); => logf); + (fn log10(x: f32) -> (f32); => log10f); (fn log1p(x: f32) -> (f32); => log1pf); (fn log2(x: f32) -> (f32); => log2f); - (fn log10(x: f32) -> (f32); => log10f); (fn modf(x: f32) -> (f32, f32); => modff); (fn nextafter(x: f32, y: f32) -> (f32); => nextafterf); (fn pow(x: f32, y: f32) -> (f32); => powf); @@ -104,12 +106,14 @@ libm_helper! { (fn y0(x: f32) -> (f32); => y0f); (fn y1(x: f32) -> (f32); => y1f); (fn yn(n: i32, x: f32) -> (f32); => ynf); + // verify-sorted-end } } libm_helper! { f64, funcs: { + // verify-sorted-start (fn acos(x: f64) -> (f64); => acos); (fn acosh(x: f64) -> (f64); => acosh); (fn asin(x: f64) -> (f64); => asin); @@ -125,8 +129,8 @@ libm_helper! { (fn erf(x: f64) -> (f64); => erf); (fn erfc(x: f64) -> (f64); => erfc); (fn exp(x: f64) -> (f64); => exp); - (fn exp2(x: f64) -> (f64); => exp2); (fn exp10(x: f64) -> (f64); => exp10); + (fn exp2(x: f64) -> (f64); => exp2); (fn expm1(x: f64) -> (f64); => expm1); (fn fabs(x: f64) -> (f64); => fabs); (fn fdim(x: f64, y: f64) -> (f64); => fdim); @@ -142,12 +146,12 @@ libm_helper! { (fn j1(x: f64) -> (f64); => j1); (fn jn(n: i32, x: f64) -> (f64); => jn); (fn ldexp(x: f64, n: i32) -> (f64); => ldexp); - (fn lgamma_r(x: f64) -> (f64, i32); => lgamma_r); (fn lgamma(x: f64) -> (f64); => lgamma); + (fn lgamma_r(x: f64) -> (f64, i32); => lgamma_r); (fn log(x: f64) -> (f64); => log); + (fn log10(x: f64) -> (f64); => log10); (fn log1p(x: f64) -> (f64); => log1p); (fn log2(x: f64) -> (f64); => log2); - (fn log10(x: f64) -> (f64); => log10); (fn modf(x: f64) -> (f64, f64); => modf); (fn nextafter(x: f64, y: f64) -> (f64); => nextafter); (fn pow(x: f64, y: f64) -> (f64); => pow); @@ -167,6 +171,7 @@ libm_helper! { (fn y0(x: f64) -> (f64); => y0); (fn y1(x: f64) -> (f64); => y1); (fn yn(n: i32, x: f64) -> (f64); => yn); + // verify-sorted-end } } @@ -174,9 +179,22 @@ libm_helper! { libm_helper! { f16, funcs: { + // verify-sorted-start + (fn ceilf(x: f16) -> (f16); => ceilf16); (fn copysign(x: f16, y: f16) -> (f16); => copysignf16); (fn fabs(x: f16) -> (f16); => fabsf16); (fn fdim(x: f16, y: f16) -> (f16); => fdimf16); + (fn floorf(x: f16) -> (f16); => floorf16); + (fn fmaxf(x: f16, y: f16) -> (f16); => fmaxf16); + (fn fminf(x: f16, y: f16) -> (f16); => fminf16); + (fn fmodf(x: f16, y: f16) -> (f16); => fmodf16); + (fn ldexpf16(x: f16, n: i32) -> (f16); => ldexpf16); + (fn rintf(x: f16) -> (f16); => rintf16); + (fn roundf(x: f16) -> (f16); => roundf16); + (fn scalbnf16(x: f16, n: i32) -> (f16); => ldexpf16); + (fn sqrtf(x: f16) -> (f16); => sqrtf16); + (fn truncf(x: f16) -> (f16); => truncf16); + // verify-sorted-end } } @@ -184,8 +202,22 @@ libm_helper! { libm_helper! { f128, funcs: { + // verify-sorted-start + (fn ceil(x: f128) -> (f128); => ceilf128); (fn copysign(x: f128, y: f128) -> (f128); => copysignf128); (fn fabs(x: f128) -> (f128); => fabsf128); (fn fdim(x: f128, y: f128) -> (f128); => fdimf128); + (fn floor(x: f128) -> (f128); => floorf128); + (fn fmax(x: f128, y: f128) -> (f128); => fmaxf128); + (fn fmin(x: f128, y: f128) -> (f128); => fminf128); + (fn fmod(x: f128, y: f128) -> (f128); => fmodf128); + (fn ldexpf128(x: f128, n: i32) -> (f128); => ldexpf128); + (fn rint(x: f128) -> (f128); => rintf128); + (fn round(x: f128) -> (f128); => roundf128); + (fn scalbnf128(x: f128, n: i32) -> (f128); => ldexpf128); + (fn sqrt(x: f128) -> (f128); => sqrtf128); + (fn trunc(x: f128) -> (f128); => truncf128); + // verify-sorted-end } } +// verify-apilist-end diff --git a/library/compiler-builtins/libm/src/math/mod.rs b/library/compiler-builtins/libm/src/math/mod.rs index 9b07dc8a75eb..f0698ad02eba 100644 --- a/library/compiler-builtins/libm/src/math/mod.rs +++ b/library/compiler-builtins/libm/src/math/mod.rs @@ -341,6 +341,7 @@ pub use self::truncf::truncf; cfg_if! { if #[cfg(f16_enabled)] { + // verify-sorted-start mod ceilf16; mod copysignf16; mod fabsf16; @@ -355,7 +356,9 @@ cfg_if! { mod scalbnf16; mod sqrtf16; mod truncf16; + // verify-sorted-end + // verify-sorted-start pub use self::ceilf16::ceilf16; pub use self::copysignf16::copysignf16; pub use self::fabsf16::fabsf16; @@ -370,11 +373,13 @@ cfg_if! { pub use self::scalbnf16::scalbnf16; pub use self::sqrtf16::sqrtf16; pub use self::truncf16::truncf16; + // verify-sorted-end } } cfg_if! { if #[cfg(f128_enabled)] { + // verify-sorted-start mod ceilf128; mod copysignf128; mod fabsf128; @@ -389,7 +394,9 @@ cfg_if! { mod scalbnf128; mod sqrtf128; mod truncf128; + // verify-sorted-end + // verify-sorted-start pub use self::ceilf128::ceilf128; pub use self::copysignf128::copysignf128; pub use self::fabsf128::fabsf128; @@ -404,6 +411,7 @@ cfg_if! { pub use self::scalbnf128::scalbnf128; pub use self::sqrtf128::sqrtf128; pub use self::truncf128::truncf128; + // verify-sorted-end } }