Skip to content
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

PURE_PYTHON: Name check for setting attributes is applied to all classes, not just C classes #36

Open
jamadden opened this issue Oct 1, 2021 · 3 comments
Labels

Comments

@jamadden
Copy link
Member

jamadden commented Oct 1, 2021

(Tested with 4.5.1, but this goes back to the very oldest code in this repo.)

The Python version of ExtensionClass applies the check for special methods to all classes:

def __setattr__(self, name, value):
if name not in ('__get__', '__doc__', '__of__'):
if (name.startswith('__') and name.endswith('__') and
name.count('_') == 4):
raise TypeError(
"can't set attributes of built-in/extension type '%s.%s' "
"if the attribute name begins and ends with __ and "
"contains only 4 _ characters" %
(self.__module__, self.__name__))
return type.__setattr__(self, name, value)

But the intent in the C version is to only apply the check to other built-in types:

if (! (type->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
PyObject* as_bytes = convert_name(name);
if (as_bytes == NULL) {
return -1;
}
if (_is_bad_setattr_name(type, as_bytes)) {
Py_DECREF(as_bytes);
return -1;
}

This leads to a noticeable difference in PURE_PYTHON mode, such that code that works with the C extension fails in PURE_PYTHON. Compare the C version:

>>> os.environ.get("PURE_PYTHON")
None
>>> from ExtensionClass import Base
>>> class X(Base):
...     pass
...
>>> X.__eq__ = 'added by decorator'
>>>

to the Python version:

>>> os.environ['PURE_PYTHON'] = '1'
>>> from ExtensionClass import Base
>>> class X(Base):
...     pass
...
>>> X.__eq__ = 'added by decorator'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/python3.10/site-packages/ExtensionClass/__init__.py", line 214, in __setattr__
    raise TypeError(
TypeError: can't set attributes of built-in/extension type '__main__.X' if the attribute name begins and ends with __ and contains only 4 _ characters

I'm not sure how to reliably detect built-in classes in Python, so I'm not sure how to fix this?

@jamadden jamadden added the bug label Oct 1, 2021
@d-maurer
Copy link

d-maurer commented Oct 1, 2021 via email

@jamadden
Copy link
Member Author

jamadden commented Oct 1, 2021

instead, it is important whether it would be implemented in "C" for the "C" version.

I'm not sure I agree with that. This is all happening at runtime, and at runtime, you care about what (sub)classes you actually have, not what you theoretically might have at some other time.

I think I removed the restriction alltogether

That seems like a reasonable approach to me, especially if we just pass everything we don't specifically care about (__of__, etc) through to type.__setattr__: it can worry about whether or not some attribute should be writable and whether or not some class is a heap type or not.

@d-maurer
Copy link

d-maurer commented Oct 1, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants