-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-98811: use full source location to simplify __future__ imports error checking. This also fixes an incorrect error offset. #98812
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
…ts error checking. Also fixes the offset of the error in one case
@@ -60,7 +60,7 @@ def test_badfuture6(self): | |||
def test_badfuture7(self): | |||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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]>
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.
LGTM. Maybe convert a macro to an inline function?
Python/compile.c
Outdated
|
||
#define LOCATION(LNO, END_LNO, COL, END_COL) \ | ||
((const location){(LNO), (END_LNO), (COL), (END_COL)}) | ||
|
||
static location NO_LOCATION = {-1, -1, -1, -1}; | ||
|
||
#define LOCATION_IS_GT(L1, L2) \ |
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.
Make this an inline function, please.
Also, maybe compare_location(l1, l2)
for more generality?
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.
It's not a total order - how would this function work?
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if I should put an underscore so it's _PyCompilerSrcLocation
. Are we ready to make this public?
Fixes #98811 .