Skip to content

Use empty list as default according to Python.asdl #6812

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.

Sign up for GitHub

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

Jump to bottom
Merged
youknowone merged 4 commits into from
Jan 20, 2026
Merged

Use empty list as default according to Python.asdl #6812

youknowone merged 4 commits into from
Jan 20, 2026
+22 −10

Conversation

@moreal
Copy link
Contributor

@moreal moreal commented Jan 20, 2026 •
edited by coderabbitai bot
Loading

Overview

This pull request corrects some Node::ast_to_object implementations according to CPython ASDL. So ClassDef.type_params, ClassDef.keywords, ClassDef.bases and FunctionDef.type_params became to use [] as default because they have * modifier in Python.asdl.

Background

When running cargo run --release -q -- scripts/update_lib auto-mark Lib/test/test_warnings, I met the following logs:

Running test: test_warnings
  __init__.py: Removing expectedFailure: CFilterTests.test_filter_module
  __init__.py: Removing expectedFailure: PyFilterTests.test_filter_module
Traceback (most recent call last):
  File "/path/to/RustPython/crates/pylib/Lib/runpy.py", line 199, in _run_module_as_main
    "__main__", mod_spec)
  File "/path/to/RustPython/crates/pylib/Lib/runpy.py", line 88, in _run_code
    exec(code, run_globals)
  File "/path/to/RustPython/scripts/update_lib/__main__.py", line 84, in <module>
    sys.exit(main())
  File "/path/to/RustPython/scripts/update_lib/__main__.py", line 78, in main
    return auto_mark_main(remaining)
  File "/path/to/RustPython/scripts/update_lib/auto_mark.py", line 689, in main
    args.path, mark_failure=args.mark_failure, skip_build=not args.build
  File "/path/to/RustPython/scripts/update_lib/auto_mark.py", line 634, in auto_mark_directory
    contents, failing_tests, unexpected_successes, error_messages
  File "/path/to/RustPython/scripts/update_lib/auto_mark.py", line 432, in apply_test_changes
    contents = remove_expected_failures(contents, unexpected_successes)
  File "/path/to/RustPython/scripts/update_lib/auto_mark.py", line 313, in remove_expected_failures
    class_bases, class_methods = _build_inheritance_info(tree)
  File "/path/to/RustPython/scripts/update_lib/auto_mark.py", line 265, in _build_inheritance_info
    for base in node.bases
TypeError: 'NoneType' object is not iterable

The source lines are:

...
if isinstance(node, ast.ClassDef):
  bases = [
      base.id
      for base in node.bases
      if isinstance(base, ast.Name) and base.id in all_classes
  ]
...

So we can reproduce with the following code:

import ast
parsed = ast.parse("""class A: ...""")
assert isinstance(parsed, ast.Module)
assert isinstance(parsed.body[0], ast.ClassDef)
class_def = parsed.body[0]
print(class_def.bases)

CPython does:

% python3
Python 3.14.2 (main, Dec  5 2025, 16:49:16) [Clang 17.0.0 (clang-1700.4.4.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ast
... parsed = ast.parse("""class A: ...""")
... assert isinstance(parsed, ast.Module)
... assert isinstance(parsed.body[0], ast.ClassDef)
... class_def = parsed.body[0]
... print(class_def.bases)
...
[]

RustPython (before this patch) does:

% cargo run --release
    Finished `release` profile [optimized] target(s) in 0.27s
     Running `target/release/rustpython`
Welcome to the magnificent Rust Python 0.4.0 interpreter 😱 🖖
RustPython 3.14.0
Type "help", "copyright", "credits" or "license" for more information.
>>>>> import ast
>>>>> parsed = ast.parse("""class A: ...""")
>>>>> assert isinstance(parsed, ast.Module)
>>>>> assert isinstance(parsed.body[0], ast.ClassDef)
>>>>> class_def = parsed.body[0]
>>>>> print(class_def.bases)
None

CPython behavior

https://github.com/python/cpython/blob/71cbffde61449a224dffcae937f5f6be4f86ad09/InternalDocs/compiler.md#L89-L94

Modifiers on the argument type specify the number of values needed; ? means it is optional, * means 0 or more, while no modifier means only one value for the argument and it is required. FunctionDef, for instance,
takes an identifier for the name, arguments for args, zero or more stmt arguments for body, and zero or more expr arguments for decorators.

To quote the definitions of FunctionDef and ClassDef from Python.asdl:

FunctionDef(identifier name, arguments args,
					 stmt* body, expr* decorator_list, expr? returns,
					 string? type_comment, type_param* type_params)

ClassDef(identifier name,
	             expr* bases,
	             keyword* keywords,
	             stmt* body,
	             expr* decorator_list,
	             type_param* type_params)

So ClassDef.type_params, ClassDef.keywords, ClassDef.bases and FunctionDef.type_params should be [] because they have * modifier.

CPython internal (at 3.14.2) - `ast.parse` calls `builtins.compile`. - Internally, it calls `ast2obj_mod` through `builtin_compile_impl`, `Py_CompileStringObject`, `PyAST_mod2obj`. - It calls `ast2obj_stmt` (corresponding to `StmtClassDef`) https://github.com/python/cpython/blob/v3.14.2/Python/Python-ast.c#L8888 ```c // ast2obj_mod(struct ast_state *state, void* _o) case Module_kind: tp = (PyTypeObject *)state->Module_type; result = PyType_GenericNew(tp, NULL, NULL); if (!result) goto failed; value = ast2obj_list(state, (asdl_seq*)o->v.Module.body, ast2obj_stmt); // ast2obj_stmt(struct ast_state *state, void* _o) case ClassDef_kind: tp = (PyTypeObject *)state->ClassDef_type; result = PyType_GenericNew(tp, NULL, NULL); if (!result) goto failed; value = ast2obj_identifier(state, o->v.ClassDef.name); if (!value) goto failed; if (PyObject_SetAttr(result, state->name, value) == -1) goto failed; Py_DECREF(value); value = ast2obj_list(state, (asdl_seq*)o->v.ClassDef.bases, ast2obj_expr); if (!value) goto failed; if (PyObject_SetAttr(result, state->bases, value) == -1) goto failed; ```

Following lines are full implementation of ast2obj_list in cpython 3.14.2, and the asdl_seq_LEN macro returns(?) 0 when the given param is NULL.

static PyObject* ast2obj_list(struct ast_state *state, asdl_seq *seq,
                              PyObject* (*func)(struct ast_state *state, void*))
{
    Py_ssize_t i, n = asdl_seq_LEN(seq);
    PyObject *result = PyList_New(n);
    PyObject *value;
    if (!result)
        return NULL;
    for (i = 0; i < n; i++) {
        value = func(state, asdl_seq_GET_UNTYPED(seq, i));
        if (!value) {
            Py_DECREF(result);
            return NULL;
        }
        PyList_SET_ITEM(result, i, value);
    }
    return result;
}
// https://github.com/python/cpython/blob/v3.14.2/Include/internal/pycore_asdl.h#L83
#define asdl_seq_LEN(S) _Py_RVALUE(((S) == NULL ? 0 : (S)->size))

So the ast2obj_list returns empty list if the given seq(i.e., bases) is NULL.

Summary by CodeRabbit

  • Refactor
    • Improved internal robustness of type parameter and class definition handling by introducing safer optional value handling with appropriate fallbacks, reducing potential error conditions in the compilation pipeline.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 20, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

The changes modify how optional fields (type_params, bases, keywords) are handled in StmtFunctionDef and StmtClassDef AST structures. Instead of direct unwrapping, the code now uses map/unwrap_or_else patterns to safely provide empty Python lists when these fields are None.

Changes

Cohort / File(s) Summary
Optional field handling in AST statements
crates/vm/src/stdlib/ast/statement.rs		 Updated StmtFunctionDef and StmtClassDef to safely handle optional type_params, bases, and keywords fields using `.map(...).unwrap_or_else(

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 The code hops with care, no unwraps in despair,
Optional fields now map with gentle flair,
Empty lists replace the None with grace,
AST statements find their safer place! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: using empty lists as defaults for AST fields according to Python.asdl semantics to align with CPython behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@moreal moreal force-pushed the ast-bases-behavior branch from 16ffc9f to e06f323 Compare January 20, 2026 08:25
@moreal moreal changed the title Correct StmtClassDef::ast_to_object behavior Correct ast_to_object behavior Jan 20, 2026
@moreal moreal changed the title Correct ast_to_object behavior Use empty list as default according to Python.asdl Jan 20, 2026
@moreal moreal marked this pull request as ready for review January 20, 2026 08:54
youknowone
youknowone approved these changes Jan 20, 2026
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@youknowone youknowone merged commit 0cad3cb into RustPython:main Jan 20, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@moreal @youknowone