gh-98811: use full source location to simplify __future__ imports error checking. This also fixes an incorrect error offset. #98812
Conversation
…ts error checking. Also fixes the offset of the error in one case
iritkatriel
added
type-bug
| with self.assertRaises(SyntaxError) as cm: | ||
| from test import badsyntax_future7 | ||
| self.check_syntax_error(cm.exception, "badsyntax_future7", 3, 53) | ||
| self.check_syntax_error(cm.exception, "badsyntax_future7", 3, 54) |
There was a problem hiding this comment.
53 was wrong. It caused this:
python.exe -c "import test.badsyntax_future7.py"
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/Users/iritkatriel/src/cpython/Lib/test/badsyntax_future7.py", line 3
from __future__ import nested_scopes; import string; from __future__ import \
^
SyntaxError: from __future__ imports must occur at the beginning of the file
which should be this:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/Users/iritkatriel/src/cpython-654/Lib/test/badsyntax_future7.py", line 3
from __future__ import nested_scopes; import string; from __future__ import \
^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: from __future__ imports must occur at the beginning of the file
Misc/NEWS.d/next/Core and Builtins/2022-10-28-13-59-51.gh-issue-98811.XQypJa.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Waygood <[email protected]>
markshannon
left a comment
markshannon
left a comment
There was a problem hiding this comment.
LGTM. Maybe convert a macro to an inline function?
Python/compile.c
Outdated
|
|
||
| static location NO_LOCATION = {-1, -1, -1, -1}; | ||
|
|
||
| #define LOCATION_IS_GT(L1, L2) \ |
There was a problem hiding this comment.
Make this an inline function, please.
Also, maybe compare_location(l1, l2) for more generality?
There was a problem hiding this comment.
It's not a total order - how would this function work?
There was a problem hiding this comment.
Could be "lt, gt, or neither", and then you can't use it for equality. So we add an equality testing function too?
There was a problem hiding this comment.
I'll make a function just for gt for now - checking both directions would be less efficient (if it's not GT we don't care about the other distinction) and we don't have a use case for it now. We can always add a location_compare that does two GT calls if we need it.
There was a problem hiding this comment.
It's not a total order - how would this function work?
Good point.
Include/cpython/compile.h
Outdated
| int end_lineno; | ||
| int col_offset; | ||
| int end_col_offset; | ||
| } PyCompilerSrcLocation; |
There was a problem hiding this comment.
I wonder if I should put an underscore so it's _PyCompilerSrcLocation. Are we ready to make this public?
4 participants
Fixes #98811 .