Re: [lmi] lmi tests and MSVC

[Vadim Zeitlin]

On Sun, 19 Feb 2023 13:39:37 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 2/19/23 00:20, Vadim Zeitlin wrote:
GC> [...]
GC> > On Mon, 24 Apr 2017 22:19:46 +0000 Greg Chicares 
<gchicares@sbcglobal.net> wrote:
GC> [...]
GC> > GC> It would be good to know whether at least lmi's unit tests (except for
GC> > GC> 'any_member_test') build and report zero errors for msvc. I'd 
especially
GC> > GC> like to know whether the very recent 'bourn_cast_test' does.
GC> > 
GC> >  I still don't have a build system allowing me to build all the lmi tests
GC> > with MSVC, but I tried at least this one (even though it's not so recent
GC> > any more) and it fails with the following error:
GC> > 
GC> > ???? uncaught exception: std::runtime_error: Cast would transgress upper 
limit.
GC> > 
GC> > This happens in test_same<long long int> on the line
GC> > 
GC> >   T imax = bourn_cast<T>(max);
GC> > 
GC> > because "limit" and "from" are both equal to 9.2233720368547758e+18 (or
GC> > 43e0000000000000 as IEEE-754 binary, i.e. 1b63) inside bourn_cast(). I
GC> > don't really understand why should we throw in case of equality nor why 
are
GC> > they equal only with MSVC but not the other compilers. I could look at 
this
GC> > in more details but I wanted to ask first just in case the problem is
GC> > already immediately clear to you?
GC> 
GC> Here, 'max' is of type 'long double'; test_same() says:
GC> 
GC>     // an 80-bit long double type whose 64-bit mantissa suffices to
GC>     // test the limits of every integral type up to 64 digits exactly
GC>     // because it can distinguish +/-(2^64) from +/-(2^64 - 1).

 Ah, I see, thanks.

GC> To guard against compilers that lack the necessary support,

 And MSVS is indeed one, as its current versions treat "long double" as
being identical (although still being a different type) to "double"
(curiously enough, its ancient versions did have a 80-bit long double, but
no more).

GC> we could do something like this:
GC> 
GC> +    // [comment explaining the extra condition]
GC> -    if(traits::is_integer)
GC> +    if(traits::is_integer && 64 <= LDBL_MANT_DIG)
GC> 
GC> I'll commit such a revision next week unless you disagree.

 No objections and I can confirm that this fixes this problem with MSVC.
Unfortunately this just means that now I hit the next one (it's one of my
long standing annoyances with lmi tests: they stop on first error instead
of continuing while it makes sense, but I guess we're not going to return
to this discussion today):

 I get unexpected "Cast would transgress upper limit." from the call to
bourn_cast<unsigned long long int>(f_uninteresting) at line 528 because
f_uninteresting is actually exactly the same as f_interesting for me.
I am not sure how it happens, but I do see that static_cast-ing
0xffffff7fffffffff to float gives f_interesting. Looking at the
implementation, it uses cvtsi2sd instructions to convert lower and higher
parts independently and then adds them together which yields the correct
0x43effffff0000000 64-bit double which should get converted to 0x5f7fffff
32-bit float but somehow ends up as 0x5f80000 instead... I have to admit
that I don't know at all what's going on here right now but, again, I could
look into this further if you think it's worth doing it.

GC> >  FWIW there are also several compilation warnings in this test with MSVC:
GC> > 
GC> > bourn_cast_test.cpp(473,36): warning C4305: 'initializing': truncation 
from 'const unsigned __int64' to 'float'
GC> > bourn_cast_test.cpp(90,9): warning C4805: '==': unsafe mix of type 'bool' 
and type 'const long double' in operation
GC> > bourn_cast_test.cpp(737): message : see reference to function template 
instantiation 'void test_same<bool>(const char *,int)' being compiled
GC> > bourn_cast_test.cpp(89,41): warning C4805: '==': unsafe mix of type 
'bool' and type 'long double' in operation
GC> > bourn_cast_test.cpp(166,1): warning C4245: 'initializing': conversion 
from 'int' to 'CFrom', signed/unsigned mismatch
GC> > bourn_cast_test.cpp(780): message : see reference to function template 
instantiation 'void test_signednesses<true,false>(const char *,int)' being 
compiled
GC> > bourn_cast_test.cpp(167,1): warning C4245: 'initializing': conversion 
from 'int' to 'IFrom', signed/unsigned mismatch
GC> > bourn_cast_test.cpp(168,1): warning C4245: 'initializing': conversion 
from '__int64' to 'LFrom', signed/unsigned mismatch
GC> > bourn_cast_test.cpp(170,1): warning C4245: 'initializing': conversion 
from 'int' to 'CTo', signed/unsigned mismatch
GC> > bourn_cast_test.cpp(784): message : see reference to function template 
instantiation 'void test_signednesses<false,true>(const char *,int)' being 
compiled
GC> > bourn_cast_test.cpp(171,1): warning C4245: 'initializing': conversion 
from 'int' to 'ITo', signed/unsigned mismatch
GC> > bourn_cast_test.cpp(172,1): warning C4245: 'initializing': conversion 
from '__int64' to 'LTo', signed/unsigned mismatch
GC> > 
GC> > but they look mostly harmless (even though definitely annoying).
GC> 
GC> They would occur for other compilers as well, but are suppressed
GC> by pragmata. I'd accept a patch adding msvc pragmata.

 I've created https://github.com/let-me-illustrate/lmi/pull/229
corresponding to xanadu/msvc-bourn_cast-test-warn branch with the mostly
trivial fixes.

GC> > GC> BTW, does LMI_MSVCRT serve any purpose any more? It's used only in
GC> > GC> 'numeric_io*', to work around printf() problems in the msw system C 
RTL
GC> > GC> like missing support for "%Lf" and formatting infinity as "1.#INF". 
The
GC> > GC> mingw* toolchains work around that with libmingwex. Does msvc now use 
an
GC> > GC> updated C RTL that doesn't have these defects, so that these 
LMI_MSVCRT
GC> > GC> workarounds can be expunged?
GC> > 
GC> >  I wanted to check this one as well, but numeric_io_test.cpp currently
GC> > fails even with LMI_MSVCRT defined:
GC> > 
GC> > ???? test failed:   '15' == '12'
GC> > [file numeric_io_test.cpp, line 141]
GC> > 
GC> > ???? test failed:   '15' == '16'
GC> > [file numeric_io_test.cpp, line 174]
GC> > Conversions:
GC> >   2/3, lmi  : 1.032e-05 s mean;          9 us least of 970 runs
GC> >   inf, lmi  : 5.285e-06 s mean;          4 us least of 1893 runs
GC> > 
GC> > ???? 2 test errors detected; 441 tests succeeded
GC> > 
GC> > Again, it doesn't seem immediately obvious to me what is wrong here and I
GC> > could try to find out if you think this would be useful.
GC> 
GC> Commit e90e53748f697ea probably explains msvc's failure on line 174
GC> as well as valgrind's. I would guess that the failure on line 141 has
GC> a similar cause.

 Yes, it seems related to this, but the details are still hazy, at least to
me. Notably, note that floating_point_decimals() returns a _higher_ than
expected value 16 here -- and how can this be explained by lack of 80 bit
precision?

GC> BTW, given that we use clang's sanitizers, do we still care about
GC> valgrind?

 There are some bugs that are not caught by sanitizers but are caught by
valgrind, so if we can afford using it, I think we should keep doing it,
especially as it doesn't really cost us much (and I should probably add a
CI job running the tests under it).

 Regards,
VZ

pgpZ567CrtGw5.pgp
Description: PGP signature