You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Report configuration syntax errors through the normal diagnostic - #331
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
devatdc5c04daThe same three failures occur on both versions. The full regression suite has not been run.