Skip to content

Catch misspelled / wrong-stage env var references in flow scripts#4191

Closed
oharboe wants to merge 1 commit intoThe-OpenROAD-Project:masterfrom
oharboe:spell-check
Closed

Catch misspelled / wrong-stage env var references in flow scripts#4191
oharboe wants to merge 1 commit intoThe-OpenROAD-Project:masterfrom
oharboe:spell-check

Conversation

@oharboe
Copy link
Copy Markdown
Collaborator

@oharboe oharboe commented Apr 26, 2026

Add a typo guard in util.tcl: env_var_equals,
env_var_exists_and_non_empty, env_var_or_empty, append_env_var, and source_env_var_if_exists now error if the variable name is not declared in variables.yaml for the current stage. erase_non_stage_variables populates the per-stage declared set; utility scripts that don't run it (e.g. open.tcl) fall back to the full set.

Add declared_variables.py to emit the per-stage declared set as a sibling of non_stage_variables.py.

Declare 6 helper-referenced vars that were missing from variables.yaml: DEF_FILE, V_FILE, LIB_MODEL, RTLMP_RPT_FILE, RTLMP_BLOCKAGE_FILE, SDC_FILE_CLOCK_PERIOD.

Widen FOOTPRINT and FOOTPRINT_TCL to stages [floorplan, place]. Fixes a latent bug: io_placement.tcl and global_place.tcl read them in the place stage but they were declared floorplan-only and so were always silently empty after erase_non_stage_variables.

Regenerate variables.json and docs/user/FlowVariables.md via yaml_to_json.py and generate-variables-docs.py.

Add a typo guard in util.tcl: env_var_equals,
env_var_exists_and_non_empty, env_var_or_empty, append_env_var, and
source_env_var_if_exists now error if the variable name is not declared
in variables.yaml for the current stage. erase_non_stage_variables
populates the per-stage declared set; utility scripts that don't run it
(e.g. open.tcl) fall back to the full set.

Add declared_variables.py to emit the per-stage declared set as a
sibling of non_stage_variables.py.

Declare 6 helper-referenced vars that were missing from variables.yaml:
DEF_FILE, V_FILE, LIB_MODEL, RTLMP_RPT_FILE, RTLMP_BLOCKAGE_FILE,
SDC_FILE_CLOCK_PERIOD.

Widen FOOTPRINT and FOOTPRINT_TCL to stages [floorplan, place]. Fixes a
latent bug: io_placement.tcl and global_place.tcl read them in the
place stage but they were declared floorplan-only and so were always
silently empty after erase_non_stage_variables.

Regenerate variables.json and docs/user/FlowVariables.md via
yaml_to_json.py and generate-variables-docs.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Copy link
Copy Markdown

@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 implements a typo guard for ORFS variables by validating environment variable lookups against declarations in variables.yaml. It introduces a Python script to fetch valid variables for specific stages and updates Tcl utility functions to enforce these checks. Additionally, several new variables are added and documented. Feedback was provided to improve the robustness of the Tcl execution logic by explicitly calling the Python interpreter and ensuring the output is correctly parsed as a list.

Comment thread flow/scripts/util.tcl
Comment on lines +121 to +125
set cmd [list $::env(SCRIPTS_DIR)/declared_variables.py]
if { $stage_name ne "" } {
lappend cmd $stage_name
}
set ::orfs_declared_vars [exec {*}$cmd]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The current implementation of orfs_load_declared_vars relies on the script being executable and correctly handled by the shell. For better robustness and portability across different environments, it is recommended to explicitly call python3 and use file join for path construction. Additionally, using split and string trim on the exec output ensures that the resulting variable list is clean and correctly formatted as a Tcl list, avoiding potential issues with trailing newlines or platform-specific line endings.

  set cmd [list python3 [file join $::env(SCRIPTS_DIR) declared_variables.py]]
  if { $stage_name ne "" } {
    lappend cmd $stage_name
  }
  set ::orfs_declared_vars [split [string trim [exec {*}$cmd]] "\n"]

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Apr 26, 2026

leave be for now

@oharboe oharboe closed this Apr 26, 2026
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.

1 participant