Skip to content

Commit 39448ad

Browse files
iritkatrieliritkatriel
authored
gh-98811: use full source location to simplify __future__ imports error checking. This also fixes an incorrect error offset. (GH-98812)
1 parent 29f98b4 commit 39448ad

File tree

5 files changed

+71
-69
lines changed

5 files changed

+71
-69
lines changed

‎Include/cpython/compile.h‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,26 @@ typedef struct {
3131
#define _PyCompilerFlags_INIT \
3232
(PyCompilerFlags){.cf_flags = 0, .cf_feature_version = PY_MINOR_VERSION}
3333

34+
/* source location information */
35+
typedef struct {
36+
int lineno;
37+
int end_lineno;
38+
int col_offset;
39+
int end_col_offset;
40+
} _PyCompilerSrcLocation;
41+
42+
#define SRC_LOCATION_FROM_AST(n) \
43+
(_PyCompilerSrcLocation){ \
44+
.lineno = (n)->lineno, \
45+
.end_lineno = (n)->end_lineno, \
46+
.col_offset = (n)->col_offset, \
47+
.end_col_offset = (n)->end_col_offset }
48+
3449
/* Future feature support */
3550

3651
typedef struct {
37-
int ff_features; /* flags set by future statements */
38-
int ff_lineno; /* line number of last future statement */
52+
int ff_features; /* flags set by future statements */
53+
_PyCompilerSrcLocation ff_location; /* location of last future statement */
3954
} PyFutureFeatures;
4055

4156
#define FUTURE_NESTED_SCOPES "nested_scopes"

‎Lib/test/test_future.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def test_badfuture6(self):
6060
def test_badfuture7(self):
6161
with self.assertRaises(SyntaxError) as cm:
6262
from test import badsyntax_future7
63-
self.check_syntax_error(cm.exception, "badsyntax_future7", 3, 53)
63+
self.check_syntax_error(cm.exception, "badsyntax_future7", 3, 54)
6464

6565
def test_badfuture8(self):
6666
with self.assertRaises(SyntaxError) as cm:

‎Misc/NEWS.d/next/Core and Builtins/2022-10-28-13-59-51.gh-issue-98811.XQypJa.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Use complete source locations to simplify detection of ``__future__``
2+
imports which are not at the beginning of the file. Also corrects the offset
3+
in the exception raised in one case, which was off by one and impeded
4+
highlighting.

‎Python/compile.c‎

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -125,18 +125,23 @@
125125
(c->c_flags->cf_flags & PyCF_ALLOW_TOP_LEVEL_AWAIT) \
126126
&& (c->u->u_ste->ste_type == ModuleBlock))
127127

128-
typedef struct location_ {
129-
int lineno;
130-
int end_lineno;
131-
int col_offset;
132-
int end_col_offset;
133-
} location;
128+
typedef _PyCompilerSrcLocation location;
134129

135130
#define LOCATION(LNO, END_LNO, COL, END_COL) \
136131
((const location){(LNO), (END_LNO), (COL), (END_COL)})
137132

138133
static location NO_LOCATION = {-1, -1, -1, -1};
139134

135+
/* Return true if loc1 starts after loc2 ends. */
136+
static inline bool
137+
location_is_after(location loc1, location loc2) {
138+
return (loc1.lineno > loc2.end_lineno) ||
139+
((loc1.lineno == loc2.end_lineno) &&
140+
(loc1.col_offset > loc2.end_col_offset));
141+
}
142+
143+
#define LOC(x) SRC_LOCATION_FROM_AST(x)
144+
140145
typedef struct jump_target_label_ {
141146
int id;
142147
} jump_target_label;
@@ -1012,11 +1017,6 @@ basicblock_next_instr(basicblock *b)
10121017
// Artificial instructions
10131018
#define UNSET_LOC(c)
10141019

1015-
#define LOC(x) LOCATION((x)->lineno, \
1016-
(x)->end_lineno, \
1017-
(x)->col_offset, \
1018-
(x)->end_col_offset)
1019-
10201020

10211021
/* Return the stack effect of opcode with argument oparg.
10221022
@@ -3911,59 +3911,61 @@ compiler_import(struct compiler *c, stmt_ty s)
39113911
static int
39123912
compiler_from_import(struct compiler *c, stmt_ty s)
39133913
{
3914-
location loc = LOC(s);
3915-
Py_ssize_t i, n = asdl_seq_LEN(s->v.ImportFrom.names);
3916-
PyObject *names;
3914+
Py_ssize_t n = asdl_seq_LEN(s->v.ImportFrom.names);
39173915

3918-
ADDOP_LOAD_CONST_NEW(c, loc, PyLong_FromLong(s->v.ImportFrom.level));
3916+
ADDOP_LOAD_CONST_NEW(c, LOC(s), PyLong_FromLong(s->v.ImportFrom.level));
39193917

3920-
names = PyTuple_New(n);
3921-
if (!names)
3918+
PyObject *names = PyTuple_New(n);
3919+
if (!names) {
39223920
return 0;
3921+
}
39233922

39243923
/* build up the names */
3925-
for (i = 0; i < n; i++) {
3924+
for (Py_ssize_t i = 0; i < n; i++) {
39263925
alias_ty alias = (alias_ty)asdl_seq_GET(s->v.ImportFrom.names, i);
39273926
Py_INCREF(alias->name);
39283927
PyTuple_SET_ITEM(names, i, alias->name);
39293928
}
39303929

3931-
if (s->lineno > c->c_future->ff_lineno && s->v.ImportFrom.module &&
3932-
_PyUnicode_EqualToASCIIString(s->v.ImportFrom.module, "__future__")) {
3930+
if (location_is_after(LOC(s), c->c_future->ff_location) &&
3931+
s->v.ImportFrom.module &&
3932+
_PyUnicode_EqualToASCIIString(s->v.ImportFrom.module, "__future__"))
3933+
{
39333934
Py_DECREF(names);
3934-
return compiler_error(c, loc, "from __future__ imports must occur "
3935+
return compiler_error(c, LOC(s), "from __future__ imports must occur "
39353936
"at the beginning of the file");
39363937
}
3937-
ADDOP_LOAD_CONST_NEW(c, loc, names);
3938+
ADDOP_LOAD_CONST_NEW(c, LOC(s), names);
39383939

39393940
if (s->v.ImportFrom.module) {
3940-
ADDOP_NAME(c, loc, IMPORT_NAME, s->v.ImportFrom.module, names);
3941+
ADDOP_NAME(c, LOC(s), IMPORT_NAME, s->v.ImportFrom.module, names);
39413942
}
39423943
else {
39433944
_Py_DECLARE_STR(empty, "");
3944-
ADDOP_NAME(c, loc, IMPORT_NAME, &_Py_STR(empty), names);
3945+
ADDOP_NAME(c, LOC(s), IMPORT_NAME, &_Py_STR(empty), names);
39453946
}
3946-
for (i = 0; i < n; i++) {
3947+
for (Py_ssize_t i = 0; i < n; i++) {
39473948
alias_ty alias = (alias_ty)asdl_seq_GET(s->v.ImportFrom.names, i);
39483949
identifier store_name;
39493950

39503951
if (i == 0 && PyUnicode_READ_CHAR(alias->name, 0) == '*') {
39513952
assert(n == 1);
3952-
ADDOP(c, loc, IMPORT_STAR);
3953+
ADDOP(c, LOC(s), IMPORT_STAR);
39533954
return 1;
39543955
}
39553956

3956-
ADDOP_NAME(c, loc, IMPORT_FROM, alias->name, names);
3957+
ADDOP_NAME(c, LOC(s), IMPORT_FROM, alias->name, names);
39573958
store_name = alias->name;
3958-
if (alias->asname)
3959+
if (alias->asname) {
39593960
store_name = alias->asname;
3961+
}
39603962

3961-
if (!compiler_nameop(c, loc, store_name, Store)) {
3963+
if (!compiler_nameop(c, LOC(s), store_name, Store)) {
39623964
return 0;
39633965
}
39643966
}
39653967
/* remove imported module */
3966-
ADDOP(c, loc, POP_TOP);
3968+
ADDOP(c, LOC(s), POP_TOP);
39673969
return 1;
39683970
}
39693971

‎Python/future.c‎

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
#include "pycore_ast.h" // _PyAST_GetDocString()
33

44
#define UNDEFINED_FUTURE_FEATURE "future feature %.100s is not defined"
5-
#define ERR_LATE_FUTURE \
6-
"from __future__ imports must occur at the beginning of the file"
75

86
static int
97
future_check_features(PyFutureFeatures *ff, stmt_ty s, PyObject *filename)
@@ -56,59 +54,42 @@ future_check_features(PyFutureFeatures *ff, stmt_ty s, PyObject *filename)
5654
static int
5755
future_parse(PyFutureFeatures *ff, mod_ty mod, PyObject *filename)
5856
{
59-
int i, done = 0, prev_line = 0;
60-
61-
if (!(mod->kind == Module_kind || mod->kind == Interactive_kind))
57+
if (!(mod->kind == Module_kind || mod->kind == Interactive_kind)) {
6258
return 1;
59+
}
6360

64-
if (asdl_seq_LEN(mod->v.Module.body) == 0)
61+
Py_ssize_t n = asdl_seq_LEN(mod->v.Module.body);
62+
if (n == 0) {
6563
return 1;
64+
}
6665

67-
/* A subsequent pass will detect future imports that don't
68-
appear at the beginning of the file. There's one case,
69-
however, that is easier to handle here: A series of imports
70-
joined by semi-colons, where the first import is a future
71-
statement but some subsequent import has the future form
72-
but is preceded by a regular import.
73-
*/
74-
75-
i = 0;
76-
if (_PyAST_GetDocString(mod->v.Module.body) != NULL)
66+
Py_ssize_t i = 0;
67+
if (_PyAST_GetDocString(mod->v.Module.body) != NULL) {
7768
i++;
69+
}
7870

79-
for (; i < asdl_seq_LEN(mod->v.Module.body); i++) {
71+
for (; i < n; i++) {
8072
stmt_ty s = (stmt_ty)asdl_seq_GET(mod->v.Module.body, i);
8173

82-
if (done && s->lineno > prev_line)
83-
return 1;
84-
prev_line = s->lineno;
85-
86-
/* The tests below will return from this function unless it is
87-
still possible to find a future statement. The only things
88-
that can precede a future statement are another future
89-
statement and a doc string.
90-
*/
74+
/* The only things that can precede a future statement
75+
* are another future statement and a doc string.
76+
*/
9177

9278
if (s->kind == ImportFrom_kind) {
9379
identifier modname = s->v.ImportFrom.module;
9480
if (modname &&
9581
_PyUnicode_EqualToASCIIString(modname, "__future__")) {
96-
if (done) {
97-
PyErr_SetString(PyExc_SyntaxError,
98-
ERR_LATE_FUTURE);
99-
PyErr_SyntaxLocationObject(filename, s->lineno, s->col_offset);
82+
if (!future_check_features(ff, s, filename)) {
10083
return 0;
10184
}
102-
if (!future_check_features(ff, s, filename))
103-
return 0;
104-
ff->ff_lineno = s->lineno;
85+
ff->ff_location = SRC_LOCATION_FROM_AST(s);
10586
}
10687
else {
107-
done = 1;
88+
return 1;
10889
}
10990
}
11091
else {
111-
done = 1;
92+
return 1;
11293
}
11394
}
11495
return 1;
@@ -126,7 +107,7 @@ _PyFuture_FromAST(mod_ty mod, PyObject *filename)
126107
return NULL;
127108
}
128109
ff->ff_features = 0;
129-
ff->ff_lineno = -1;
110+
ff->ff_location = (_PyCompilerSrcLocation){-1, -1, -1, -1};
130111

131112
if (!future_parse(ff, mod, filename)) {
132113
PyObject_Free(ff);

0 commit comments

Comments
 (0)