Skip to content

Fix eval type check #5908

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

Merged
merged 2 commits into from
Jul 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ RustPython is a Python 3 interpreter written in Rust, implementing Python 3.13.0
- `parser/` - Parser for converting Python source to AST
- `core/` - Bytecode representation in Rust structures
- `codegen/` - AST to bytecode compiler
- `Lib/` - CPython's standard library in Python (copied from CPython)
- `Lib/` - CPython's standard library in Python (copied from CPython). **IMPORTANT**: Do not edit this directory directly; The only allowed operation is copying files from CPython.
- `derive/` - Rust macros for RustPython
- `common/` - Common utilities
- `extra_tests/` - Integration tests and snippets
Expand Down
73 changes: 73 additions & 0 deletions extra_tests/snippets/builtin_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,76 @@

code = compile("5+3", "x.py", "eval")
assert eval(code) == 8

# Test that globals must be a dict
import collections

# UserDict is a mapping but not a dict - should fail in eval
user_dict = collections.UserDict({"x": 5})
try:
eval("x", user_dict)
assert False, "eval with UserDict globals should fail"
except TypeError as e:
# CPython: "globals must be a real dict; try eval(expr, {}, mapping)"
assert "globals must be a real dict" in str(e), e

# Non-mapping should have different error message
try:
eval("x", 123)
assert False, "eval with int globals should fail"
except TypeError as e:
# CPython: "globals must be a dict"
assert "globals must be a dict" in str(e)
assert "real dict" not in str(e)

# List is not a mapping
try:
eval("x", [])
assert False, "eval with list globals should fail"
except TypeError as e:
assert "globals must be a real dict" in str(e), e

Choose a reason for hiding this comment

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

high

The assertion for eval with a list as globals is not quite right. Since a list is not a mapping, eval should raise a TypeError indicating that globals must be a dict, not a "real dict". The "real dict" message is for mapping-like objects that aren't actual dictionaries (e.g., collections.UserDict).

Suggested change
assert "globals must be a real dict" in str(e), e
assert "globals must be a dict" in str(e)
assert "real dict" not in str(e), e


# Regular dict should work
assert eval("x", {"x": 42}) == 42

# None should use current globals
x = 100
assert eval("x", None) == 100

# Test locals parameter
# Locals can be any mapping (unlike globals which must be dict)
assert eval("y", {"y": 1}, user_dict) == 1 # UserDict as locals is OK

# But locals must still be a mapping
try:
eval("x", {"x": 1}, 123)
assert False, "eval with int locals should fail"
except TypeError as e:
# This error is handled by ArgMapping validation
assert "not a mapping" in str(e) or "locals must be a mapping" in str(e)

# Test that __builtins__ is added if missing
globals_without_builtins = {"x": 5}
result = eval("x", globals_without_builtins)
assert result == 5
assert "__builtins__" in globals_without_builtins

# Test with both globals and locals
assert eval("x + y", {"x": 10}, {"y": 20}) == 30

# Test that when globals is None and locals is provided, it still works
assert eval("x + y", None, {"x": 1, "y": 2}) == 3


# Test code object with free variables
def make_closure():
z = 10
return compile("x + z", "<string>", "eval")


closure_code = make_closure()
try:
eval(closure_code, {"x": 5})
assert False, "eval with code containing free variables should fail"
except NameError as e:
pass
49 changes: 41 additions & 8 deletions vm/src/stdlib/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,24 +249,56 @@ mod builtins {
#[derive(FromArgs)]
struct ScopeArgs {
#[pyarg(any, default)]
globals: Option<PyDictRef>,
globals: Option<PyObjectRef>,
#[pyarg(any, default)]
locals: Option<ArgMapping>,
}

impl ScopeArgs {
fn make_scope(self, vm: &VirtualMachine) -> PyResult<crate::scope::Scope> {
fn make_scope(
self,
vm: &VirtualMachine,
func_name: &'static str,
) -> PyResult<crate::scope::Scope> {
fn validate_globals_dict(
globals: &PyObjectRef,
vm: &VirtualMachine,
func_name: &'static str,
) -> PyResult<()> {
if !globals.fast_isinstance(vm.ctx.types.dict_type) {
return Err(match func_name {
"eval" => {
let is_mapping = crate::protocol::PyMapping::check(globals);
vm.new_type_error(if is_mapping {
"globals must be a real dict; try eval(expr, {}, mapping)"
.to_owned()
} else {
"globals must be a dict".to_owned()
})
}
"exec" => vm.new_type_error(format!(
"exec() globals must be a dict, not {}",
globals.class().name()
)),
_ => vm.new_type_error("globals must be a dict".to_owned()),
});
}
Ok(())
}

let (globals, locals) = match self.globals {
Some(globals) => {
validate_globals_dict(&globals, vm, func_name)?;

let globals = PyDictRef::try_from_object(vm, globals)?;
if !globals.contains_key(identifier!(vm, __builtins__), vm) {
let builtins_dict = vm.builtins.dict().into();
globals.set_item(identifier!(vm, __builtins__), builtins_dict, vm)?;
}
(
globals.clone(),
self.locals.unwrap_or_else(|| {
ArgMapping::try_from_object(vm, globals.into()).unwrap()
}),
self.locals
.unwrap_or_else(|| ArgMapping::from_dict_exact(globals.clone())),
)
}
None => (
Expand All @@ -290,6 +322,8 @@ mod builtins {
scope: ScopeArgs,
vm: &VirtualMachine,
) -> PyResult {
let scope = scope.make_scope(vm, "eval")?;

// source as string
let code = match source {
Either::A(either) => {
Expand Down Expand Up @@ -323,18 +357,17 @@ mod builtins {
scope: ScopeArgs,
vm: &VirtualMachine,
) -> PyResult {
let scope = scope.make_scope(vm, "exec")?;
run_code(vm, source, scope, crate::compiler::Mode::Exec, "exec")
}

fn run_code(
vm: &VirtualMachine,
source: Either<PyStrRef, PyRef<crate::builtins::PyCode>>,
scope: ScopeArgs,
scope: crate::scope::Scope,
#[allow(unused_variables)] mode: crate::compiler::Mode,
func: &str,
) -> PyResult {
let scope = scope.make_scope(vm)?;

// Determine code object:
let code_obj = match source {
#[cfg(feature = "rustpython-compiler")]
Expand Down
Loading