Support for arbitrary decorators on methods of cdef classes #3966
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also fix issue due to change to calling convention
(but with the property-related changes cut out)
c0f5ce0 to
965e596
Compare
mattip
reviewed
Feb 9, 2021
serge-sans-paille/pythran#1706 - it's unrelated to this |
da-woods
added a commit
to da-woods/cython
that referenced
this pull request
Feb 27, 2021
scoder
reviewed
May 24, 2021
Cython/Compiler/Nodes.py
Outdated
Comment on lines
890
to
897
| @property | ||
| def type_set_manually(self): | ||
| if self._type_set_manually: | ||
| return True | ||
| return bool(getattr(self.base_type, "name", False)) | ||
| @type_set_manually.setter | ||
| def type_set_manually(self, value): | ||
| self._type_set_manually = value |
There was a problem hiding this comment.
This feels like a work-around for some other place in the code that isn't smart enough to determine a type correctly.
Do you see a way to do this differently, without such a flag?
There was a problem hiding this comment.
I've replaced it with a flag stored on FuncDefNode which is probably the right place to store this information.
(Edit: doesn't work... I'll have another look at this...) works now
Removed the property of the type in favour of keeping track of this for FuncDefNode
The pure Python behaviour has changed so the tests need to capture that.
(and run if-statement only once)
…arb_decorators_standalone
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of the attempt to split #3383 into more reasonable pieces. This includes #3961 by necessity (so will look a bit smaller if that is merged ahead of this PR)
Relax some restriction on cdef class staticmethods/classmethods
Where it isn't able to do the efficient thing it falls back to calling the builtin method staticmethod/classmethod.
staticmethod/classmethod are only special-cased when the decorator is innermost. Fixes Combining @staticmethod with other decorators is broken #1434
Fixes Decorated methods in cdef classes should be ordinary functions #1274
Where a method is arbitrarily decorated we don't really know the type of the
selfargument - it's probably just the type of thecdef class, but it could be anything else. Therefore we produce a warning with the suggestion to type the self argument appropriately. This is hopefully fairly intuitive, preserves speed for the common case while allowing the less common case to be supported.Additionally adds a compiler option
fused_types_arbitrary_decorators. With this enabled we produce a fused function with theselftype either that of thecdef class, orobject. This option supports both cases automatically but is off by default (since it's potentially an unexpected change). This would be mostly transparent to the user, but if they look hard they will be able to tell it's afusedfunction, mostly because the function has__getitem__defined.I suspect it fixes
classmethod is unable to decorate a callable that is function (and not method) #3072
class-level classmethod() can only be called on a method_descriptor or instance method. #3044
but the associated test-cases were large and fairly confusing