Skip to content
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

core: Remove runtime type check in Data #1126

Merged
merged 1 commit into from Jun 14, 2023

Conversation

math-fehr
Copy link
Collaborator

Previously, Data was expecting a verify method whenever the Data type parameter was not a class.
This is now removed, as we trust the user to respect the types.

@math-fehr math-fehr added the core xDSL core (ir, textual format, ...) label Jun 13, 2023
@math-fehr math-fehr marked this pull request as ready for review June 13, 2023 13:30
@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (ef4b54d) 87.21% compared to head (ef4be8d) 87.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1126      +/-   ##
==========================================
+ Coverage   87.21%   87.24%   +0.02%     
==========================================
  Files         142      142              
  Lines       21034    20997      -37     
  Branches     3169     3161       -8     
==========================================
- Hits        18345    18318      -27     
+ Misses       2177     2172       -5     
+ Partials      512      507       -5     
Impacted Files Coverage Δ
tests/test_attribute_definition.py 95.48% <ø> (-0.12%) ⬇️
xdsl/dialects/builtin.py 83.61% <100.00%> (ø)
xdsl/irdl.py 92.04% <100.00%> (+0.96%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@AntonLydike
Copy link
Collaborator

Notably, this made it impossible to have things like Data[tuple[int,...]], as PyRDL did not understand these constraints, but pyright would complain when the encapsulating type was left out (e.g. Data[tuple]).

@math-fehr
Copy link
Collaborator Author

Notably, this made it impossible to have things like Data[tuple[int,...]], as PyRDL did not understand these constraints, but pyright would complain when the encapsulating type was left out (e.g. Data[tuple]).

Actually, it was possible, you just needed to define verify. But that was an unnecessary and dumb restriction.

@AntonLydike AntonLydike merged commit 3da2ef7 into main Jun 14, 2023
12 checks passed
@AntonLydike AntonLydike deleted the fehr/remove-data-type-check branch June 14, 2023 07:16
cigarichard pushed a commit to cigarichard/xdsl that referenced this pull request Jun 17, 2023
Previously, `Data` was expecting a `verify` method whenever the `Data`
type parameter was not a class.
This is now removed, as we trust the user to respect the types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core xDSL core (ir, textual format, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants