-
-
Notifications
You must be signed in to change notification settings - Fork 32k
BUILTINS
is not a valid field name for a frozen dataclass
#96151
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
Comments
hauntsaninja
added a commit
to hauntsaninja/cpython
that referenced
this issue
Oct 10, 2022
There's no indication that BUILTINS is a special name. Other names that are special to dataclass are all prefixed by an underscore. As mentioned in the issue, we can also avoid this locals dance altogether by using `().__class__.__base__` instead of `BUILTINS.object`.
ericvsmith
pushed a commit
that referenced
this issue
Oct 31, 2022
…now allows for a field named BUILTIN (gh-98143)
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this issue
Oct 31, 2022
… This now allows for a field named BUILTIN (pythongh-98143) (cherry picked from commit 29f98b4) Co-authored-by: Shantanu <[email protected]>
This was referenced Oct 31, 2022
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this issue
Oct 31, 2022
… This now allows for a field named BUILTIN (pythongh-98143) (cherry picked from commit 29f98b4) Co-authored-by: Shantanu <[email protected]>
ericvsmith
pushed a commit
that referenced
this issue
Oct 31, 2022
…. This now allows for a field named BUILTIN (gh-98143) (gh-98899) gh-96151: Use a private name for passing builtins to dataclass. This now allows for a field named BUILTIN (gh-98143) (cherry picked from commit 29f98b4) Co-authored-by: Shantanu <[email protected]> Co-authored-by: Shantanu <[email protected]>
ericvsmith
pushed a commit
that referenced
this issue
Oct 31, 2022
…. This now allows for a field named BUILTIN (gh-98143) (gh-98900) gh-96151: Use a private name for passing builtins to dataclass. This now allows for a field named BUILTIN (gh-98143) (cherry picked from commit 29f98b4) Co-authored-by: Shantanu <[email protected]> Co-authored-by: Shantanu <[email protected]>
Merge and backports complete. Thanks, @hauntsaninja ! |
hauntsaninja
added a commit
to hauntsaninja/cpython
that referenced
this issue
Feb 18, 2023
This commit prefixes `__dataclass` to several things in the locals dict: - Names like _dflt_ (which cause trouble, see first test) - Names like _type_ (not known to be able to cause trouble) - _return_type (not known to able to cause trouble) - _HAS_DEFAULT_FACTORY (which causes trouble, see second test) In addition, this removes `MISSING` from the locals dict. As far as I can tell, this wasn't needed even in the initial implementation of dataclasses.py (and tests on that version passed with it removed). This is basically a continuation of python#96151, where fixing this was welcomed in python#98143 (comment)
hauntsaninja
added a commit
that referenced
this issue
Mar 25, 2023
…mes (#102032) This commit prefixes `__dataclass` to several things in the locals dict: - Names like `_dflt_` (which cause trouble, see first test) - Names like `_type_` (not known to be able to cause trouble) - `_return_type` (not known to able to cause trouble) - `_HAS_DEFAULT_FACTORY` (which causes trouble, see second test) In addition, this removes `MISSING` from the locals dict. As far as I can tell, this wasn't needed even in the initial implementation of dataclasses.py (and tests on that version passed with it removed). This makes me wary :-) This is basically a continuation of #96151, where fixing this was welcomed in #98143 (comment)
Fidget-Spinner
pushed a commit
to Fidget-Spinner/cpython
that referenced
this issue
Mar 27, 2023
…ore names (python#102032) This commit prefixes `__dataclass` to several things in the locals dict: - Names like `_dflt_` (which cause trouble, see first test) - Names like `_type_` (not known to be able to cause trouble) - `_return_type` (not known to able to cause trouble) - `_HAS_DEFAULT_FACTORY` (which causes trouble, see second test) In addition, this removes `MISSING` from the locals dict. As far as I can tell, this wasn't needed even in the initial implementation of dataclasses.py (and tests on that version passed with it removed). This makes me wary :-) This is basically a continuation of python#96151, where fixing this was welcomed in python#98143 (comment)
warsaw
pushed a commit
to warsaw/cpython
that referenced
this issue
Apr 11, 2023
…ore names (python#102032) This commit prefixes `__dataclass` to several things in the locals dict: - Names like `_dflt_` (which cause trouble, see first test) - Names like `_type_` (not known to be able to cause trouble) - `_return_type` (not known to able to cause trouble) - `_HAS_DEFAULT_FACTORY` (which causes trouble, see second test) In addition, this removes `MISSING` from the locals dict. As far as I can tell, this wasn't needed even in the initial implementation of dataclasses.py (and tests on that version passed with it removed). This makes me wary :-) This is basically a continuation of python#96151, where fixing this was welcomed in python#98143 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is not a bug that I encountered in the wild, but I happened to notice the possibility for this while reading the source.
will give you:
There are other field names that you can use to trick dataclass into bad behaviour, but all of the other ones are prefixed by an underscore.
So one "fix" is pretty simple: just rename
BUILTINS
to_BUILTINS
in dataclasses.py to bring it in line with the others.If a single underscore doesn't feel like sufficient "this is the user's fault", we could use
__dataclass_*
(like we do for__dataclass_self__
param to__init__
)For what it's worth, the only thing we need
BUILTINS
for is so we can accessobject
inside__init__
, so an alternative fix could be to use().__class__.__base__
instead ofBUILTINS.object
(and skip the locals dance needed to defineBUILTINS
altogether)If any of that sounds worth doing, I'd be happy to open a PR
The text was updated successfully, but these errors were encountered: