odb: fix BumpMapEntry declaration mismatch

[ahmed532]

src/odb/include/odb/3dblox.h forward declared BumpMapEntry as a class while its definition is a struct. Clang warned about it.
Fixes: #9093

[gemini-code-assist]

Code Review

This pull request correctly fixes a forward declaration mismatch for BumpMapEntry in src/odb/include/odb/3dblox.h. The change modifies the forward declaration from class to struct to match its definition, which resolves a Clang compiler warning. The change is correct and improves code clarity. I approve these changes.

[github-actions]

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

[stefanottili]

Hard to fix things if you don't use a compiler that issues these warnings.
Sorry that I wasn't more explicit in my bug report, but it seems the change from class to struct has to be applied to all "non db" classes in 3dblox.h.

This compiles using clang without -Wmismatched-tags

src/odb/include/odb/3dblox.h

 17 namespace odb {
 18 class dbDatabase;
 19 class dbChip;
 20 class dbChipRegionInst;
 21 class dbChipInst;
 22 class dbChipRegion;
 23 class dbBlock;
 24 class dbBTerm;
 25 class dbInst;
 26 class dbTech;
 27 class dbLib;
 28 
 29 struct ChipletDef;
 30 struct ChipletRegion;
 31 struct ChipletInst;
 32 struct DesignDef;
 33 struct Connection;
 34 struct BumpMapEntry;
 35 
 36 class ThreeDBlox
 37 {

[ahmed532]

Sorry I missed it 😅 I'll create another PR. What is the standard way of compiling with clang then? I used bazel; it doesn't have all the needed flags set?

[maliberty]

You can manually run cmake with -DCMAKE_C_COMPILER=<path>/clang -DCMAKE_CXX_COMPILER=<path>/clang++

[ahmed532]

I tried -DCMAKE_C_COMPILER=<path>/clang -DCMAKE_CXX_COMPILER=<path>/clang++ but unfortunately there were errors because of my particular env I think (too many libraries everywhere). So, I used bazel. Maybe I should try cmake in the container?

[maliberty]

bazel doesn't show this error. It may also be specific to the mac version of clang.

[ahmed532]

usually clang on mac is older I think.

[ahmed532]

GCC supports the warning flag. I can find them with cmake -B mismatch-build -S . -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS="-Werror=mismatched-tags"
Will fix them all.

[maliberty]

Please add that flag to add_compile_options in src/CMakeLists.txt and OPENROAD_COPTS in BUILD.bazel (if supported in that compiler too).

[ahmed532]

I'm encountering internal compiler errors with this flag on GCC. It looks like a known issue and it has complex interactions with libraries, PreCompiled Headers, and other flags.
I do not think we should add it. Especially because the C++ standard does not distinguish between class and struct in declarations. Only the definition matters in specifying the visibility struct -> public by default and class -> private by default.

I think I found all the mismatches anyway.

[ahmed532]

The only concern, I think, is "MSVC (Microsoft Visual C++) can exhibit issues with Name Mangling if tags do not match between declaration and definition".

[ahmed532]

I can add it conditionally to clang and MSVC though 🤔

[maliberty]

We don't support MSVC so I wouldn't worry about it. I suspect there would be lots of other issues.