Re: [lmi] [lmi-commits] master 7f767c85 1/5: Make signed zeros easy to distinguish in tests

[Vadim Zeitlin]

On Thu,  2 Jun 2022 20:13:04 -0400 (EDT) Greg Chicares 
<gchicares@sbcglobal.net> wrote:

GC> branch: master
GC> commit 7f767c854200710c5f8be427a30a575a88e35a45
GC> Author: Gregory W. Chicares <gchicares@sbcglobal.net>
GC> Commit: Gregory W. Chicares <gchicares@sbcglobal.net>
GC> 
GC>     Make signed zeros easy to distinguish in tests
GC> ---
GC>  math_functions.hpp      | 20 ++++++++++++++++++++
GC>  math_functions_test.cpp | 46 ++++++++++++++++++++++++++++++++++++++++++++++
GC>  2 files changed, 66 insertions(+)
GC> 
GC> diff --git a/math_functions.hpp b/math_functions.hpp
GC> index 30f43f99..dcf6cc96 100644
GC> --- a/math_functions.hpp
GC> +++ b/math_functions.hpp
GC> @@ -96,6 +96,26 @@ inline T outward_quotient(T numerator, T denominator)
GC>      return (0 < numerator == 0 < denominator) ? x + y : x - y;
GC>  }
GC>  
GC> +/// Signed zeros, for comparison tests.
GC> +
GC> +enum signed_zero
GC> +    {pos0 // positive zero
GC> +    ,neg0 // negative zero
GC> +    };
GC> +
GC> +template<typename T>
GC> +bool operator==(T t, signed_zero z)
GC> +{
GC> +    static_assert(std::is_floating_point_v<T>);

 In C++11 I would have used SFINAE to ensure that this overload isn't even
considered for non-floating-point types. In C++20 I'd use concepts to do
the same thing. I don't think it's worth writing SFINAE now when a better
solution is so close, but I don't think we can use the concepts yet
neither.

 But I'd either add some "CONCEPT !!" comment here to remind us to make
them when they can or, even simpler, just provide overloads for float,
double and long double (forwarding to the same private implementation-only
function) -- by the time another floating point is introduced in C++ we'll
have concepts too.

 This is not an entirely useless exercise because many compilers (both MSVC
and gcc do it, although clang is better) list _all_ the global comparison
operators whenever there is a problem choosing a candidate for any
comparison. The list they output is already pretty long (wx is not
blameless here, as wx headers define a number of global operator==()s), but
it's still better to avoid yet another candidate to it.

GC> +    // These intentionally do not compile:
GC> +//  LMI_TEST(pos0 == 0);
GC> +//  LMI_TEST(pos0 == '0');
GC> +//  LMI_TEST(pos0 == nullptr);

 BTW, why not use the same stupid trick I use in wx tests and have some
special LMI_TEST_COMPILE_FAILURE symbol defining which would enable
compiling this code and then have a separate "make check-fail" target that
would use "if $(CXX) -DLMI_TEST_COMPILE_FAILURE -c $<; then exit 1; fi"?

 FWIW in wx case I used a few separate symbols, i.e. you'd have
LMI_TEST_COMPILE_FAILURE_xxx, LMI_TEST_COMPILE_FAILURE_yyy etc. This is not
ideal, but at least allows to be sure that the code not supposed to
compiler really does not compile.

VZ

pgpsVG2bvGvAU.pgp
Description: PGP signature