Skip to content

Commit

Permalink
Fix a warning with MSVC
Browse files Browse the repository at this point in the history 
warning C4706: assignment within conditional expression

No functional change.
  • Loading branch information
@mcostalba
mcostalba committed Sep 17, 2016
1 parent ea41f18 commit 92f01aa
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/search.cpp
View file
Expand Up @@ -506,7 +506,7 @@ void Thread::search() {

if ( rootMoves.size() == 1
|| Time.elapsed() > Time.optimum() * unstablePvFactor * improvingFactor / 628
|| (mainThread->easyMovePlayed = doEasyMove))
|| (mainThread->easyMovePlayed = doEasyMove, doEasyMove))
{
// If we are allowed to ponder do not stop the search now but
// keep pondering until the GUI sends "ponderhit" or "stop".
Expand Down

3 comments on commit 92f01aa

@lucasart
Copy link

@lucasart lucasart commented on 92f01aa Sep 17, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be a better fix to disable this silly warning ? Original code was more readable.

@mcostalba
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning may be silly in this case but in other cases may be very useful to catch subtle bugs, moreover the original version is not idiomatic and actually it is very rare. It is much more common the case with ==, so a reader could reasonably wonder if 'mainThread->easyMovePlayed = doEasyMove' it is actually intended or it is a bug....now the reader does not wonder anymore :-)

@ttruscott
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative which might be more readable:

              || (mainThread->easyMovePlayed = doEasyMove) == true)

Please sign in to comment.