Skip to content

WIP: Added default __and__ #183

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

Merged

Conversation

BojanKogoj
Copy link
Contributor

For all objects without defined and this should be a default error

For all objects without defined __and__ this should be a default error
@cthulahoops
Copy link
Collaborator

I don't think object should have an and.

>>> dir(object)
['__class__', '__delattr__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__']

I think, that for a & b we need to:

  1. Call a.__and__(b) if defined.
  2. Call b.__rand__(a) if defined.
  3. Otherwise raise a TypeError.

@BojanKogoj
Copy link
Contributor Author

Right now if object has __and__ implementation it will call it (for example objint), otherwise throws TypeError.
What you are suggesting is not to implement default __and__ at all? That means I would have to check if __and__ and __rand__ exist some other way, not sure how

@windelbouwman
Copy link
Contributor

windelbouwman commented Oct 29, 2018

You can use obj.has_attr("__and__") and modify the _and function in vm.rs. I mean to say that the logic for checking __and__ and __rand__ should be implemented in the vm, not in the methods themselves.

@windelbouwman
Copy link
Contributor

I added the discussed logic in for substraction: https://github.com/RustPython/RustPython/blob/master/vm/src/vm.rs#L380

@windelbouwman windelbouwman merged commit 9ba9ca1 into RustPython:master Nov 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants