Repository navigation
ast.parse() believes valid context manager py38 syntax to be invalid when feature_version=(3, 8) is passed #115881
Description
Bug report
Bug description:
The following code is completely valid on Python 3.8:
from contextlib import nullcontext
with (
nullcontext() if bool() else nullcontext()
):
passHowever, following 0daba82 (which was backported to Python 3.11 and Python 3.10), ast.parse() incorrectly throws an error if you try to parse this code with feature_version=(3, 8):
% python
Python 3.10.6 (main, Feb 24 2024, 10:35:05) [Clang 15.0.0 (clang-1500.1.0.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ast
>>> source = """
... with (
... nullcontext() if bool() else nullcontext()
... ):
... pass
... """
>>> ast.parse(source, feature_version=(3, 8))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/alexw/.pyenv/versions/3.10.6/lib/python3.10/ast.py", line 50, in parse
return compile(source, filename, mode, flags,
File "<unknown>", line 5
pass
^
SyntaxError: Parenthesized context managers are only supported in Python 3.9 and greaterCc. @hauntsaninja / @pablogsal
CPython versions tested on:
3.8, 3.9, 3.10, 3.11, 3.12
Operating systems tested on:
macOS
Linked PRs
- gh-115881: Ensure
ast.parse()parses conditional context managers even with lowfeature_versionpassed #115920 - [3.12] gh-115881: Ensure
ast.parse()parses conditional context managers even with lowfeature_versionpassed (#115920) #115959 - [3.11] gh-115881: Ensure
ast.parse()parses conditional context managers even with lowfeature_versionpassed (#115920) #115960 - gh-115881: Document feature_version limitations #115980
- [3.12] gh-115881: Document feature_version limitations (#115980) #116173
- [3.11] gh-115881: Document feature_version limitations (#115980) #116174
Activity
[-]`ast.parse()` believes valid py38 syntax to be invalid when `feature_version=(3, 8)` is passed[/-][+]`ast.parse()` believes valid context manager py38 syntax to be invalid when `feature_version=(3, 8)` is passed[/+]pablogsal commentedon Feb 24, 2024
I think there isn't going to be any super clean way to do this because this new construct has a non trivial syntactic overlap with the case reported here. This was precisely what made it impossible to do with the old parser. I think the best way is to revert 0daba82 but maybe I am missing some elegant way to do it so I am open to alternatives
pablogsal commentedon Feb 24, 2024
CC: @lysnikolaou
hauntsaninja commentedon Feb 25, 2024
Thanks for reporting! I think we can fix by adding another grammar rule. I don't know if that meets the bar for elegant, if not, I agree reverting could be fine. I'm thinking of something like:
By the way, I notice that
with (contextlib.nullcontext(),): ...andwith (contextlib.nullcontext(),): ...parse differently with old and new parser! :-)pablogsal commentedon Feb 25, 2024
I prefer to revert the commit. The grammar is already quite complicated and we are automatically generating docs from it so I would recommend you try to keep complexity controlled
pablogsal commentedon Feb 25, 2024
Maybe it's because I am reading this in a phone but I cannot see any difference 😅
In any case we were aware of that being a thing and other similar cases but in all of them we were ok with the change since tuples will always fail as context managers 😉
AlexWaygood commentedon Feb 25, 2024
We'll want to backport the fix as well, so I agree that simply reverting feels safer for now (and possibly consider a more complex solution for Python 3.13+ only? But I'll defer to those more experienced with hacking on the parser on whether that's worth it or not :)
lysnikolaou commentedon Feb 26, 2024
Yup, I agree with reverting this.
Like I've said a few times before, deprecating
feature_versionmight also be a good idea. @hauntsaninja Do I remember correctly thatmypyuses this? Is it still relevant?gh-115881: Ensure `ast.parse()` parses conditional context managers e…
pythongh-115881: Ensure `ast.parse()` parses conditional context mana…
9 remaining items
hauntsaninja commentedon Feb 26, 2024
Thanks Alex! Yes, I believe it's used by at least mypy, black, pytype. We should document more clearly that
feature_versionis a "best effort" sort of thing, here's a PR with some possible wording: #115980I think the functionality is useful, even if limited, so I'd prefer not to deprecate
gh-115881: Document feature_version limitations (#115980)
[3.12] pythongh-115881: Document feature_version limitations (python#…
[3.11] pythongh-115881: Document feature_version limitations (python#…
[3.12] gh-115881: Document feature_version limitations (#115980) (#11…
[3.11] gh-115881: Document feature_version limitations (#115980) (#11…
hauntsaninja commentedon Mar 1, 2024
Okay, we did the revert, we've updated docs and I did the backports, so I think we can close. Let me know if I missed something!
pythongh-115881: Ensure `ast.parse()` parses conditional context mana…
pythongh-115881: Document feature_version limitations (python#115980)
pythongh-115881: Ensure `ast.parse()` parses conditional context mana…
pythongh-115881: Document feature_version limitations (python#115980)
pythongh-115881: Ensure `ast.parse()` parses conditional context mana…
pythongh-115881: Document feature_version limitations (python#115980)
pythongh-115881: Ensure `ast.parse()` parses conditional context mana…