couple of questions about stockfish code ?

[BeyondCritics]

Yes, and then you can use it everywhere without it being greppable. Does that help? How does hiding something help the programmer realise the dangers of what he is doing?

All type casts remain fully greppable.

What danger is hidden here? I do not see any danger hidden.

Every type cast circumvents the type system. You could make a typo and inadvertly cast some other variable. The type system is there, to help you staying out of trouble. Why not use it?

In modern software development you typically think "object oriented". That means (among other things) that operations and data are considered as a whole. Operations on your data do not enter the stage out of thin air. They are considered as part of your interface. If you do sudden type casts you break this interface.

Of course, in your own code you do what you like.

[syzygy]

Yes, and then you can use it everywhere without it being greppable. Does that help? How does hiding something help the programmer realise the dangers of what he is doing?

All type casts remain fully greppable.

What danger is hidden here? I do not see any danger hidden.

Neither do I, that's why I would not use static_cast in the first place

Every type cast circumvents the type system. You could make a typo and inadvertly cast some other variable. The type system is there, to help you staying out of trouble. Why not use it?

That is an argument that make sense to me.

[Rein Halbersma]

Yes, and then you can use it everywhere without it being greppable. Does that help? How does hiding something help the programmer realise the dangers of what he is doing?

All type casts remain fully greppable.

What danger is hidden here? I do not see any danger hidden.

Neither do I, that's why I would not use static_cast in the first place

Code is typically written once, but read many times. So readability and greppability of code constructs should receive more weight than number of characters to type.

Every type cast circumvents the type system. You could make a typo and inadvertly cast some other variable. The type system is there, to help you staying out of trouble. Why not use it?

That is an argument that make sense to me.

For integer casting, you "only" have to deal with signedness and promotion rules. But for pointer casting in C++, you also have to deal with base and derived classes, which can give very nasty effects if you do plain C style casting. You typically want to choose between the specific types of C++ casts (const, static, reinterpret or dynamic), whereas C-style casts will try all of them and will silently pick the first to succeed. This can be very surprising, and even depend on which types of headers you have included. Apart from number of characters, static_cast doesn't harm you in anyway, and is a good habit to get into. (like seatbelts, even though you probably don't use those either if you re-park your car a few meters, you wouldn't argue that you don't wear those on your daily commute "just because you know what you are doing")

[kbhearn]

Then I wonder a bit why C++ allows C-type casts at all (perhaps to improve compatibility with C) and even provides an arguably more convenient "function"-notation for it (definitely not of any help for improving compatibility with C).

c-style for compatibility
function-style for homogenity with class constructors
static_cast<> and friends for explicitness of meaning (in the event that your meaning isn't possible the compiler will squeal or there's multiple candidates and the one you wanted isn't the one it picks you can specify which cast)

personally i find the verboseness of static_cast<> not very readable so for simple casts between base types i use c-style - but i understand how people working with complex casts between classes would prefer to always show the cast they mean. And in the spirit of using a different cast i think reinterpret_cast can actually be used in a round-about way correctly for the int16_t cast problem we've done a bunch of different ways. It didn't occur to me because all the restrictions on reinterpret_cast being well defined always made it seem like it couldn't be used for its intent even but here goes. It relies on the following two phrases from http://en.cppreference.com/w/cpp/langua ... rpret_cast

An lvalue expression of type T1 can be converted to reference to another type T2. The result is an lvalue or xvalue referring to the same object as the original lvalue, but with a different type. No temporary is created, no copy is made, no constructors or conversion functions are called. The resulting reference can only be accessed safely if allowed by the type aliasing rules (see below)

AliasedType is the (possibly cv-qualified) signed or unsigned variant of DynamicType

Code: Select all

inline Value mg_value(Score s) {
  int16_t rv;
  reinterpret_cast<uint16_t&>(rv) = s; // automatically truncated in conversion to uint16_t
  return Value(rv);  // darn enum
}

I definitely like the C philosophy better than the C++ one. If I know that a cast is correct and what I want, just let me do it. By typing (int) I tell the compiler what I want it to do. But OK, C++ is C++.

(What I don't understand is how the C++ style can be reconciled with things like function and operator overloading, ideal devices for avoiding that the programmer knows what he is doing.)

personally i love that operator overloading is possible - and almost never use it ;) it's much nicer to be able to write numeric-related classes you're going to make heavy use of with overloaded operators and be able to write:

Code: Select all

  scaledvector = myvector * 2;

than

Code: Select all

  vector_copy(scaledvector, myvector);
  vector_scale(scaledvector, 2);

so long as the overloaded operators aren't abused and given different or unclear meanings they're a nice addition :)

[syzygy]

Yes, and then you can use it everywhere without it being greppable. Does that help? How does hiding something help the programmer realise the dangers of what he is doing?

All type casts remain fully greppable.

What danger is hidden here? I do not see any danger hidden.

Neither do I, that's why I would not use static_cast in the first place

Code is typically written once, but read many times. So readability and greppability of code constructs should receive more weight than number of characters to type.

Sure, but why would anyone start looking for this cast?

And if one hides the static_cast behind a function, then greppability disappears: one can still find the function, but now one has to grep for "as_int" or whatever the function is called. (Of course I am trying to respond here to comments by separate people who may not agree with each other. Perhaps you would agree that hiding the static_cast defeats the purpose of making it greppable, but Oliver might not see a need for greppable casts.)

For integer casting, you "only" have to deal with signedness and promotion rules. But for pointer casting in C++, you also have to deal with base and derived classes, which can give very nasty effects if you do plain C style casting. You typically want to choose between the specific types of C++ casts (const, static, reinterpret or dynamic), whereas C-style casts will try all of them and will silently pick the first to succeed.

I accept that C++ may really need different types of casts because of the complexity of the language.

As far as I'm aware, casting a pointer from one pointer type to another in C never changes the memory address to which the pointer points. Of course the pointer with its new pointer type may behave very differently, but that is all governed by that new pointer type. Or is my view of C too simplistic?

(OK, what you are casting to a pointer might not be a pointer but an array, etc.)

This can be very surprising, and even depend on which types of headers you have included.

Yes, if the headers define int16_t as a pointer, then all bets are off

[syzygy]

And in the spirit of using a different cast i think reinterpret_cast can actually be used in a round-about way correctly for the int16_t cast problem we've done a bunch of different ways. It didn't occur to me because all the restrictions on reinterpret_cast being well defined always made it seem like it couldn't be used for its intent even but here goes. It relies on the following two phrases from http://en.cppreference.com/w/cpp/langua ... rpret_cast

I came across reinterpret_cast myself, but the things I read about it gave me the impression that it did not solve the problem completely.

Judging merely from the wording, it should be a "legal" way of doing the union-trick: start from the uint16_t value, take the bit pattern, reinterpret it as int16_t. If that is how it works, it will always work OK in practice, but would still not be guaranteed to work by the standard. And if, despite what "reinterpret" suggests, it converts the uint16_t value to int16_t, then it will rely on implementation-defined behavior.

But I will take a better look...

[BeyondCritics]

Yes, and then you can use it everywhere without it being greppable. Does that help? How does hiding something help the programmer realise the dangers of what he is doing?

All type casts remain fully greppable.

What danger is hidden here? I do not see any danger hidden.

Neither do I, that's why I would not use static_cast in the first place

Every type cast circumvents the type system. You could make a typo and inadvertly cast some other variable. The type system is there, to help you staying out of trouble. Why not use it?

That is an argument that make sense to me.

Sorry i do not get how both of your comments are fitting together. Somehow i feel there are contracitory, but maybe this is just a matter of my thinking style
In any case, i do not pretend to be infallible and i have always admitted any error i made. Isn't that right?

[syzygy]

Yes, and then you can use it everywhere without it being greppable. Does that help? How does hiding something help the programmer realise the dangers of what he is doing?

All type casts remain fully greppable.

What danger is hidden here? I do not see any danger hidden.

Neither do I, that's why I would not use static_cast in the first place

Every type cast circumvents the type system. You could make a typo and inadvertly cast some other variable. The type system is there, to help you staying out of trouble. Why not use it?

That is an argument that make sense to me.

Sorry i do not get how both of your comments are fitting together. Somehow i feel there are contracitory, but maybe this is just a matter of my thinking style

When I say that an argument makes sense, that does not mean I am so convinced by it that I will change my style.

It makes sense to have the habit of using static_cast just in case it will catch a typing error here and there. I can see that it can have that benefit. On the other hand, I cannot see benefits of the kind "better readability" or "better greppability" (yes, better greppable... but why is that a benefit here). At least not for simple integer casts. (Which aren't always so simple of course, as we see here, but static_cast doesn't check ranges so at most gives a sense of false security I'd say.)

So, were I to write a program in C++, I would not use static_cast for a simple cast of integer types for better readability (which I do not see), for better greppability (for which I see no need) or for better catching of typing errors (which I accept might be a small benefit, but I can live with having to catch those myself).

Were I to write a program in C++, I might well use static_cast for complicated casts where it is helpful to verify my own thinking about the cast. It then also helps readability.

[syzygy]

Code: Select all

inline Value mg_value(Score s) {
  int16_t rv;
  reinterpret_cast<uint16_t&>(rv) = s; // automatically truncated in conversion to uint16_t
  return Value(rv);  // darn enum
}

I'm afraid any solution using reinterpret_cast will necessarily rely on implementation-defined behavior:

Unlike static_cast, but like const_cast, the reinterpret_cast expression does not compile to any CPU instructions. It is purely a compiler directive which instructs the compiler to treat the sequence of bits (object representation) of expression as if it had the type new_type.

So it does what the union trick is supposed to do. On a weird machine where int16_t is stored big endian and uint16_t is stored little_endian, this mg_value() function will give surprising results.

I'm now wondering whether your sign-bit approach does not suffer from the same problem.

Code: Select all

inline Value mg_Value(Score s) {
    static const uint32_t mask = 0xFFFFU;
    static const int sign = 0x8000;
    return ((int)(s & mask) ^ sign) - sign;
}

I guess it does work correctly. Since s and mask are both uint32_t, s & mask is certain to give the expected result. The result is then cast to int, which is well defined because the result value is in the range of int. xor-ing with sign works fine, since sign is an int. Subtracting sign is again OK.

So the manual sign extension approach seems fully correct. On the weird machine I mentioned, there may be changes in byte ordering when going from uint32_t to int, but that does not pose any problems. The casts will then compile to byte swap instructions.

Your approach subtracts 0x8000 twice in case (s & mask) >= 0x8000, then casts. If (s & mask) < 0x8000, it adds 0x8000 and subtracts it again, then casts. So it is equivalent to:

Code: Select all

inline Value mg_value(Score score) {
    uint16_t u = score;
    return Value(u < 0x8000 ? u : u - 0x10000);
}

So it is not even necessary to cast to int16_t.

Code: Select all

inline Value eg_value(Score score) {
    uint16_t u = (score + 0x8000) >> 16;
    return Value(u < 0x8000 ? u : u - 0x10000);
}

Another attempt:

Code: Select all

inline Value mg_value(Score s) {
  int v = (uint16_t)s;
  return Value(v < 0x8000 ? v : v - 0x10000);
}

or

Code: Select all

inline Value mg_value(Score s) {
  int v = s & 0xffff;
  return Value(v < 0x8000 ? v : v - 0x10000);
}

We don't even need uint16_t / int16_t to be defined. As in your original solution, in fact.

[syzygy]

Code: Select all

inline Value mg_value(Score s) {
  int v = s & 0xffff;
  return Value(v < 0x8000 ? v : v - 0x10000);
}

We don't even need uint16_t / int16_t to be defined. As in your original solution, in fact.

But unfortunately the compiler now stops producing optimal code.
Or is the code simply incorrect.... hmm

The compiler does get it if I include an int16_t cast:

Code: Select all

inline Value mg_value(Score s) {
  int v = s & 0xffff;
  return Value(int16_t(v < 0x8000 ? v : v - 0x10000));
}

Why would that make a difference?