-
Notifications
You must be signed in to change notification settings - Fork 840
Hrw: Supports nested If #12562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Hrw: Supports nested If #12562
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for nested if/elif/else conditionals in the header rewrite system by refactoring the conditional logic into a new OperatorIf
pseudo-operator. The changes enable arbitrary nesting depth for complex conditional logic while maintaining backward compatibility.
Key changes:
- Introduced
OperatorIf
class to encapsulate conditional evaluation logic - Added
if
/endif
keywords to enable nested conditional blocks - Implemented explicit slot assignment syntax for variables (e.g.,
@7
) in hrw4u - Updated grammar files and parsers to support new syntax
Reviewed Changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
tools/hrw4u/grammar/hrw4u.g4 | Added AT token and optional slot syntax to variable declarations |
tools/hrw4u/grammar/u4wrh.g4 | Added IF_OP and ENDIF_OP tokens and corresponding grammar rules |
tools/hrw4u/src/visitor.py | Updated to handle explicit slot assignments and nested conditionals |
tools/hrw4u/src/symbols.py | Refactored slot allocation to support explicit slot assignment |
tools/hrw4u/src/types.py | Renamed index to slot for clarity |
tools/hrw4u/src/hrw_visitor.py | Added support for if/endif operators and fixed nesting depth tracking |
tools/hrw4u/scripts/testcase.py | Added exceptions mechanism to skip direction-specific tests |
plugins/header_rewrite/ruleset.h | Refactored to delegate conditional logic to OperatorIf |
plugins/header_rewrite/ruleset.cc | Simplified execution to use OperatorIf::exec_and_return_mods |
plugins/header_rewrite/operators.h | Added OperatorIf class and OperatorAndMods struct |
plugins/header_rewrite/operators.cc | Implemented OperatorIf conditional evaluation logic |
plugins/header_rewrite/operator.h | Moved OperatorAndMods to operator.h for shared use |
plugins/header_rewrite/parser.h | Added IF and ENDIF clause types |
plugins/header_rewrite/parser.cc | Added parsing support for if/endif keywords |
plugins/header_rewrite/header_rewrite.cc | Implemented if/endif stack-based parsing with nesting validation |
doc/admin-guide/plugins/header_rewrite.en.rst | Added comprehensive documentation for nested conditionals |
doc/admin-guide/configuration/hrw4u.en.rst | Documented explicit slot assignment syntax |
tests/gold_tests/pluginTest/header_rewrite/* | Added test cases for nested conditionals |
tools/hrw4u/tests/data/* | Added test cases for explicit slots and updated error messages |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Ignored the nitpick comments, the current code is inline with other uses within the code base. Consistency is better. |
This refactors the parsing / evaluation, to put most of the exec / evaluation logic into
a new OperatorIf. This is not a real operator, that configurations would use, but allows
for the conditionals to be nested. The syntax in HRW is not awesome, but best we can
do within the constraints of backwards compatibility.
As such, this PR also includes changes and additions to hrw4u, where nested if's are
now supported and added to the existing if() / else semantics. E.g.