-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-58749: Removes the erroneous explanation regarding the restrictions of the global statement #126523
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
Conversation
… a name declared as global can appear later as a target in :keyword:with statements, :keyword:except clauses, :keyword:for target lists, :keyword:class definitions, function definitions, :keyword:import statements.
As a side note, in #13232, @andresdelfino addressed the same section. However, it did not correct the erroneous explanation noted in #58749; it merely added cases to the existing list of name usages, which was already errorneous. |
Have you noticed the tests I added in the PR? Because I ran them today, with Python 3.13.0, and they still fail. |
@andresdelfino Yes! I've seen all the cases Since this works, it's not accurate to describe that names listed in a :keyword:
Could you let me know if i'm missing something here? |
Something that has changed since I wrote that post back in 2012 is the rise of type hints and static type checking. Even if CPython isn't enforcing the name binding restrictions in the paragraph being modified here, it may be that |
@ncoghlan thanks for noting an important point. I've checked if mypy prevents certain usages of the same names after a global declaration. I've found no mention of such things in the documentation. Also, the following code passed the latest stable mypy (1.13.0)
If you wish me to conduct more research on another type hints and static type checking tool, I would be happy to do that. However, I personally think, even if type checking tools prevent using the same names after a global declaration, it may not be a huge problem, since they are supposed to impose restrictions that the language spec does not have anyway. The main problem this PR deals with is that there is a discrepancy btw the actual CPython implementation and the language spec document. I think it's better to get rid of the discrepancy either by changing the docs, or changing the compiler. |
Thanks for running that check, @bombs-kim. However, several of those checks aren't using their respective Re-reading the comments on the original issue, this change should also include a test suite addition (in https://github.com/python/cpython/blob/main/Lib/test/test_scope.py so the Something like the following (using https://docs.python.org/3/reference/executionmodel.html#naming-and-binding to build the list of name binding operations to be checked): import contextlib
from types import SimpleNamespace
class TestGlobalNameBinding:
# Ensure binding global names works as expected.
# Genuine scope conflicts (such as names being both local and
# nonlocal, or local and global) are covered in the compiler tests.
# Note: does not check `async def`, `async for`, or `async with`.
def test_assignment_statement(self):
global _global_assignment_statement
value = object()
_global_assignment_statement = value
self.assertIs(globals()["_global_assignment_statement"], value)
del _global_assignment_expression
def test_unpacking_assignment(self):
global _global_unpacking_assignment
value = object()
_, _global_unpacking_assignment = [None, value]
self.assertIs(globals()["_global_unpacking_assignment"], value)
del _global_unpacking_assignment
def test_assignment_expression(self):
global _global_assignment_expression
value = object()
if (_global_assignment_expression := value):
pass
self.assertIs(globals()["_global_assignment_expression"], value)
del _global_assignment_expression
def test_iteration_variable(self):
global _global_iteration_variable
value = object()
for _global_iteration_variable in [value]:
pass
self.assertIs(globals()["_global_iteration_variable"], value)
del _global_iteration_variable
def test_func_def(self):
global _global_func_def
def _global_func_def():
pass
value = _global_func_def
self.assertIs(globals()["_global_func_def"], value)
del _global_func_def
def test_class_def(self):
global _global_class_def
class _global_class_def():
pass
value = _global_class_def
self.assertIs(globals()["_global_class_def"], value)
del _global_class_def
def test_type_alias(self):
global _global_type_alias
type _global_type_alias = tuple[int, int]
value = _global_type_alias
self.assertIs(globals()["_global_type_statement"], value)
del _global_type_alias
def test_caught_exception(self):
global _global_caught_exc
try:
1/0
except ZeroDivisionError as _global_caught_exc:
value = _global_caught_exc
self.assertIs(globals()["_global_caught_exc"], value)
del _global_caught_exc
def test_caught_exception_group(self):
global _global_caught_exc_group
try:
try:
1/0
except ZeroDivisionError as exc:
raise ExceptionGroup("eg", [exc])
except* ZeroDivisionError as _global_caught_exc_group:
value = _global_caught_exc_group
self.assertIs(globals()["_global_caught_exc_group"], value)
del _global_caught_exc
def test_enter_result(self):
global _global_enter_result
value = object()
with contextlib.nullcontext(value) as _global_enter_result:
pass
self.assertIs(globals()["_global_enter_result"], value)
del
8000
_global_enter_result
def test_import_result(self):
global _global_import_result
value = contextlib
import contextlib as _global_import_result
self.assertIs(globals()["_global_import_result"], value)
del _global_import_result
def test_match(self):
global _global_match
value = object()
match value:
case _global_match:
pass
self.assertIs(globals()["_global_match"], value)
del _global_match
def test_match_as(self):
global _global_match_as
value = object()
match value:
case _ as _global_match_as:
pass
self.assertIs(globals()["_global_match_as"], value)
del _global_match_as
def test_match_seq(self):
global _global_match_seq
value = object()
match (None, value):
case (_, _global_match_seq):
pass
self.assertIs(globals()["_global_match_seq"], value)
del _global_match_seq
def test_match_map(self):
global _global_match_map
value = object()
match {"key": value}:
case {"key": _global_match_map}:
pass
self.assertIs(globals()["_global_match_map"], value)
del _global_match_map
def test_match_attr(self):
global _global_match_attr
value = object()
match SimpleNamespace(key=value):
case SimpleNamespace(key=_global_match_attr):
pass
self.assertIs(globals()["_global_match_attr"], value)
del _global_match_attr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking comment from the original issue discussion: the spec wording change should be accompanied by a test suite update that ensures the revised wording remains accurate.
For closing the original issue, the nonlocal
wording also needs to be updated, but that can be done in a separate PR after the global
change is resolved.
There's also a process question that I need to follow up before this can be merged (the level of review needed for language spec changes is generally higher than it was back when I first filed the linked issue, even when the spec change is fixing an error in the spec)
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
In relation to the "global after use" error reporting behaviour, that's covered in https://github.com/python/cpython/blob/main/Lib/test/test_global.py . There is currently no So while I suggested using
The biggest advantage of this would be test discoverability, since the tests for these statements would be in files specifically named for them. The headers of each statement-specific test module could then refer off to Edit to clarify: The reason statement order matters in the "global after use" and "nonlocal after use" cases is because the initial usage already implicitly declared the symbol as being local, making the later declaration as a global (or nonlocal) invalid for the same reason you can't declare function arguments as global (or nonlocal) - every symbol in an optimised scope has to be local, nonlocal, or global, it can't be in multiple categories at once. You also get an error if you try to declare something as both global and nonlocal, regardless of the statement order: >>> def outer():
... x = 1
... def inner():
... nonlocal x
... global x
...
File "<stdin>", line 4
SyntaxError: name 'x' is nonlocal and global
>>> def outer():
... x = 1
... def inner():
... global x
... nonlocal x
...
File "<stdin>", line 4
SyntaxError: name 'x' is nonlocal and global By contrast, when the explicit declaration comes first, it turns off the implicit local declarations for that symbol, so there's no conflict with the later name binding statements (they respect the explicit target scope declaration). |
Refer to test_scope.py in the description for additional behavioural tests Verify that syntax errors are correctly raised for improper `global` statements Test various name-binding scenarios for global variables to ensure correct behavior
Hi @ncoghlan, I’m updating you on the changes based on your feedback:
Also, the previous implementation details section has been removed. With the recent documentation update, the prior warning no longer applies. Please let me know if you have additional suggestions. |
Core dev consensus was that this is still a clear bug in the spec, so there's no need to get the SC directly involved. |
The :keyword:`global` statement is a declaration which holds for the entire | ||
current code block. It means that the listed identifiers are to be interpreted | ||
as globals. It would be impossible to assign to a global variable without | ||
The :keyword:`global` causes the listed identifiers to be interpreted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads The global
, so is missing a noun: The global keyword
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…l statement (pythonGH-126523) * Removes erroneous explanation of the `global` statement restrictions; a name declared as global can be subsequently bound using any kind of name binding operation. * Updates `test_global.py` to also test various name-binding scenarios for global variables to ensure correct behavior
…l statement (pythonGH-126523) * Removes erroneous explanation of the `global` statement restrictions; a name declared as global can be subsequently bound using any kind of name binding operation. * Updates `test_global.py` to also test various name-binding scenarios for global variables to ensure correct behavior
As pointed out by @ncoghlan , CPython does not restrict a name in a global statement from reappearing in a for target list, class definition, function definition, or import statement
Additionally, CPython does not impose the same restrictions on with statements or except clauses, or or variable annotations.
The only real restriction is that a variable cannot be used or assigned to prior to its global declaration in the scope.
A few more notes on the changes
📚 Documentation preview 📚: https://cpython-previews--126523.org.readthedocs.build/