[JIT] Add support for del to TS classes #44352
Conversation
**Summary** This commit adds support for `del` with class instances. If a class implements `__delitem__`, then `del class_instance[key]` is syntactic sugar for `class_instance.__delitem__[key]`. **Test Plan** This commit adds a unit test to TestClassTypes to test this feature. [ghstack-poisoned]
**Summary** This commit adds support for `del` with class instances. If a class implements `__delitem__`, then `del class_instance[key]` is syntactic sugar for `class_instance.__delitem__[key]`. **Test Plan** This commit adds a unit test to TestClassTypes to test this feature. ghstack-source-id: 840e127b58f312fe61c735826b88b01ef0ef74e6 Pull Request resolved: #44352
facebook-github-bot
added
the
oncall: jit
💊 CI failures summary and remediationsAs of commit 7ed071e (more details on the Dr. CI page):
ci.pytorch.org: 1 failedThis comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 1 time. |
|
Super excited for this, thanks a lot @SplitInfinity! |
Codecov Report
@@ Coverage Diff @@
## gh/splitinfinity/42/base #44352 +/- ##
=========================================================
Coverage 69.24% 69.24%
=========================================================
Files 381 381
Lines 47575 47575
=========================================================
+ Hits 32944 32945 +1
+ Misses 14631 14630 -1
Continue to review full report at Codecov.
|
| // If val is a class instance, this is a method call to a type-specific | ||
| // implementation of del defined in a __delitem__ method. | ||
| if (auto cls = val->type()->cast<ClassType>()) { | ||
| if (!cls->findMethod("__delitem__")) { |
There was a problem hiding this comment.
If a method doesn't define __delitem__, we should just defer to the normal environment_stack->removeVar(var.name(), /*check_if_removed=*/true); behavior
There was a problem hiding this comment.
Hm, I thought this code path is for Subscript expressions (e.g. del variable[key], where variable is a class, list or dict). If variable is a class but does not define __delitem__, this produces an error in Python too:
>>> class A:
... def __init__(self):
... pass
...
>>> a = A()
>>> del a["key"]
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: 'A' object does not support item deletion
There was a problem hiding this comment.
Oh word, yea i didn't realize this was within the subscript branch. Yea lgtm
|
@SplitInfinity merged this pull request in d3b6d5c. |
Stack from ghstack:
Summary
This commit adds support for
delwith class instances. If a classimplements
__delitem__, thendel class_instance[key]is syntacticsugar for
class_instance.__delitem__[key].Test Plan
This commit adds a unit test to TestClassTypes to test this feature.
Differential Revision: D23603102