[libcxx-commits] [PATCH] D118238: [libcxx] [test] Simplify the handling of platform specific NAN formatting in put_long_double

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp:10891
                                     std::string ex(str, iter.base());
-#if defined(TEST_HAS_GLIBC)
-                                    assert(ex == "+nan");
-#else
-                                    assert(ex == "nan");
-#endif
+                                    assert(ex == pnan_sign + "nan");
                                     assert(ios.width() == 0);
----------------
Quuxplusone wrote:
> This is obscure enough I shouldn't really care, but wouldn't it be easier to understand the test if it was like
> ```
> assert(ex == IF_GLIBC("+nan", "nan"));
> ~~~
> assert(ex == IF_GLIBC("+nan*********************",
>                       "nan**********************"));
> ```
> and so on? (The `IF_GLIBC` macro would just be scoped to this one file.)
I think I’d prefer to not go with this approach. I’ll add windows into the mix soon (which does the same as glibc here) so the macro naming would be less straightforward then. And going further, there could (theoretically, at least in some other test) be more than two different variations, which is easier if the conditionals are centralized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118238