[libc-commits] [PATCH] D116777: [libc] Add rounding mode support for MPFR testing macros.

Wed Jan 12 00:10:35 PST 2022

sivachandra added inline comments.

================
Comment at: libc/utils/MPFRWrapper/MPFRUtils.h:295
+
+#define GET_MPFR_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
----------------
Add double-underscore prefix to internal macro names.

================
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?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116777/new/

https://reviews.llvm.org/D116777