Skip to content

src/drt/src/global.h is very generic; rename#10611

Merged
maliberty merged 1 commit into
The-OpenROAD-Project:masterfrom
hzeller:feature-20260608-drt-global
Jun 8, 2026
Merged

src/drt/src/global.h is very generic; rename#10611
maliberty merged 1 commit into
The-OpenROAD-Project:masterfrom
hzeller:feature-20260608-drt-global

Conversation

@hzeller

@hzeller hzeller commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

src/drt/src/global.h is often used witout any prefix at all (so just included as "global.h"), which is conflicing if there happens to be e.g. @abc in the include path (which also has a "global.h". It also makes the current cyclic dependencies to fix a bit more confusing.

Rename to drt-global.h to have an unambiguous name.

@hzeller hzeller requested a review from a team as a code owner June 8, 2026 14:26
@hzeller hzeller requested a review from maliberty June 8, 2026 14:26
@github-actions github-actions Bot added the size/S label Jun 8, 2026

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

Copy link
Copy Markdown
Contributor

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 renames the header file 'global.h' to 'drt-global.h' and updates its inclusion across various source and header files in the 'drt' tool. The review feedback suggests renaming 'global.cpp' to 'drt-global.cpp' for consistency and ensuring that any references in 'CMakeLists.txt' are updated to prevent build failures.

Comment thread src/drt/BUILD Outdated
"src/frRegionQuery.h",
"src/gc/FlexGC.h",
"src/global.h",
"src/drt-global.h",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Since 'global.h' has been renamed to 'drt-global.h', please make sure to update any corresponding references in 'CMakeLists.txt' (the primary build system for OpenROAD) to prevent build failures.

Comment thread src/drt/src/global.cpp
#include "db/obj/frMPin.h"
#include "db/obj/frMarker.h"
#include "db/obj/frMaster.h"
#include "drt-global.h"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since 'global.h' has been renamed to 'drt-global.h', please consider renaming 'global.cpp' to 'drt-global.cpp' as well. Keeping the header and source file names consistent improves maintainability and avoids confusion.

`src/drt/src/global.h` is often used witout any prefix
at all (so just included as "global.h"), which is conflicing
if there happens to be e.g. `@abc` in the include path (which also
has a `"global.h"`. It also makes the current cyclic dependencies
to fix a bit more confusing.

Rename to `drt-global.h` to have an unambiguous name.

Signed-off-by: Henner Zeller <h.zeller@acm.org>
@hzeller hzeller force-pushed the feature-20260608-drt-global branch from 5844d13 to 60accdb Compare June 8, 2026 14:50
@maliberty maliberty enabled auto-merge June 8, 2026 14:59
@maliberty maliberty merged commit b20a36c into The-OpenROAD-Project:master Jun 8, 2026
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants