Skip to content

stt: adjust naming to Google-style (excluding method names)#10194

Merged
maliberty merged 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:stt-tidy-naming
Apr 20, 2026
Merged

stt: adjust naming to Google-style (excluding method names)#10194
maliberty merged 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:stt-tidy-naming

Conversation

@openroad-ci

Copy link
Copy Markdown
Member

Summary

adjust naming to Google-style (excluding method names)

Type of Change

  • Refactoring

Impact

None

Verification

  • I have verified that the local build succeeds (./etc/Build.sh).
  • I have run the relevant tests and they pass.
  • My code follows the repository's formatting guidelines.
  • I have signed my commits (DCO).

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

@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 updates variable and constant naming across the stt module to improve consistency and better align with standard C++ naming conventions, such as using kPascalCase for constants and trailing underscores for member variables. The review feedback suggests further refining the names in src/stt/src/flt/flute.cpp by removing redundant wl suffixes from several constants and variables to ensure uniform naming throughout the file.

Comment thread src/stt/src/flt/flute.cpp
Comment on lines +448 to +451
constexpr double kAawl = 0.6;
constexpr double kBbwl = 0.3;
const float ccwl = 7.4 / ((d + 10.) * (d - 3.));
const float ddwl = 4.8 / (d - 1);

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

To improve consistency with flutes_medium_degree and better adhere to the Google C++ Style Guide, consider using kAa, kBb, cc, and dd instead of the wl-suffixed names. This also avoids the lack of underscores in ccwl and ddwl (which should be cc_wl and dd_wl if the suffix is kept) and ensures tunable parameters are defined as named constants.

Suggested change
constexpr double kAawl = 0.6;
constexpr double kBbwl = 0.3;
const float ccwl = 7.4 / ((d + 10.) * (d - 3.));
const float ddwl = 4.8 / (d - 1);
constexpr double kAa = 0.6;
constexpr double kBb = 0.3;
const float cc = 7.4 / ((d + 10.) * (d - 3.));
const float dd = 4.8 / (d - 1);
References
  1. Variables should use snake_case and constants should use kPascalCase. Consistency across similar functions is also a key maintainability principle. (link)
  2. Define tunable parameters as named constants instead of using hardcoded magic numbers.

Comment thread src/stt/src/flt/flute.cpp
Comment on lines +455 to +456
const float dx = ccwl * (xs[d - 2] - xs[1]);
const float dy = ccwl * (ys[d - 2] - ys[1]);

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

Update usages of ccwl to cc for consistency.

Suggested change
const float dx = ccwl * (xs[d - 2] - xs[1]);
const float dy = ccwl * (ys[d - 2] - ys[1]);
const float dx = cc * (xs[d - 2] - xs[1]);
const float dy = cc * (ys[d - 2] - ys[1]);

Comment thread src/stt/src/flt/flute.cpp
if (si[r] == 0 || si[r] == d - 1) {
score[nbp] = (xs[r + 1] - xs[r - 1]) - penalty[r]
- AAWL * (ys[d - 2] - ys[1]) - DDWL * disty[r];
- kAawl * (ys[d - 2] - ys[1]) - ddwl * disty[r];

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

Update usages of kAawl and ddwl to kAa and dd for consistency.

Suggested change
- kAawl * (ys[d - 2] - ys[1]) - ddwl * disty[r];
- kAa * (ys[d - 2] - ys[1]) - dd * disty[r];

Comment thread src/stt/src/flt/flute.cpp
} else {
score[nbp] = (xs[r + 1] - xs[r - 1]) - penalty[r]
- BBWL * (ys[si[r] + 1] - ys[si[r] - 1]) - DDWL * disty[r];
- kBbwl * (ys[si[r] + 1] - ys[si[r] - 1]) - ddwl * disty[r];

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

Update usages of kBbwl and ddwl to kBb and dd for consistency.

Suggested change
- kBbwl * (ys[si[r] + 1] - ys[si[r] - 1]) - ddwl * disty[r];
- kBb * (ys[si[r] + 1] - ys[si[r] - 1]) - dd * disty[r];

Comment thread src/stt/src/flt/flute.cpp
if (s[r] == 0 || s[r] == d - 1) {
score[nbp] = (ys[r + 1] - ys[r - 1]) - penalty[s[r]]
- AAWL * (xs[d - 2] - xs[1]) - DDWL * distx[r];
- kAawl * (xs[d - 2] - xs[1]) - ddwl * distx[r];

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

Update usages of kAawl and ddwl to kAa and dd for consistency.

Suggested change
- kAawl * (xs[d - 2] - xs[1]) - ddwl * distx[r];
- kAa * (xs[d - 2] - xs[1]) - dd * distx[r];

Comment thread src/stt/src/flt/flute.cpp
} else {
score[nbp] = (ys[r + 1] - ys[r - 1]) - penalty[s[r]]
- BBWL * (xs[s[r] + 1] - xs[s[r] - 1]) - DDWL * distx[r];
- kBbwl * (xs[s[r] + 1] - xs[s[r] - 1]) - ddwl * distx[r];

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

Update usages of kBbwl and ddwl to kBb and dd for consistency.

Suggested change
- kBbwl * (xs[s[r] + 1] - xs[s[r] - 1]) - ddwl * distx[r];
- kBb * (xs[s[r] + 1] - xs[s[r] - 1]) - dd * distx[r];

@maliberty maliberty merged commit 48d687f into The-OpenROAD-Project:master Apr 20, 2026
17 checks passed
@maliberty maliberty deleted the stt-tidy-naming branch April 20, 2026 00:57
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