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

Fix del builtin #355

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix del builtin #355

wants to merge 2 commits into from

Conversation

jayvdb
Copy link
Member

@jayvdb jayvdb commented Aug 3, 2018

The recent builtin rewrite had one regression, in that it allowed them to be deleted.

The first commit fixes that, but there are several builtins which can be deleted.

The second commit allows those builtins to be deleted without an UndefinedName being reported , which is an enhancement.

This also adds a test rig for determining which builtins can be deleted.

@jayvdb jayvdb changed the title Fix del builtin regression DNM: Fix del builtin regression Aug 3, 2018
@jayvdb jayvdb changed the title DNM: Fix del builtin regression Fix del builtin regression Aug 3, 2018
@jayvdb jayvdb changed the title Fix del builtin regression Fix del builtin Aug 3, 2018
@jayvdb jayvdb requested a review from myint August 3, 2018 16:49
@@ -185,6 +185,16 @@ class Builtin(Definition):

def __init__(self, name):
super(Builtin, self).__init__(name, None)
self.can_delete = False
# __file__ can only be deleted on Python 3
if not PY2 and name == '__file__':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do verify if __file__ can be deleted or not in Python 2.

@jayvdb jayvdb force-pushed the del-builtin branch 2 times, most recently from d5ffe4d to ed63c89 Compare August 11, 2018 10:36
An exception to the handling of builtins is they can not be
deleted.
__builtins__, __file__ and __debug__ can be deleted.

Adds a test runner that ensures defined builtins can be
loaded, and can be deleted when the python runtime allows it.
@@ -185,6 +185,13 @@ class Builtin(Definition):

def __init__(self, name):
super(Builtin, self).__init__(name, None)
self.can_delete = False
# __builtins__ and __file__ can always be deleted.
# __debug__ can be deleted sometimes and not deleted other times.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please elaborate on this, it is failing for me.

surfer-172-20-4-99-hotspot:Desktop macbox$ python py-test.py 
Traceback (most recent call last):
  File "py-test.py", line 1, in <module>
    del __debug__
NameError: name '__debug__' is not defined
surfer-172-20-4-99-hotspot:Desktop macbox$ python3 py-test.py 
Traceback (most recent call last):
  File "py-test.py", line 1, in <module>
    del __debug__
NameError: name '__debug__' is not defined

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my Python 3.7 (Rawhide), __debug__ can be deleted.
On earlier Python, locally, it isnt.
However it might come down to whether the Python build had debug enabled in it.
We might need to dig into the CPython source to find out, as I dont see any mention of this in peps

# __debug__ can be deleted sometimes and not deleted other times.
# Safest course of action is to assume it can be deleted, in
# order that no error is reported by pyflakes
elif name in ('__builtins__', '__file__', '__debug__'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elif should be if.

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.

None yet

3 participants