This diff adds casting for Attributes similar to that of Types.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Awesome, just some minor comments.
There may be some comments to update as well, like https://github.com/llvm/llvm-project/blob/9ad3ca4e9a310c1507578b1c8165cd61a9a45037/mlir/include/mlir-c/Bindings/Python/Interop.h#L112
mlir/lib/Bindings/Python/IRAttributes.cpp | ||
---|---|---|
415 | maybe a comment here explaining why getTypeIdFunction isn't used? | |
mlir/lib/Bindings/Python/IRCore.cpp | ||
3195–3198 | can this ever happen? below it's just an assert | |
mlir/test/python/ir/builtin_types.py | ||
404–408 ↗ | (On Diff #528682) | returning None here might be a better API? I don't have any strong opinion, but it'd be more consistent with PyRankedTensorType.encoding as well |
update comments
mlir/lib/Bindings/Python/IRCore.cpp | ||
---|---|---|
3195–3198 | if you get to the caster then you definitely have a typeid (otherwise there wouldn't have been a match for a/some key in the typecaster densemap) but here you might be able to call typeid on some strange attribute. |
mlir/include/mlir/Bindings/Python/PybindAdaptors.h | ||
---|---|---|
418–419 | just wanted to point out how descriptive this comment string is 😂 |
mlir/lib/Bindings/Python/IRCore.cpp | ||
---|---|---|
3195–3198 |
Only case I can think of is if you called mlirAttributeGetTypeID on a null attribute, but in that case it looks like the call would crash instead of returning a null TypeID |
mlir/lib/Bindings/Python/IRCore.cpp | ||
---|---|---|
3195–3198 | i double checked and right you are - it segfaults. so i did as you imply and changed to an assert. |
just wanted to point out how descriptive this comment string is 😂