Report configuration syntax errors through the normal diagnostic by jinchuntan · Pull Request #331 · VLSIDA/OpenRAM · GitHub
Skip to content

Report configuration syntax errors through the normal diagnostic - #331

Merged
mguthaus merged 1 commit into
VLSIDA:devfrom
jinchuntan:fix/config-syntax-diagnostics
Oct 21, 2026
Merged

mguthaus merged 1 commit into
VLSIDA:devfrom
jinchuntan:fix/config-syntax-diagnostics

Conversation

@jinchuntan

Copy link
Copy Markdown

Syntax and indentation mistakes in config files now use OpenRAM's existing error reporting with the file and line number. This follows up on the review of #329.

Added tests for invalid syntax, errors in sibling imports and retrying a corrected config. All 11 configuration tests pass under each technology option.

The selected hardware checks produced the same results on dev and this branch with eight passes and three existing failures.

Selected hardware checks dev at dc5c04da This branch
Passed 8 8
Failed 3 3

The same three failures occur on both versions. The full regression suite has not been run.

@mguthaus
mguthaus merged commit e164e5b into VLSIDA:dev Oct 21, 2026
@mguthaus

Copy link
Copy Markdown
Collaborator

Merged. Thanks for picking this up — and for choosing the narrow fix rather than the broad one.

I had suggested except Exception as one option; catching SyntaxError specifically is better, because a config file is executable Python and an error raised by the user's own computation should not be reported as "unable to read the file". test_runtime_errors_are_not_reclassified is exactly the right guard for that boundary, and your three syntax cases cover the subclasses — IndentationError and TabError are both SyntaxError, which is why missing-indentation and mixed-indentation are caught too.

Verified

Reproduced the before/after on a config with a syntax error, dev (dc5c04d) as control:

dev this PR
raised SyntaxError, raw traceback AssertionError via debug.error
OpenRAM diagnostic no yes
names file and line no yes (badsyntax.py, line 3)
RuntimeError reclassified no no
cache clean after failure yes yes

Both trees leave _config_modules clean on failure and allow a retry after the file is fixed — that falls out of #329 putting the cache assignment after exec_module, and your test_configuration_can_be_retried_after_syntax_fix pins it so a future refactor cannot quietly break it.

Test run on freepdk45, scn4m_subm and sky130: 14 pass / 0 fail on both trees, and the new methods genuinely ran rather than being skipped — Ran 7 tests on dev versus Ran 11 tests here, in 0.021s.

Minor note

The copyright header bump on globals.py (2024 → 2026) is unrelated to the change and now makes that file inconsistent with the rest of the tree, which still says 2016-2024. Harmless, and not worth another round, but a repo-wide bump is better done as its own commit than piecemeal.

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.

2 participants