odb: Fixed a bug where _dbModule::modbterm_hash_ is emtpy after read_db is executed

[openroad-ci]

Fixes #9123

Problem

The following data structures are empty after read_db if the module is in a child block (non-top block).

_dbModule::dbinst_hash_
_dbModule::modinst_hash_
_dbModule::modbterm_hash_
_dbModule::modnet_hash_

The issue blocks operator swapping.

Root-cause

When it deserializes the hash table during read_db, the deserialize logic searches the module in the top block always, instead of the correct child block.

Solution

Fixed the deserialize logic to correctly find the right child block.

[gemini-code-assist]

Code Review

This pull request correctly centralizes the logic for reconstructing module-level hash tables during database deserialization, fixing a bug related to hierarchical designs in child blocks. The approach of moving this logic into _dbBlock::operator>> is sound.

My review includes two main points:

1. A suggestion to improve robustness by adding null checks to prevent potential crashes if module lookups fail.
2. A recommendation to refactor duplicated code into a template function to enhance maintainability.

Additionally, I've pointed out that one of the new tests doesn't fully validate the fix for dbinst_hash_ and suggested a correction.

Overall, the changes are in the right direction to solve the issue.

[gemini-code-assist]

There are a couple of issues with the new hash reconstruction logic:

1. Potential Null Pointer Dereference: In all four loops, block.module_tbl_->getPtr() could return nullptr, which would lead to a crash when module is dereferenced. A null check is needed.
2. Code Duplication: The loops for modinst_hash_, modbterm_hash_, and modnet_hash_ are nearly identical and can be refactored into a template function to improve maintainability.

I'd suggest refactoring the three similar loops into a template helper function that also includes the null check. The loop for dbinst_hash_ can be a separate function or left as is, but also needs the null check.

Example refactoring for the three similar loops:

template <typename T, typename T_impl>
void constructParentBasedHash(
    _dbBlock& block,
    dbTable<T_impl>* table,
    dbHashTable<T> _dbModule::* hash_member)
{
    dbSet<T> items((dbBlock*)&block, table);
    for (T* obj : items) {
        T_impl* _obj = (T_impl*)obj;
        _dbModule* module = block.module_tbl_->getPtr(_obj->parent_);
        if (module && _obj->name_) {
            (module->*hash_member)[_obj->name_] = _obj->getId();
        }
    }
}

And for dbinst_hash_, you should add a null check like this:

// ...
_dbModule* module = ...; // logic to get module
if (module && _obj->name_) {
    module->dbinst_hash_[_obj->name_] = _obj->getId();
}

[gemini-code-assist]

This test uses child_block2->findInst(), which checks the _dbBlock::inst_hash_. However, the fix in this PR is for populating _dbModule::dbinst_hash_. To correctly verify the fix, you should use mod0_2->findDbInst() to check the module-level hash table.

Suggested change

	dbInst* child_inst_2 = child_block2->findInst("child_inst");
	ASSERT_NE(child_inst_2, nullptr);
	EXPECT_EQ(child_inst_2->getName(), "child_inst");
	dbInst* child_inst_2 = mod0_2->findDbInst("child_inst");
	ASSERT_NE(child_inst_2, nullptr);
	EXPECT_EQ(child_inst_2->getName(), "child_inst");

[github-actions]

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

[github-actions]

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

[github-actions]

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

[jhkim-pii]

@maliberty @precisionmoon
Operator mapping requires this fix.
Ready for review.