Repository: python/cpython
Pull request: gh-93351: Ensure the position information in AST nodes created by the parser is always consistent
Closes: #93351
<!--
Thanks for your contribution!
Please read this comment in its entirety. It's quite important.

# Pull Request title

It should be in the following format:

```
gh-NNNNN: Summary of the changes made
```

Where: gh-NNNNN refers to the GitHub issue number.

Most PRs will require an issue number. Trivial changes, like fixing a typo, do not need an issue.

# Backport Pull Request title

If this is a backport PR (PR made against branches other than `main`),
please ensure that the PR title is in the following format:

```
[X.Y] <title from the original PR> (GH-NNNN)
```

Where: [X.Y] is the branch name, e.g. [3.6].

GH-NNNN refers to the PR number from `main`.

-->


Problem statement:
Ensure the position information in AST nodes created by the parser is always consistent
If we create an AST node with invalid position information, the error is reported in the compiler when it validates the line numbers and end line numbers, which is too far away and debugging this can be quite challenging. To improve the debugging experience, ensure that the parser cannot create an AST node with invalid position information.

Hints from issue discussion:

Commit message: gh-93351: Validate the position information when compiling AST nodes

Patch to review:
diff --git a/Lib/test/test_ast.py b/Lib/test/test_ast.py
index 03d9b310f161b9..896eb5bedd7f38 100644
--- a/Lib/test/test_ast.py
+++ b/Lib/test/test_ast.py
@@ -335,6 +335,33 @@ def test_ast_validation(self):
         for snippet in snippets_to_validate:
             tree = ast.parse(snippet)
             compile(tree, '<string>', 'exec')
+    
+    def test_invalid_position_information(self):
+        invalid_linenos = [
+            (10, 1), (-10, -11), (10, -11), (-5, -2), (-5, 1)
+        ]
+
+        for lineno, end_lineno in invalid_linenos:
+            with self.subTest(f"Check invalid linenos {lineno}:{end_lineno}"):
+                snippet = "a = 1"
+                tree = ast.parse(snippet)
+                tree.body[0].lineno = lineno
+                tree.body[0].end_lineno = end_lineno
+                with self.assertRaises(ValueError):
+                    compile(tree, '<string>', 'exec')
+
+        invalid_col_offsets = [
+            (10, 1), (-10, -11), (10, -11), (-5, -2), (-5, 1)
+        ]
+        for col_offset, end_col_offset in invalid_col_offsets:
+            with self.subTest(f"Check invalid col_offset {col_offset}:{end_col_offset}"):
+                snippet = "a = 1"
+                tree = ast.parse(snippet)
+                tree.body[0].col_offset = col_offset
+                tree.body[0].end_col_offset = end_col_offset
+                with self.assertRaises(ValueError):
+                    compile(tree, '<string>', 'exec')
+
 
     def test_slice(self):
         slc = ast.parse("x[::]").body[0].value.slice
diff --git a/Python/ast.c b/Python/ast.c
index 607281e2685535..55f7477eae3c96 100644
--- a/Python/ast.c
+++ b/Python/ast.c
@@ -22,6 +22,27 @@ static int validate_stmt(struct validator *, stmt_ty);
 static int validate_expr(struct validator *, expr_ty, expr_context_ty);
 static int validate_pattern(struct validator *, pattern_ty, int);
 
+#define VALIDATE_POSITIONS(node) \
+    if (node->lineno > node->end_lineno) { \
+        PyErr_Format(PyExc_ValueError, \
+                     "line %d-%d is not a valid range", \
+                     node->lineno, node->end_lineno); \
+        return 0; \
+    } \
+    if ((node->lineno < 0 && node->end_lineno != node->lineno) || \
+        (node->col_offset < 0 &&  node->col_offset != node->end_col_offset)) { \
+        PyErr_Format(PyExc_ValueError, \
+                     "line %d-%d, column %d-%d is not a valid range", \
+                     node->lineno, node->end_lineno, node->col_offset, node->end_col_offset); \
+        return 0; \
+    } \
+    if (node->lineno == node->end_lineno && node->col_offset > node->end_col_offset) { \
+        PyErr_Format(PyExc_ValueError, \
+                     "line %d, column %d-%d is not a valid range", \
+                     node->lineno, node->col_offset, node->end_col_offset); \
+        return 0; \
+    }
+
 static int
 validate_name(PyObject *name)
 {
@@ -183,6 +204,7 @@ validate_constant(struct validator *state, PyObject *value)
 static int
 validate_expr(struct validator *state, expr_ty exp, expr_context_ty ctx)
 {
+    VALIDATE_POSITIONS(exp);
     int ret = -1;
     if (++state->recursion_depth > state->recursion_limit) {
         PyErr_SetString(PyExc_RecursionError,
@@ -505,6 +527,7 @@ validate_capture(PyObject *name)
 static int
 validate_pattern(struct validator *state, pattern_ty p, int star_ok)
 {
+    VALIDATE_POSITIONS(p);
     int ret = -1;
     if (++state->recursion_depth > state->recursion_limit) {
         PyErr_SetString(PyExc_RecursionError,
@@ -674,6 +697,7 @@ validate_body(struct validator *state, asdl_stmt_seq *body, const char *owner)
 static int
 validate_stmt(struct validator *state, stmt_ty stmt)
 {
+    VALIDATE_POSITIONS(stmt);
     int ret = -1;
     Py_ssize_t i;
     if (++state->recursion_depth > state->recursion_limit) {


Code review comments:
diff_hunk: @@ -22,6 +22,27 @@ static int validate_stmt(struct validator *, stmt_ty);
 static int validate_expr(struct validator *, expr_ty, expr_context_ty);
 static int validate_pattern(struct validator *, pattern_ty, int);
 
+#define VALIDATE_POSITIONS(node) \
+    if (node->lineno > node->end_lineno) { \
+        PyErr_Format(PyExc_ValueError, \
+                     "line %d-%d is not a valid range", \
+                     node->lineno, node->end_lineno); \
+        return 0; \
+    } \
+    if ((node->lineno < 0 && node->end_lineno != node->lineno) || \
+        (node->col_offset < 0 &&  node->col_offset != node->end_col_offset)) { \
path: Python/ast.c
text: @user1:
NIT:
```suggestion
        (node->col_offset < 0 && node->col_offset != node->end_col_offset)) { \
```