Re: [lmi] MSVC warnings [all but TOMS 748]

[Greg Chicares]

On 6/6/22 14:20, Vadim Zeitlin wrote:
> On Mon, 6 Jun 2022 03:37:50 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
[...]
> GC> The whole point of
> GC>     return (t < 0) ? -static_cast<U>(t) : static_cast<U>(t);
> GC> is to return the signed value 't' unchanged if it's positive,
> GC> else to return its modular-additive inverse. It's already perfect.
> 
>  Sorry, but I have to strongly disagree with the conclusion here. It's not
> perfect at all because it's very non-obvious that this works correctly, it
> took me quite some time to convince myself of this.

But now, in retrospect, you know it's correct. And in retrospect,
having gained the knowledge that the technique is correct, I feel
that it's the simplest and most elegant way to write the expression.

> A comment would
> probably help, but using "~t+1" is completely unambiguous about what it is
> doing unlike "-(U)t".

My view is the opposite. Having acquired the knowledge that
operator-(unsigned int) means "additive inverse", I see "-(U)t"
as natural and obvious: if t is positive, use it; otherwise,
use its additive inverse. Perhaps I should rename it:
  u_abs() --> additive_inverse()

OTOH, I shrink in horror from "~t+1", which requires me to understand
two's complement. I don't truly understand it: i.e., I have never
"internalized" it, and I have no wish to do so, because it's an
unnecessary concept. Knowing that it's the way the hardware is
designed is like knowing that they use MOSFETs instead of bipolar
junction transistors. And "-(U)t" is what makes it unnecessary:
I move the problem from the twos-complement signed-integer domain
into the clear and clean unsigned=modular domain, where the problem
can be solved directly.

>  We may disable the warning, although I'd really rather avoid doing it
> globally because this one has caught unintentional errors many times in the
> past, but it won't make this function any more clear.

I'm willing to disable the warning and write commentary, if you
would please tell me what msvc pragma to write.

> [back to my original reply]
> 
> GC> > But could we perhaps use "~t+1" instead of "-t"
> GC> > here? As we're guaranteed 2-complement representation in C++ now, it 
> should
> GC> > do exactly the same thing even in theory now, and not just in practice.
> GC> 
> GC> But that's such an abstruse theory. I considered ideas (like this)
> GC> that perform a negation by operating on the bits, and thought
> GC> them too obscure--even though it's a theorem that they're valid,
> GC> they're still jarringly unreadable.
> 
>  But this is how 2 complement works.

I don't know how it works.
I don't need to know how it works.
I don't want to know how it works.

> Something else may be simpler, but
> doesn't correspond to objective reality, etched down in silicon.
> 
> GC> And of course the approach in HEAD is just the Law of the Negation
> GC> of the Negation, "which every child can understand as soon as the
> GC> mysterious junk in which the old idealistic philosophy wrapped
> GC> itself is stripped off." [Herrn Eugen Dührings Umwälzung der
> GC> Wissenschaft, Marx Engels Werke, Berlin: Dietz Verlag, 1956--,
> GC> vol. 20, p. 126.]
> 
>  Fortunately for the children they only need to operate with the idealised
> Platonic arithmetics and not its imperfect applied implementation.

There's the crux of our difference: Realism vs. Idealism.

> Clearly
> the Law of the Negation of the Negation doesn't work for C++ because you
> can't negate INT_MIN at all.

I don't want to negate it. I want to take its additive inverse.
Engels just wasn't enough of a mathematician to know the difference,
or he would have formulated that Law more rigorously.

I look at this as a well-defined operation on an abstract C machine,
so the operation is reality, while silicon exists only in the world
of illusion. I think you're taking silicon as fundamental, and
reasoning that C is just an abstraction created after the fact to
describe that hardware reality.

The best we can hope for is to understand each other's viewpoint,
because neither viewpoint is going to change.

> GC> >  Except for this warning, there are only a few more:
> GC> 
> GC> Two of them are like this:
> GC> 
> GC> > ihs_acctval.cpp(421,5): warning C4805: '==': unsafe mix of type 'bool' 
> and
> GC> > type 'double' in operation
> GC> > 
> GC> >    This could again be fixed by just using 0.0 instead of false.
> GC> 
> GC> Here's the line in question:
> GC>     LMI_ASSERT(false                    == InvariantValues().IsMec   );
> GC> I hesitate to change that, because the RHS is semantically boolean,
> GC> just as though it were 'IsDead' (even though it's held as a double),
> 
>  I've raised several times in the past the question of just why exactly it
> is held as a double. I seem to remember that the conclusion was that I
> shouldn't return to this question again, but, of course, I've completely
> forgotten why :-/

'double' is a universal arithmetic type in the practical sense
that it can hold any integer or boolean value that arises in
life-insurance data, losslessly, as well as floating-point values.

C++ has no heterogeneous arrays, but double[] serves the purpose.

> GC> Instead, I first want to ask how this can be called unsafe.
> GC> Isn't the comparison perfectly well defined, as follows?
> GC>   (0.0 = dbl_prec_value) ? false : true
> 
> (sorry for nitpicking, but you definitely meant "==" and not "=" here)

I still make that mistake often. That's why I write the literal on the LHS.

>  Anyhow, there is no doubt that the behaviour of this code is well defined.
> But IMNSHO it's also pretty clear that in many cases comparing double with
> bool indicates a mistake, when you meant to compare it with something else,

That's a mistake I don't imagine that I often make, or else this warning
would arise more often.

> because there is usually no reason at all to compare variables of different
> types nor rely on implicit conversions from double to int or bool.

I generally agree that implicit conversions can be dangerous when values
are being mutated or assigned. For comparisons, it seems less dangerous.

>  There is also, of course, an even more fundamental question of comparing
> floating point numbers for equality being suspicious (and gcc has
> -Wfloat-equal for this).

Another vain attempt to idiot-proof C and C++, which can't be idiot-proofed.
There's nothing wrong with tests like these:

  double e = some_function();
  double f = some_function();
  assert(0.0 == f - e);

  double d {1.5};
  do_something();
  assert(1.5 == d); // could fire if do_something() changes directed rounding

> GC> If we were to change this, we'd want to reconsider each instance of:
> GC>   git grep 'false.*=='
> GC>   git grep 'true.*=='
> GC> But I'm present inclined to think that this warning is worse than
> GC> useless, and should be suppressed.
> 
>  I admit that I don't remember making any actual mistakes that were caught
> by this warning, so it indeed doesn't seem to very useful in practice. But
> OTOH comparing doubles and booleans very clearly seems like not the right
> thing to do.

I do agree that generally it's better to use operator!(double).
But that performs a test for floating-point equality to {0.0},
so we're already in a state of sin, if sin that be.

>  To summarize, I'd rather stop using doubles for booleans entirely, but,
> even if I don't remember the reasons for it,

Arrays can't contain heterogeneous members. But double[] can
contain all the heterogeneous arithmetic values lmi cares about.

> I think this is impossible to
> do in practice.

Correct.

> If we can't do it, then I think explicitly casting doubles
> to bool is the next best idea, just to show that we really know what we're
> doing.

Consider:

  double d {0.25};
  int i = (int) d;
  if(!i) printf("'d' is false: i.e., it must equal 0.0 exactly");

Does that show that we really know what we're doing?
I think it cries out for simplification.

> But if this is undesirable neither, I can indeed just suppress this
> warning globally.

Yes, please.