Re: [lmi] PATCH: MSVC compilation fix

[Greg Chicares]

On 6/3/22 01:44, Vadim Zeitlin wrote:
> On Fri, 3 Jun 2022 01:13:49 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
> 
> GC> On 6/2/22 15:11, Vadim Zeitlin wrote:
[...fdlibm...]
> GC> I've been resisting the temptation to rewrite this code, but this ancient
> GC> style is not good in this century. I also thought I might try compiling it
> GC> as C++, because I recently read somewhere that that might make it faster;
> 
>  I haven't looked at this code very carefully, so I could be missing
> something, but there doesn't seem to be anything in it that could possibly
> benefit from using C++.

The idea was simply to recompile the same code as C++ instead of C,
due to a notion that C++ compilers optimize better. I don't see why
gcc would optimize this code better than g++, but I thought it was
so easy to test that I might try it...except:

> GC> but I hesitate to do so, because AFAICT type punning with unions is legal
> GC> in C, but UB in C++.
> 
>  Formally speaking it is, of course, and you're supposed to be using
> memcpy() instead, but I doubt very much that any compiler dares to really
> break this, considering how common it is. OTOH modern compilers did already
> surprise me many times in the past, so why not again.

My concern would be edge cases. Even if compiler writers strive not
to break type-punning via unions, they might accidentally break it
in curious and unforeseen circumstances.

With g++11, we get std::bit_cast<>(). For g++10, we could do e.g.:

template<typename To, typename From>
inline // not constexpr due to memcpy()
To bit_cast(From const from)
{
    static_assert(sizeof(To) == sizeof(From));
    static_assert(std::is_trivially_copyable_v<To>);
    static_assert(std::is_trivially_copyable_v<From>);
    // deprecated in C++23
    // typename std::aligned_storage<sizeof(To), alignof(To)>::type t;
    To t;
    std::memcpy(&to, &from, sizeof to);
    return t;
}

and then we could compile the fdlibm TUs as C++, without UB.

But I don't imagine that's important or worthwhile.

> GC> > (there are, BTW, still plenty of other warnings in MSVC builds related 
> to
> GC> > using enums and doubles in arithmetic expressions
[...]
> I think I mentioned it a couple of times after this, but I can't find
> anything in the list archives somehow

I found these two PRs:
  https://github.com/let-me-illustrate/lmi/pull/179
  https://github.com/let-me-illustrate/lmi/pull/193
which are both marked as "merged".

I found some references to this issue in last year's archives, and AFAICT
the only reason I didn't address them is that I wasn't building with gcc-11.
These deprecated conversions shouldn't be relied upon.

>  If you'd like to check these warnings yourself, removing
> -Wno-deprecated-enum-float-conversion from
> gcc_version_specific_cxx_warnings in workhorse.make should show most of

Done, fixed, and pushed:
  d9bdcd4d7..b6916ab8e  master -> master

> them, even if I think clang and MSVC give a few more than gcc does.

I'd be inclined to fix deprecated conversions found by other tools, too.