core: (irdl) Add support for regions in optional groups to declarative assembly format #3106
Conversation
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
…/github.com/xdslproject/xdsl into kim/declarative-assembly-parser/add-regions
…s' into kim/declarative-assembly-parser/add-regions. Remove casts.
…tests for optional groups with regions
kimxworrall
added
minor
kimxworrall
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3106 +/- ##
========================================
Coverage 89.91% 89.91%
========================================
Files 419 421 +2
Lines 53070 53319 +249
Branches 8226 8267 +41
========================================
+ Hits 47718 47942 +224
- Misses 4023 4039 +16
- Partials 1329 1338 +9 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
alexarice
changed the title
|
Perhaps you and @superlopuh have discussed this in person but would it be possible to describe what wasn't working before/brief summary of what you've changed, as there seems to be lots of changes in the "files changed section" |
| ): | ||
| case VariadicLikeVariable(), VariadicLikeVariable(): | ||
| if not ( | ||
| isinstance(a, RegionDirective, VariadicLikeTypeDirective) |
There was a problem hiding this comment.
My bad
| isinstance(a, RegionDirective, VariadicLikeTypeDirective) | |
| isinstance(a, RegionDirective | VariadicLikeTypeDirective) |
|
This commit adds a RegionDirective that all RegionVariables inherit from. When sequences of variables in declarative assembly format are checked, if two consecutive variables are optional (either two optional operands or two optional regions in a row, but not one optional region and one optional operand), an error is raised to inidicate ambiguity. test_optional_groups_regions is modified to catch this error, and two other tests are made cleaner as hang overs from the previous pr on this branch. |
|
Is this ready to merge? |
|
Yeah seems ready to me, feel free to hit the merge button at your convenience :) @kimxworrall |
Added support for regions in optional groups and tests for this.