Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Allow for builtin slots to be overriden #71

Open
meadori opened this issue Jan 9, 2017 · 4 comments
Open

Allow for builtin slots to be overriden #71

meadori opened this issue Jan 9, 2017 · 4 comments

Comments

@meadori
Copy link
Contributor

meadori commented Jan 9, 2017

Consider the following program:

class T(object):
   def __repr__(self):
      return "__repr__" 

def foo(self):
   return "foo"

a = T()

print a.__repr__
print repr(a)

T.__repr__ = foo

print a.__repr__
print repr(a)

And the following execution of CPython and Grumpy:

$ python --version
Python 2.7.10
$ python bar.py   
<bound method T.__repr__ of __repr__>
__repr__
<bound method T.foo of foo>
foo
$ ./tools/grumpc bar.py > bar.go 
$ go run bar.go 
<bound method T.__repr__ of __repr__>
__repr__
<bound method T.foo of __repr__>
__repr__

CPython allows for the slot method to be overriden at runtime. For the builtins where this applies (which can be determined from Objects/typeobject.c in the CPython source), Grumpy should lookup the method by name.

That being said, I don't know how common this is in real code. I noticed it reading the CPython source while implementing __cmp__.

@trotterdylan
Copy link
Contributor

trotterdylan commented Jan 9, 2017

Thanks for the report! This is something that I've fretted a bit about. The real trick is that you have to update the slot on all the subclasses. Here's how CPython does it. Unfortunately, whereas CPython holds the GIL while it does all this work, Grumpy must find some other way to synchronize it, or live with poor consistency guarantees around updating types.

Worse is when you assign to a type's __bases__ attribute. This is not something I can ever imagine a reasonable program doing, but CPython jumps through hoops to support it.

Personally I think it's crazy to support this kind of monkey patching and I'm inclined to raise an exception when people try things like this. But maybe there are legitimate use cases that I'm overlooking. Thoughts and opinions are welcome here.

@ns-cweber
Copy link
Contributor

ns-cweber commented Jan 9, 2017

I'm all for raising until a reasonable use case presents itself. At least as long as grumpy is in alpha/beta.

@alanjds
Copy link
Contributor

alanjds commented Jan 20, 2017

I would love to monkeypatch datetime.datetime.now() to be allowed to test time in my billing code...

https://bugs.python.org/issue9528

@trotterdylan
Copy link
Contributor

You're right that tests are a potentially valid use case for that. Note that normal functions (ones that reside in type(obj).__dict__) can be monkey patched perfectly well. The issue is just the slot functions that don't work quite right, like __add__.

cclauss pushed a commit to cclauss/grumpy that referenced this issue Aug 31, 2018
cclauss pushed a commit to cclauss/grumpy that referenced this issue Sep 1, 2018
Travis CI: Simplify using ${SUDO} and ${USER_FLAG}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants