misc: add more .constr helpers and constraints #3273
Conversation
| @@ -1559,7 +1562,7 @@ class OptSuccessorOp(IRDLOperation): | |||
| # Inference # | |||
| ################################################################################ | |||
|
|
|||
| _T = TypeVar("_T", bound=Attribute) | |||
| _T = TypeVar("_T", bound=Attribute, covariant=True) | |||
There was a problem hiding this comment.
Necessary for constr definition below, that way BaseAttr[ParamOne[_T]] is a subclass of BaseAttr[ParamOne[Attribute]].
| @@ -1436,7 +1457,7 @@ def print(self, printer: Printer) -> None: | |||
| f128 = Float128Type() | |||
|
|
|||
|
|
|||
| _MemRefTypeElement = TypeVar("_MemRefTypeElement", bound=Attribute) | |||
| _MemRefTypeElement = TypeVar("_MemRefTypeElement", bound=Attribute, covariant=True) | |||
There was a problem hiding this comment.
this change was made to appease Pyright below, but I'm not really sure that it's kosher... My understanding is that, even though a memref is mutable, and hence a memref type parameter of the memref itself should be covariant, a memref constraint is immutable, so a memref constraint that only accepts cats should be ok to give whenever the client expects a constraint that only accepts animals. Does this seem right? Am I missing something?
There was a problem hiding this comment.
If you can store stuff in the memref then this is technically unsound I believe
There was a problem hiding this comment.
Actually, I can't even really figure out why this type is generic at all?
There was a problem hiding this comment.
This is the type not the memref itself. I am now pretty convinced that it's sound, just because all attributes and attribute constraints are immutable, so it makes sense to have their type parameters be covariant. It is not really related to the type of the objects in the IR, as far as the Python type system is concerned.
There was a problem hiding this comment.
Looking through the uses of MemRefType, I'm pretty sure this generic argument serves no function whatsoever
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3273 +/- ##
=======================================
Coverage ? 89.98%
=======================================
Files ? 444
Lines ? 55615
Branches ? 5350
=======================================
Hits ? 50045
Misses ? 4172
Partials ? 1398 ☔ View full report in Codecov by Sentry. |
Pylance errors: 84 -> 74.
Part of #3264