[libc-commits] [PATCH] D116777: [libc] Add rounding mode support for MPFR testing macros.
Tue Ly via Phabricator via libc-commits
libc-commits at lists.llvm.org
Wed Jan 12 04:05:12 PST 2022
lntue added inline comments.
Comment at: libc/utils/MPFRWrapper/MPFRUtils.h:291
-#define EXPECT_MPFR_MATCH(op, input, match_value, tolerance) \
+#define MISSING_ARGS(...) static_assert(true, "Missing arguments");
> lntue wrote:
> > sivachandra wrote:
> > > I am not sure if this `MISSING_ARGS` macro is any useful. For, if any error occurs, it will print an error message but tell us nothing about what is actually wrong. I would think a better approach would be to let the `RoundingMode` arg to `get_mpfr_matcher` take a default value of `RoundingMode::Nearest`. That way, you can also simplify the macros below to some thing like this:
> > >
> > > ```
> > > #define EXPECT_MPFR_MATCH(op, input, match_value, tolerance, ...) \
> > > EXPECT_THAT(match_value, \
> > > __llvm_libc::testing::mpfr::get_mpfr_matcher<op>( \
> > > input, match_value, tolerance \
> > > __VA_OPT__(,) __VA_ARGS__))
> > > ```
> > >
> > > This way, if there is any error in the args, we will actually get a proper compiler error message. If using `__VA_OPT__` is a concern, we can use `-std=c++20` for compiling MPFRWrapper.
> > I use MISSING_ARGS as a simple way to avoid gnu-zero-variadic-macro-arguments warnings. It is simpler and does not require extra compiler flags as __VA_OPT__ option. I changed it to GET_MPFR_DUMMY_ARG and added comments to make the intention clearer.
> Ah! So, why should `GET_MPFR_MACRO` be a variadic macro at all?
To support and not litter all current usages of EXPECT_MPFR_MATCH with default rounding mode as the 5th argument, it is expanded to either EXPECT_MPFR_MATCH_ROUNDING with 5 arguments, or EXPECT_MPFR_MATCH_DEFAULT with 4 arguments by GET_MPFR_MACRO.
If 5 arguments are provided to EXPECT_MPFR_MATCH, GET_MPFR_MACRO will get 6 arguments (not including GET_MPFR_DUMMY_ARG), and if 4 arguments are provided to EXPECT_MPFR_MATCH, GET_MPFR_MACRO will only get 5 arguments (not including GET_MPFR_DUMMY_ARG), and hence it needs to be variadic.
It is the 4 argument case that variadic GET_MPFR_MACRO got complained by -Wgnu-zero-variadic-macro-arguments without GET_MPFR_DUMMY_ARG.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libc-commits