[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lmi] lmi tests and MSVC
Re: [lmi] lmi tests and MSVC
Sun, 19 Feb 2023 16:34:00 +0100
On Sun, 19 Feb 2023 13:39:37 +0000 Greg Chicares <firstname.lastname@example.org>
GC> On 2/19/23 00:20, Vadim Zeitlin wrote:
GC> > On Mon, 24 Apr 2017 22:19:46 +0000 Greg Chicares
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
GC> > GC> like to know whether the very recent 'bourn_cast_test' does.
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> > ???? uncaught exception: std::runtime_error: Cast would transgress upper
GC> > This happens in test_same<long long int> on the line
GC> > T imax = bourn_cast<T>(max);
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
GC> > they equal only with MSVC but not the other compilers. I could look at
GC> > in more details but I wanted to ask first just in case the problem is
GC> > already immediately clear to you?
GC> Here, 'max' is of type 'long double'; test_same() says:
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
GC> we could do something like this:
GC> + // [comment explaining the extra condition]
GC> - if(traits::is_integer)
GC> + if(traits::is_integer && 64 <= LDBL_MANT_DIG)
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> > 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
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
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> > but they look mostly harmless (even though definitely annoying).
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
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
GC> > GC> like missing support for "%Lf" and formatting infinity as "1.#INF".
GC> > GC> mingw* toolchains work around that with libmingwex. Does msvc now use
GC> > GC> updated C RTL that doesn't have these defects, so that these
GC> > GC> workarounds can be expunged?
GC> > I wanted to check this one as well, but numeric_io_test.cpp currently
GC> > fails even with LMI_MSVCRT defined:
GC> > ???? test failed: '15' == '12'
GC> > [file numeric_io_test.cpp, line 141]
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> > ???? 2 test errors detected; 441 tests succeeded
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> 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
GC> BTW, given that we use clang's sanitizers, do we still care about
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).
Description: PGP signature