odb: Refactored insert buffer code

[openroad-ci]

• Added const qualifiers.
• Added std::vector.reserve(16) calls.
• Minor name changes.
• Fixed lint issues.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[QuantamHD]

Is the idea here that most often we need just a small number of list items? If so using boost small vector would be a better choice. The current code will always cause a heap allocation, boost small vector will allocate up to N items on the stack, and only go to the heap if the initial capacity is exceeded.

https://www.boost.org/doc/libs/1_69_0/doc/html/boost/container/small_vector.html

[jhkim-pii]

small_vector looks good. Thanks for the suggestion!

[gemini-code-assist]

Code Review

This pull request primarily focuses on improving const-correctness and clarity across several OpenDB (ODB) classes, particularly dbNet, dbModule, and dbInsertBuffer. Key changes include adding const qualifiers to numerous method parameters and return types, such as load_pins in buffer insertion functions and various get and find methods in dbModule and dbInsertBuffer. Function parameter names were also updated for better readability, like changing mcf to MillerMult in dbNet::getGndTotalCap and dbid_ to oid in dbNet::getNet functions. Additionally, internal helper functions for buffer placement were renamed for more descriptive purposes (e.g., placeNewBuffer to placeBufferAtLocationOrPin). Performance improvements were made in dbModNet by pre-allocating vector capacity. The documentation for buffer insertion APIs was updated to reflect default parameter values. Review comments highlighted and suggested fixes for several instances of const_cast and potential const-correctness violations, specifically in dbInsertBuffer::findLCA and dbModule::getOwner, recommending changes to return const pointers. The reviewer also suggested reverting some parameter name changes, such as rid back to rseg_id in dbNet::set1stRSegId and mapFile back to capNodeMap in dbNet::collapseInternalCapNum, for improved descriptiveness.

[gemini-code-assist]

Using const_cast here breaks const-correctness. The function findLCA is declared const and takes const pointers, but it returns a non-const pointer. This can lead to unintended modifications of what should be a const object.

To maintain const-correctness, it's better to change the return type of findLCA to const dbModule*. This may require propagating const to the calling functions, but it leads to safer and more robust code. I've added a suggestion on the header file to fix this.

[jhkim-pii]

However, the suggested solution makes using this API more difficult.

[gemini-code-assist]

This function is const and takes const pointers, but returns a non-const pointer. This breaks const-correctness and requires a const_cast in the implementation. To fix this, the return type should be const dbModule*. This will likely require propagating const to callers of findLCA.

Suggested change

	dbModule* findLCA(const dbModule* m1, const dbModule* m2) const;
	const dbModule* findLCA(const dbModule* m1, const dbModule* m2) const;

[gemini-code-assist]

The cast (_dbModule*) this inside a const member function is a const_cast in disguise, which breaks const-correctness. This function should return a const dbBlock* to be const-correct.

Consider changing the function signature in db.h and this implementation to:
const dbBlock* dbModule::getOwner() const
{
const _dbModule* obj = (const _dbModule*) this;
return (const dbBlock*) obj->getOwner();
}
This would require _dbObject::getOwner() to be a const method, and callers of dbModule::getOwner() might need to be updated to handle a const dbBlock*.

const dbBlock* dbModule::getOwner() const
{
  const _dbModule* obj = (const _dbModule*) this;
  return (const dbBlock*) obj->getOwner();
}

[gemini-code-assist]

The parameter name rid is less descriptive than the original rseg_id. While the function name set1stRSegId provides context, a more descriptive name like rseg_id or rsegId would improve readability and clarity, especially for developers unfamiliar with this part of the codebase.

Suggested change

	void set1stRSegId(uint32_t rid);
	void set1stRSegId(uint32_t rseg_id);

[gemini-code-assist]

The new parameter name mapFile is more generic than the original capNodeMap. Given the function collapseInternalCapNum and its comment "compact internal capnode number", capNodeMap seems more descriptive of the file's contents. Consider reverting to the original name for better clarity.

Suggested change

	void collapseInternalCapNum(FILE* mapFile);
	void collapseInternalCapNum(FILE* capNodeMap);

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"