8000 Fix f-string conversion flag ValueError when compiling AST by youknowone · Pull Request #6534 · RustPython/RustPython · GitHub
[go: up one dir, main page]
Skip to content

Fix f-string conversion flag ValueError when compiling AST #6534

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

Merged
youknowone merged 1 commit into from
Dec 26, 2025
Merged

Fix f-string conversion flag ValueError when compiling AST #6534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion crates/vm/src/stdlib/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use crate::{
builtins::PyIntRef,
builtins::{PyDict, PyModule, PyStrRef, PyType},
class::{PyClassImpl, StaticType},
compiler::core::bytecode::OpArgType,
compiler::{CompileError, ParseError},
convert::ToPyObject,
};
Expand Down
22 changes: 10 additions & 12 deletions crates/vm/src/stdlib/ast/other.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use super::*;
use num_traits::ToPrimitive;
use rustpython_compiler_core::{SourceFile, bytecode};
use rustpython_compiler_core::SourceFile;

impl Node for ruff::ConversionFlag {
fn ast_to_object(self, vm: &VirtualMachine, _source_file: &SourceFile) -> PyObjectRef {
Expand All @@ -12,16 +11,15 @@ impl Node for ruff::ConversionFlag {
_source_file: &SourceFile,
object: PyObjectRef,
) -> PyResult<Self> {
i32::try_from_object(vm, object)?
.to_u32()
.and_then(bytecode::ConvertValueOparg::from_op_arg)
.map(|flag| match flag {
bytecode::ConvertValueOparg::None => Self::None,
bytecode::ConvertValueOparg::Str => Self::Str,
bytecode::ConvertValueOparg::Repr => Self::Repr,
bytecode::ConvertValueOparg::Ascii => Self::Ascii,
})
.ok_or_else(|| vm.new_value_error("invalid conversion flag"))
// Python's AST uses ASCII codes: 's', 'r', 'a', -1=None
// Note: 255 is -1i8 as u8 (ruff's ConversionFlag::None)
match i32::try_from_object(vm, object)? {
-1 | 255 => Ok(Self::None),
x if x == b's' as i32 => Ok(Self::Str),
x if x == b'r' as i32 => Ok(Self::Repr),
x if x == b'a' as i32 => Ok(Self::Ascii),
_ => Err(vm.new_value_error("invalid conversion flag")),
}
Comment on lines +14 to +22
Copy link
Contributor
@coderabbitai coderabbitai bot Dec 26, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Search for ConversionFlag enum definition
rg -n "enum ConversionFlag" --type rust -A 10

Repository: RustPython/RustPython

Length of output: 47

🏁 Script executed:

# Search for ast_to_object implementation in the file
rg -n "fn ast_to_object" crates/vm/src/stdlib/ast/other.rs -A 5

Repository: RustPython/RustPython

Length of output: 1205

🏁 Script executed:

# Check if there are any tests for ConversionFlag round-trip conversion
rg -n "ConversionFlag" crates/vm/src/stdlib/ast/ --type rust -B 2 -A 2

Repository: RustPython/RustPython

Length of output: 1254

🏁 Script executed:

# Check cargo.toml for ruff dependency
rg -n "ruff" Cargo.toml -A 2 -B 2

Repository: RustPython/RustPython

Length of output: 1048

🏁 Script executed:

# Search for ConversionFlag in ruff-related files or comments
rg -n "ConversionFlag" --type rust -B 3 -A 3 | head -50

Repository: RustPython/RustPython

Length of output: 3179

🏁 Script executed:

# Check if there's any documentation or external reference about ConversionFlag values
find . -name "*.rs" -type f | xargs grep -l "ruff::ConversionFlag" | head -5

Repository: RustPython/RustPython

Length of output: 137

🏁 Script executed:

# Search for usage of ast_to_object to understand if it's used for Python round-trips
rg -n "ast_to_object" crates/vm/src/stdlib/ast/ --type rust -B 2 -A 2 | head -40

Repository: RustPython/RustPython

Length of output: 2701

🏁 Script executed:

# Check if there are tests for AST conversion
fd "test" crates/vm/src/stdlib/ast/ --type rust

Repository: RustPython/RustPython

Length of output: 236

🏁 Script executed:

# Look at the unparse.rs file since it imports ConversionFlag - check how it uses it
head -50 crates/codegen/src/unparse.rs

Repository: RustPython/RustPython

Length of output: 1461

🏁 Script executed:

# Search for where FormattedValue is used and how conversion field is accessed
rg -n "FormattedValue" crates/vm/src/stdlib/ast/ -B 2 -A 5

Repository: RustPython/RustPython

Length of output: 7673

🏁 Script executed:

# Check how conversion flag is used in string formatting code
rg -n "conversion" crates/vm/src/stdlib/ast/string.rs -B 2 -A 2

Repository: RustPython/RustPython

Length of output: 1670

🏁 Script executed:

# Search for any documentation about Python's ConversionFlag values
rg -n "conversion.*flag\|conversion.*-1\|conversion.*255" --ignore-case -i

Repository: RustPython/RustPython

Length of output: 47

Fix ast_to_object to output -1 for ConversionFlag::None to preserve round-trip compatibility.

The round-trip conversion is real: FormattedValue uses conversion.ast_to_object() to serialize to Python AST objects (line 318 in string.rs), and later deserializes via ast_from_object() (line 340). While ast_from_object correctly accepts both -1 and 255 for None, the reverse conversion breaks round-trip compatibility:

  • ast_to_object outputs 255 (via self as u8) for ConversionFlag::None
  • Python's _ast.FormattedValue expects -1 for no conversion
  • Result: Python AST objects get conversion=255 instead of -1, breaking compatibility

Fix by special-casing None to output -1 as a signed integer:

fn ast_to_object(self, vm: &VirtualMachine, _source_file: &SourceFile) -> PyObjectRef {
    let val = match self {
        Self::None => -1i32,
        _ => self as u8 as i32,
    };
    vm.ctx.new_int(val).into()
}
🤖 Prompt for AI Agents 
In crates/vm/src/stdlib/ast/other.rs around lines 14 to 22, ast_to_object
currently emits 255 for ConversionFlag::None (via casting to u8) which breaks
round-trip with Python AST expecting -1; change ast_to_object to special-case
Self::None and produce -1i32, otherwise cast the enum to u8 then to i32, and
return vm.ctx.new_int(val).into() so the Python AST sees -1 for None and other
flags remain unchanged.

}
}

Expand Down
Loading
0