Skip to content

Conversation

@ahmed532
Copy link

Some structs in src/odb/include/odb/3dblox.h where forward declared as classes. Which errored with "-Wmismatched-tags" compiler flag. Also, found another mismatch in PartitionMgr.h
Note that C++ does not distinguish between class and struct in a forward declaration. Only MSVC can have issues with it.

Some structs in src/odb/include/odb/3dblox.h where forward declared
as classes. Which errored with "-Wmismatched-tags" compiler flag.
Also, found another mismatch in PartitionMgr.h
Note that C++ does not distinguish between class and struct in a
forward declaration. Only MSVC can have issues with it.

Signed-off-by: Ahmed R. Mohamed <[email protected]>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes mismatched forward declarations of structs that were declared as class, which resolves -Wmismatched-tags compiler warnings. The changes in src/odb/include/odb/3dblox.h and src/par/include/par/PartitionMgr.h are accurate and improve code correctness by ensuring consistency between declarations and definitions. The grouping of struct forward declarations in 3dblox.h also slightly improves readability. The changes look good to me.

@ahmed532 ahmed532 self-assigned this Dec 31, 2025
@github-actions
Copy link
Contributor

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

@maliberty
Copy link
Member

Apparently it matters on mac as well which doesn't use MSVC.

@ahmed532
Copy link
Author

Apparently it matters on mac as well which doesn't use MSVC.

OK I'll add it to compile options for clang then.

@github-actions
Copy link
Contributor

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

@ahmed532
Copy link
Author

This started with #9093 which was reported by Stefan who uses a mac. To my knowledge we have zero MSVC users (nobody has ever reported an issue). In the report it is a warning but we should clean it up. In the CI we treat warnings as errors to keep them from creeping in.
I don't think you need to make it always an error as non-devs should be able to build even if we have a mismatch
option(ALLOW_WARNINGS "Flag to allow compilation with compiler warnings: on by default" ON)

if (NOT ALLOW_WARNINGS)
  add_compile_options(
    $<$<CXX_COMPILER_ID:GNU>:-Werror>
  )
endif()

in src/CMakeLists.txt
in the CI ALLOW_WARNINGS=0

Quote by @maliberty

@osamahammad21 osamahammad21 merged commit e95d33b into The-OpenROAD-Project:master Jan 4, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants