core: Implement RangeVarConstraint #3261
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3261 +/- ##
==========================================
+ Coverage 89.99% 90.01% +0.01%
==========================================
Files 445 445
Lines 55749 55848 +99
Branches 5367 5371 +4
==========================================
+ Hits 50173 50271 +98
- Misses 4166 4169 +3
+ Partials 1410 1408 -2 ☔ View full report in Codecov by Sentry. |
|
Would be nice to get the commit attribution correct as this is still mainly @PapyChacal 's work, I just made pyright happy and put some match statements on the tests |
|
I edited the description, which should propagate authorship when the PR is merged |
There was a problem hiding this comment.
Would be great to have @math-fehr and @PapyChacal's ticks before merging
xdsl/irdl/constraints.py
Outdated
| ranges: dict[str, tuple[Attribute, ...]] = field(default_factory=dict) | ||
| """The assignment of constraint range variables.""" |
There was a problem hiding this comment.
Should we somehow check somewhere that the variable and range variables have different names?
Otherwise I think the can_infer and infer funcitons might be incorrect if someone have twice the same name for range variable and variable.
If it's too annoying to check, jsut make an issue for it, as I don't think it's actually gonna happen in practice.
There was a problem hiding this comment.
I've put this in as the last commit, can revert it if it is deemed too messy
|
@superlopuh @math-fehr any thoughts on whether to keep the last commit? |
|
IDK, I'd rather add helpers to add and get variables on the original objects, and keep all the logic to one class, from an architecture point of view there's some dubious coupling going on there, it would nice for the unicity logic to be encapsulated. |
|
Sure, I'll do that |
Co-authored-by: Fehr Mathieu <mathieu.fehr@gmail.com>
alexarice
force-pushed
the
alexarice/range-variable
branch
from
d4323c1 to
67298d7
Compare
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
I removed the last commit, implemented it as @superlopuh suggested. In the process of this I rebased past the pyright changes and now had to fix some other stuff. I think this is because I generalised |
Continuation of #2704
Co-authored-by: Emilien Bauer papychacal@gmail.com