Skip to content

Refine const function and references, remove unnecessary copy initializations #1170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KvanTTT wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Refine const function and references, remove unnecessary copy initializations #1170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b22a6cd
Apply "Parameter can be made pointer/reference to const"
KvanTTT Mar 11, 2026
9a5c31a
Apply "Member function can be made const"
KvanTTT Mar 11, 2026
2d023fe
Apply "Pass value parameters by const reference"
KvanTTT Mar 15, 2026
508c294
Fix remaining unnecessary copy initializations
KvanTTT Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 69 additions & 69 deletions cpp/book/book.cpp
View file
Open in desktop

Large diffs are not rendered by default.

134 changes: 67 additions & 67 deletions cpp/book/book.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,25 @@ struct BookHash {
BookHash();
BookHash(Hash128 historyHash, Hash128 situationHash);

bool operator<(const BookHash other) const;
bool operator>(const BookHash other) const;
bool operator<=(const BookHash other) const;
bool operator>=(const BookHash other) const;
bool operator==(const BookHash other) const;
bool operator!=(const BookHash other) const;

BookHash operator^(const BookHash other) const;
BookHash operator|(const BookHash other) const;
BookHash operator&(const BookHash other) const;
BookHash& operator^=(const BookHash other);
BookHash& operator|=(const BookHash other);
BookHash& operator&=(const BookHash other);
bool operator<(const BookHash& other) const;
bool operator>(const BookHash& other) const;
bool operator<=(const BookHash& other) const;
bool operator>=(const BookHash& other) const;
bool operator==(const BookHash& other) const;
bool operator!=(const BookHash& other) const;

BookHash operator^(const BookHash& other) const;
BookHash operator|(const BookHash& other) const;
BookHash operator&(const BookHash& other) const;
BookHash& operator^=(const BookHash& other);
BookHash& operator|=(const BookHash& other);
BookHash& operator&=(const BookHash& other);

// Get book hash, and the symmetry to apply so that hist aligns with book nodes for its current position, (histspace -> nodespace)
// and the list of symmetries such that book node is invariant under those symmetries.
static void getHashAndSymmetry(const BoardHistory& hist, int repBound, BookHash& hashRet, int& symmetryToAlignRet, std::vector<int>& symmetriesRet, int bookVersion);

friend std::ostream& operator<<(std::ostream& out, const BookHash other);
friend std::ostream& operator<<(std::ostream& out, const BookHash& other);
std::string toString() const;
static BookHash ofString(const std::string& s);
};
Expand All @@ -58,7 +58,7 @@ struct BookMove {
double biggestWLCostFromRoot; // Largest single cost due to winloss during path from root

BookMove();
BookMove(Loc move, int symmetryToAlign, BookHash hash, double rawPolicy);
BookMove(Loc move, int symmetryToAlign, const BookHash& hash, double rawPolicy);

// Apply symmetry to this BookMove.
BookMove getSymBookMove(int symmetry, int xSize, int ySize) const;
Expand Down Expand Up @@ -158,7 +158,7 @@ class BookNode {

std::atomic<int> visitedFlag; // Used for multithreaded coordination

BookNode(BookHash hash, Book* book, Player pla, const std::vector<int>& symmetries);
BookNode(const BookHash& hash, Book* book, Player pla, const std::vector<int>& symmetries);
~BookNode();
BookNode(const BookNode&) = delete;
BookNode& operator=(const BookNode&) = delete;
Expand Down Expand Up @@ -187,43 +187,43 @@ class SymBookNode {
SymBookNode(const SymBookNode& other) = default;
SymBookNode& operator=(const SymBookNode& other) = default;

bool isNull();
SymBookNode applySymmetry(int symmetry);
bool isNull() const;
SymBookNode applySymmetry(int symmetry) const;

bool isMoveInBook(Loc move);
int numUniqueMovesInBook();
std::vector<BookMove> getUniqueMovesInBook();
bool isMoveInBook(Loc move) const;
int numUniqueMovesInBook() const;
std::vector<BookMove> getUniqueMovesInBook() const;

Player pla();
BookHash hash();
std::vector<int> getSymmetries();
Player pla() const;
BookHash hash() const;
std::vector<int> getSymmetries() const;

BookValues& thisValuesNotInBook();
bool& canExpand();
bool& canReExpand();
const RecursiveBookValues& recursiveValues();
int minDepthFromRoot();
double minCostFromRoot();
double totalExpansionCost();
BookValues& thisValuesNotInBook() const;
bool& canExpand() const;
bool& canReExpand() const;
const RecursiveBookValues& recursiveValues() const;
int minDepthFromRoot() const;
double minCostFromRoot() const;
double totalExpansionCost() const;

// For testing purposes only - allows direct access to internal node
BookNode* getNodeForTesting() { return node; }
BookNode* getNodeForTesting() const { return node; }

// Returns NULL for the root or if somehow a parent is not found
SymBookNode canonicalParent();
SymBookNode canonicalParent() const;

SymBookNode follow(Loc move);
SymBookNode follow(Loc move) const;

// Returns NULL if the move is not legal OR the move is not in the book.
SymBookNode playMove(Board& board, BoardHistory& hist, Loc move);
SymBookNode playMove(Board& board, BoardHistory& hist, Loc move) const;
// Returns NULL if the move is not legal. The move *must* not be in the book.
SymBookNode playAndAddMove(Board& board, BoardHistory& hist, Loc move, double rawPolicy, bool& childIsTransposing);
SymBookNode playAndAddMove(Board& board, BoardHistory& hist, Loc move, double rawPolicy, bool& childIsTransposing) const;

// Returns false and does not modify ret if playing the moves in the book to reach here hit an illegal move.
// Fills moveHistoryRet with the sequence of moves played. If there is an illegal move, includes the illegal move.
// This should only happen if a book was loaded from disk that is corrupted, or else only astronomically rarely on hash collisions.
bool getBoardHistoryReachingHere(BoardHistory& ret, std::vector<Loc>& moveHistoryRet);
bool getBoardHistoryReachingHere(BoardHistory& ret, std::vector<Loc>& moveHistoryRet, std::vector<double>& winlossRet);
bool getBoardHistoryReachingHere(BoardHistory& ret, std::vector<Loc>& moveHistoryRet) const;
bool getBoardHistoryReachingHere(BoardHistory& ret, std::vector<Loc>& moveHistoryRet, std::vector<double>& winlossRet) const;

friend class ConstSymBookNode;
friend class Book;
Expand All @@ -244,36 +244,36 @@ class ConstSymBookNode {
ConstSymBookNode& operator=(const SymBookNode& other);
ConstSymBookNode& operator=(const ConstSymBookNode& other) = default;

bool isNull();
ConstSymBookNode applySymmetry(int symmetry);
bool isNull() const;
ConstSymBookNode applySymmetry(int symmetry) const;

Player pla();
BookHash hash();
std::vector<int> getSymmetries();
Player pla() const;
BookHash hash() const;
std::vector<int> getSymmetries() const;

bool isMoveInBook(Loc move);
int numUniqueMovesInBook();
std::vector<BookMove> getUniqueMovesInBook();
bool isMoveInBook(Loc move) const;
int numUniqueMovesInBook() const;
std::vector<BookMove> getUniqueMovesInBook() const;

const BookValues& thisValuesNotInBook();
bool canExpand();
bool canReExpand();
const RecursiveBookValues& recursiveValues();
int minDepthFromRoot();
double minCostFromRoot();
double totalExpansionCost();
const BookValues& thisValuesNotInBook() const;
bool canExpand() const;
bool canReExpand() const;
const RecursiveBookValues& recursiveValues() const;
int minDepthFromRoot() const;
double minCostFromRoot() const;
double totalExpansionCost() const;

// Returns NULL for the root or if somehow a parent is not found
ConstSymBookNode canonicalParent();
ConstSymBookNode canonicalParent() const;

ConstSymBookNode follow(Loc move);
ConstSymBookNode follow(Loc move) const;
// Returns NULL if the move is not legal OR the move is not in the book.
ConstSymBookNode playMove(Board& board, BoardHistory& hist, Loc move);
ConstSymBookNode playMove(Board& board, BoardHistory& hist, Loc move) const;

// Returns false and does not modify ret if playing the moves in the book to reach here hit an illegal move.
// This should only happen if a book was loaded from disk that is corrupted, or else only astronomically rarely on hash collisions.
bool getBoardHistoryReachingHere(BoardHistory& ret, std::vector<Loc>& moveHistoryRet);
bool getBoardHistoryReachingHere(BoardHistory& ret, std::vector<Loc>& moveHistoryRet, std::vector<double>& winlossRet);
bool getBoardHistoryReachingHere(BoardHistory& ret, std::vector<Loc>& moveHistoryRet) const;
bool getBoardHistoryReachingHere(BoardHistory& ret, std::vector<Loc>& moveHistoryRet, std::vector<double>& winlossRet) const;

friend class Book;
};
Expand Down Expand Up @@ -381,7 +381,7 @@ class Book {
Book(
int bookVersion,
const Board& board,
Rules rules,
const Rules& rules,
Player initialPla,
int repBound,
BookParams params
Expand Down Expand Up @@ -421,8 +421,8 @@ class Book {
// Returns a null SymBookNode if hist goes off the end of the book.
SymBookNode get(const BoardHistory& hist);
ConstSymBookNode get(const BoardHistory& hist) const;
SymBookNode getByHash(BookHash hash);
ConstSymBookNode getByHash(BookHash hash) const;
SymBookNode getByHash(const BookHash& hash);
ConstSymBookNode getByHash(const BookHash& hash) const;

void recompute(const std::vector<SymBookNode>& newAndChangedNodes);
void recomputeEverything();
Expand All @@ -431,7 +431,7 @@ class Book {

std::vector<SymBookNode> getNextNToExpand(int n);
std::vector<SymBookNode> getAllLeaves(double minVisits);
std::vector<SymBookNode> getAllNodes();
std::vector<SymBookNode> getAllNodes() const;

// Result for computeMinCostToChangeWinLoss
struct MinCostResult {
Expand All @@ -440,7 +440,7 @@ class Book {
std::set<BookHash> nodes;

MinCostResult() : totalCost(0.0), nodes() {}
MinCostResult(double cost, std::set<BookHash> nodeSet) : totalCost(cost), nodes(nodeSet) {}
MinCostResult(double cost, const std::set<BookHash>& nodeSet) : totalCost(cost), nodes(nodeSet) {}
};

// Computes the minimum cost in PN-search-style for potentially proving a node's winLossValue
Expand Down Expand Up @@ -488,10 +488,10 @@ class Book {
static Book* loadFromFile(const std::string& fileName, int numThreadsForRecompute=1);

private:
int64_t getIdx(BookHash hash) const;
BookNode* get(BookHash hash);
const BookNode* get(BookHash hash) const;
BookNode* getAssertNotNull(BookHash hash);
int64_t getIdx(const BookHash& hash) const;
BookNode* get(const BookHash& hash);
const BookNode* get(const BookHash& hash) const;
BookNode* getAssertNotNull(const BookHash& hash);
const BookNode* getAssertNotNull(BookHash hash) const;
bool add(BookHash hash, BookNode* node);

Expand Down
18 changes: 9 additions & 9 deletions cpp/command/benchmark.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ static vector<PlayUtils::BenchmarkResults> doFixedTuneThreads(
NNEvaluator*& nnEval,
Logger& logger,
double secondsPerGameMove,
vector<int> numThreadsToTest,
const vector<int>& numThreadsToTest,
bool printElo,
std::function<int(int)> getDesiredBatchSize
const std::function<int(int)>& getDesiredBatchSize
);
static vector<PlayUtils::BenchmarkResults> doAutoTuneThreads(
const SearchParams& params,
Expand All @@ -38,8 +38,8 @@ static vector<PlayUtils::BenchmarkResults> doAutoTuneThreads(
NNEvaluator*& nnEval,
Logger& logger,
double secondsPerGameMove,
std::function<void(int)> reallocateNNEvalWithEnoughBatchSize,
std::function<int(int)> getDesiredBatchSize
const std::function<void(int)>& reallocateNNEvalWithEnoughBatchSize,
const std::function<int(int)>& getDesiredBatchSize
);

#ifdef USE_EIGEN_BACKEND
Expand Down Expand Up @@ -387,9 +387,9 @@ static vector<PlayUtils::BenchmarkResults> doFixedTuneThreads(
NNEvaluator*& nnEval,
Logger& logger,
double secondsPerGameMove,
vector<int> numThreadsToTest,
const vector<int>& numThreadsToTest,
bool printElo,
std::function<int(int)> getDesiredBatchSize
const std::function<int(int)>& getDesiredBatchSize
) {
vector<PlayUtils::BenchmarkResults> results;

Expand Down Expand Up @@ -425,8 +425,8 @@ static vector<PlayUtils::BenchmarkResults> doAutoTuneThreads(
NNEvaluator*& nnEval,
Logger& logger,
double secondsPerGameMove,
std::function<void(int)> reallocateNNEvalWithEnoughBatchSize,
std::function<int(int)> getDesiredBatchSize
const std::function<void(int)>& reallocateNNEvalWithEnoughBatchSize,
const std::function<int(int)>& getDesiredBatchSize
) {
vector<PlayUtils::BenchmarkResults> results;

Expand Down Expand Up @@ -576,7 +576,7 @@ int MainCmds::genconfig(const vector<string>& args, const string& firstCommand)
return 1;
}

auto promptAndParseInput = [](const string& prompt, std::function<void(const string&)> parse) {
auto promptAndParseInput = [](const string& prompt, const std::function<void(const string&)>& parse) {
while(true) {
try {
cout << prompt << std::flush;
Expand Down
2 changes: 1 addition & 1 deletion cpp/command/commandline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ void KataGoCommandLine::parseArgs(const vector<string>& args) {
return parse(mutableCopy);
}

void KataGoCommandLine::setShortUsageArgLimit() {
void KataGoCommandLine::setShortUsageArgLimit() const {
helpOutput->setShortUsageArgLimit((int)_argList.size() - numBuiltInArgs);
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/command/commandline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class KataGoCommandLine : public TCLAP::CmdLine
void parseArgs(const std::vector<std::string>& args);

//Args added AFTER calling this will only show up in the long help output, and not the short usage line.
void setShortUsageArgLimit();
void setShortUsageArgLimit() const;

void addModelFileArg();
void addHumanModelFileArg();
Expand Down
4 changes: 2 additions & 2 deletions cpp/command/demoplay.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ static void writeLine(
const Search* search, const BoardHistory& baseHist,
const vector<double>& winLossHistory, const vector<double>& scoreHistory, const vector<double>& scoreStdevHistory
) {
const Board board = search->getRootBoard();
const Board& board = search->getRootBoard();
int nnXLen = search->nnXLen;
int nnYLen = search->nnYLen;

Expand Down Expand Up @@ -214,7 +214,7 @@ static void initializeDemoGame(Board& board, BoardHistory& hist, Player& pla, Ra
{ Move(g(9,8), b), nw, Move(g(9,10), b) },
};

vector<Move> chosenOpening = specialOpenings[rand.nextUInt((int)specialOpenings.size())];
const vector<Move>& chosenOpening = specialOpenings[rand.nextUInt((int)specialOpenings.size())];
vector<vector<Move>> chosenOpenings;

for(int j = 0; j<8; j++) {
Expand Down
9 changes: 4 additions & 5 deletions cpp/command/evalsgf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,14 +565,13 @@ int MainCmds::evalsgf(const vector<string>& args) {

if(printScoreNow) {
sout << "Score now (ROOT position):\n";
Board copy(board);
BoardHistory copyHist(hist);
Color area[Board::MAX_ARR_SIZE];
copyHist.endAndScoreGameNow(copy,area);
copyHist.endAndScoreGameNow(board,area);

for(int y = 0; y<copy.y_size; y++) {
for(int x = 0; x<copy.x_size; x++) {
Loc l = Location::getLoc(x,y,copy.x_size);
for(int y = 0; y<board.y_size; y++) {
for(int x = 0; x<board.x_size; x++) {
Loc l = Location::getLoc(x,y,board.x_size);
sout << PlayerIO::colorToChar(area[l]);
}
sout << endl;
Expand Down
10 changes: 5 additions & 5 deletions cpp/command/genbook.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ static void maybeParseBonusFile(
const std::string& bonusFile,
int boardSizeX,
int boardSizeY,
Rules rules,
const Rules& rules,
int repBound,
double bonusFileScale,
Logger& logger,
Expand All @@ -196,7 +196,7 @@ static void maybeParseBonusFile(
bool allowGameOver = false;
Rand seedRand("bonusByHash");
sgf->iterAllPositions(
flipIfPassOrWFirst, allowGameOver, &seedRand, [&](Sgf::PositionSample& unusedSample, const BoardHistory& sgfHist, const string& comments) {
flipIfPassOrWFirst, allowGameOver, &seedRand, [&](const Sgf::PositionSample& unusedSample, const BoardHistory& sgfHist, const string& comments) {
(void)unusedSample;
if(comments.size() > 0 && (
comments.find("BONUS") != string::npos ||
Expand Down Expand Up @@ -735,7 +735,7 @@ int MainCmds::genbook(const vector<string>& args) {

auto setNodeThisValuesFromFinishedSearch = [&](
SymBookNode node,
Search* search,
const Search* search,
const SearchNode* searchNode,
const Board& board,
const BoardHistory& hist,
Expand Down Expand Up @@ -937,7 +937,7 @@ int MainCmds::genbook(const vector<string>& args) {
std::sort(
extraMoveLocsToExpand.begin(),
extraMoveLocsToExpand.end(),
[](std::pair<Loc,float>& p0, std::pair<Loc,float>& p1) {
[](const std::pair<Loc,float>& p0, const std::pair<Loc,float>& p1) {
return p0.second > p1.second;
}
);
Expand Down Expand Up @@ -1382,7 +1382,7 @@ int MainCmds::genbook(const vector<string>& args) {
Rand seedRand("bonusByHash");
int64_t variationsAdded = 0;
sgf->iterAllPositions(
flipIfPassOrWFirst, allowGameOver, &seedRand, [&](Sgf::PositionSample& unusedSample, const BoardHistory& sgfHist, const string& comments) {
flipIfPassOrWFirst, allowGameOver, &seedRand, [&](const Sgf::PositionSample& unusedSample, const BoardHistory& sgfHist, const string& comments) {
(void)unusedSample;
(void)comments;
int gameThreadIdx = 0;
Expand Down
Loading
Loading