Small bug in Stockfish passed pawn evaluation?

[Joost Buijs]

This morning I was looking at the Stockfish passed pawn evaluation and I saw the following:

Code: Select all

else if (pos.pieces(Us) & blockSq)
mbonus += rr * 3 + r * 2 + 3, ebonus += rr + r * 2;

A bitboard containing our pieces is bitwise anded with the block square which is an enum c.q. integer, should this not be anded with the bitmask of the blocksquare?
Maybe I'm mistaken, it is possible that I don't understand the code.

[Joost Buijs]

I see the same thing happening somewhat earlier in the same piece of code.

Code: Select all

// If there aren't any enemy attacks, assign a big bonus. Otherwise
// assign a smaller bonus if the block square isn't attacked.
int k = !unsafeSquares ? 15 : !(unsafeSquares & blockSq) ? 9 : 0;


else if (defendedSquares & blockSq)
k += 4;

[Joost Buijs]

Ah, I now see that there is an & operator defined in the bitboard class which take a square number as one of the arguments.

This is a nice way to create unreadable code, one of the advantages of C++!

[JVMerlino]

Ah, I now see that there is an & operator defined in the bitboard class which take a square number as one of the arguments.

This is a nice way to create unreadable code, one of the advantages of C++!

I can't put into words how much I hate that sort of thing. I defy the Stockfish team to show that the redefinition of that operator adds any ELO.



jm

[wgarvin]

I dislike this trick too. It violates the "Principle of Least Surprise". Programmers who haven't read this bitboard class should expect & to be a bitwise-and of the two values. They should always be able to safely assume that--this trick would surprise them, and is therefore bad.

[Joerg Oster]

I dislike this trick too. It violates the "Principle of Least Surprise". Programmers who haven't read this bitboard class should expect & to be a bitwise-and of the two values. They should always be able to safely assume that--this trick would surprise them, and is therefore bad.

That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...

[Joost Buijs]

That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...

For a programmer it is just counterintuitive, and it doesn't serve any purpose.
The & operator is defined to do a bitwise and, when you overload it and give it a different meaning this is very confusing.

[vittyvirus]

That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...

For a programmer it is just counterintuitive, and it doesn't serve any purpose.
The & operator is defined to do a bitwise and, when you overload it and give it a different meaning this is very confusing.

Infact it's very useful.
1. It helps in writing short, concise code.
2. There can be bugs (and I've seen them) like doing x & sq instead of x & SqMask[sq]
3. It's extensible. For example, 1ULL << sq might be faster than SqMask[sq]. You just need to change few lines of code.
4. There are intrinsics for checking whether a given bit is set or not. You only need to change couple of lines of code to use those intrinsics.

[kbhearn]

That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...

For a programmer it is just counterintuitive, and it doesn't serve any purpose.
The & operator is defined to do a bitwise and, when you overload it and give it a different meaning this is very confusing.

Infact it's very useful.
1. It helps in writing short, concise code.
2. There can be bugs (and I've seen them) like doing x & sq instead of x & SqMask[sq]
3. It's extensible. For example, 1ULL << sq might be faster than SqMask[sq]. You just need to change few lines of code.
4. There are intrinsics for checking whether a given bit is set or not. You only need to change couple of lines of code to use those intrinsics.

all of those advantages can be gained with a named member function instead of an operator, with none of the downside of breaking expected functionality of an operator.

[Joost Buijs]

That's quite funny, because for me as a non-programmer this was rather easy to understand.
And I find it far better readable code than to always write "SquareBB[square]" ...

For a programmer it is just counterintuitive, and it doesn't serve any purpose.
The & operator is defined to do a bitwise and, when you overload it and give it a different meaning this is very confusing.

Infact it's very useful.
1. It helps in writing short, concise code.
2. There can be bugs (and I've seen them) like doing x & sq instead of x & SqMask[sq]
3. It's extensible. For example, 1ULL << sq might be faster than SqMask[sq]. You just need to change few lines of code.
4. There are intrinsics for checking whether a given bit is set or not. You only need to change couple of lines of code to use those intrinsics.

all of those advantages can be gained with a named member function instead of an operator, with none of the downside of breaking expected functionality of an operator.

Indeed, a member function or even a simple macro can do the same without the need to redefine an operator.