Re: [lmi] PATCH: MSVC compilation fix

[Greg Chicares]

On 6/2/22 15:11, Vadim Zeitlin wrote:
> 
>  I'd like to submit a small patch, which is basically this:
[...]
>  PR 212 (https://github.com/let-me-illustrate/lmi/pull/212).

Committed. Not yet pushed, because I recently pushed something
without running ./nychthemeral_test beforehand, and it broke
something--so I'll wait until I run the complete test suite.

>  Finally, to finish with MSVC-related topics, there are a couple of new
> warnings in the recently added code:
> 
> fdlibm_log1p.c(192): warning C4701: potentially uninitialized local variable 
> 'f' used
> fdlibm_log1p.c(193): warning C4701: potentially uninitialized local variable 
> 'hu' used
> fdlibm_log1p.c(196): warning C4701: potentially uninitialized local variable 
> 'c' used
> fdlibm_expm1.c(234): warning C4701: potentially uninitialized local variable 
> 'c' used
> 
> I don't think any of them indicates any actual problems, but I wonder if it
> might still make sense to initialize these variables just to avoid even
> having to think about it.

Agreed. Is there even a syntax to do that all on one line? Ah, of course:
  :'<,'>s/[,;]/=0&/g
That passes the unit tests. Now let's see if any compiler complains about
useless initializations, like:

    int32_t k=0,hx=0,hu=0,ax=0;
[...'hx' is immediately initialized to a different value...]
    hx = hi_int(x);                             // high word of x

I've been resisting the temptation to rewrite this code, but this ancient
style is not good in this century. I also thought I might try compiling it
as C++, because I recently read somewhere that that might make it faster;
but I hesitate to do so, because AFAICT type punning with unions is legal
in C, but UB in C++.

> E.g. in fdlibm_expm1.c I can see that "c" must be
> initialized if the control flow reaches the line 234, because otherwise "k"
> would have been set to 0 above and we would have returned before using it,
> but the comment saying "c is 0" on that line is still wrong, as it's really
> uninitialized there. And while, again, it doesn't really matter, it seems
> that using "double c=0" wouldn't do any harm and would avoid the issue
> entirely.

Okay, initializing it where declared makes that comment true...except
if it was changed here:

        c  = (hi-x)-lo;

However: pull one thread, and the whole sweater unravels.

> (there are, BTW, still plenty of other warnings in MSVC builds related to
> using enums and doubles in arithmetic expressions, that I still think ought
> to be really fixed rather than suppressed, but I had already posted about
> them

Is that your recent message about fixing some UB, which I have
put off reading in order to fix negation of INT_MAX? If not, could
you remind me where to find it, please?