diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index e8dc269dca..ead5dda57d 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -36,7 +36,8 @@ use rustpython_compiler_core::{ Mode, OneIndexed, PositionEncoding, SourceFile, SourceLocation, bytecode::{ self, Arg as OpArgMarker, BinaryOperator, BuildSliceArgCount, CodeObject, - ComparisonOperator, ConstantData, Instruction, Invert, OpArg, OpArgType, UnpackExArgs, + ComparisonOperator, ConstantData, ConvertValueOparg, Instruction, Invert, OpArg, OpArgType, + UnpackExArgs, }, }; use rustpython_wtf8::Wtf8Buf; @@ -5636,7 +5637,12 @@ impl Compiler { } } InterpolatedStringElement::Interpolation(fstring_expr) => { - let mut conversion = fstring_expr.conversion; + let mut conversion = match fstring_expr.conversion { + ConversionFlag::None => ConvertValueOparg::None, + ConversionFlag::Str => ConvertValueOparg::Str, + ConversionFlag::Repr => ConvertValueOparg::Repr, + ConversionFlag::Ascii => ConvertValueOparg::Ascii, + }; if let Some(DebugText { leading, trailing }) = &fstring_expr.debug_text { let range = fstring_expr.expression.range(); @@ -5645,35 +5651,39 @@ impl Compiler { self.emit_load_const(ConstantData::Str { value: text.into() }); element_count += 1; + + // Match CPython behavior: If debug text is present, apply repr conversion. + // if no `format_spec` specified. + // See: https://github.com/python/cpython/blob/f61afca262d3a0aa6a8a501db0b1936c60858e35/Parser/action_helpers.c#L1456 + if matches!( + (conversion, &fstring_expr.format_spec), + (ConvertValueOparg::None, None) + ) { + conversion = ConvertValueOparg::Repr; + } } - match &fstring_expr.format_spec { - None => { - self.emit_load_const(ConstantData::Str { - value: Wtf8Buf::new(), - }); - // Match CPython behavior: If debug text is present, apply repr conversion. - // See: https://github.com/python/cpython/blob/f61afca262d3a0aa6a8a501db0b1936c60858e35/Parser/action_helpers.c#L1456 - if conversion == ConversionFlag::None - && fstring_expr.debug_text.is_some() - { - conversion = ConversionFlag::Repr; - } + self.compile_expression(&fstring_expr.expression)?; + + match conversion { + ConvertValueOparg::None => {} + ConvertValueOparg::Str + | ConvertValueOparg::Repr + | ConvertValueOparg::Ascii => { + emit!(self, Instruction::ConvertValue { oparg: conversion }) } + } + + match &fstring_expr.format_spec { Some(format_spec) => { self.compile_fstring_elements(flags, &format_spec.elements)?; + + emit!(self, Instruction::FormatWithSpec); + } + None => { + emit!(self, Instruction::FormatSimple); } } - - self.compile_expression(&fstring_expr.expression)?; - - let conversion = match conversion { - ConversionFlag::None => bytecode::ConversionFlag::None, - ConversionFlag::Str => bytecode::ConversionFlag::Str, - ConversionFlag::Ascii => bytecode::ConversionFlag::Ascii, - ConversionFlag::Repr => bytecode::ConversionFlag::Repr, - }; - emit!(self, Instruction::FormatValue { conversion }); } } } diff --git a/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap b/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap index 3042e35f17..435b73a14d 100644 --- a/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap +++ b/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap @@ -11,7 +11,7 @@ expression: "compile_exec(\"\\\nfor stop_exc in (StopIteration('spam'), StopAsyn 6 CallFunctionPositional(1) 7 BuildTuple (2) 8 GetIter - >> 9 ForIter (72) + >> 9 ForIter (71) 10 StoreLocal (2, stop_exc) 2 11 LoadNameAny (3, self) @@ -21,7 +21,7 @@ expression: "compile_exec(\"\\\nfor stop_exc in (StopIteration('spam'), StopAsyn 15 CallFunctionPositional(1) 16 LoadConst (("type")) 17 CallMethodKeyword (1) - 18 SetupWith (69) + 18 SetupWith (68) 19 Pop 3 20 SetupExcept (42) @@ -65,23 +65,22 @@ expression: "compile_exec(\"\\\nfor stop_exc in (StopIteration('spam'), StopAsyn 53 LoadConst (None) 54 StoreLocal (8, ex) 55 DeleteLocal (8, ex) - 56 Jump (67) + 56 Jump (66) >> 57 Raise (Reraise) 9 >> 58 LoadNameAny (3, self) 59 LoadMethod (10, fail) - 60 LoadConst ("") - 61 LoadNameAny (2, stop_exc) - 62 FormatValue (None) - 63 LoadConst (" was suppressed") - 64 BuildString (2) - 65 CallMethodPositional (1) - 66 Pop + 60 LoadNameAny (2, stop_exc) + 61 FORMAT_SIMPLE + 62 LoadConst (" was suppressed") + 63 BuildString (2) + 64 CallMethodPositional (1) + 65 Pop - 2 >> 67 PopBlock - 68 EnterFinally - >> 69 WithCleanupStart - 70 WithCleanupFinish - 71 Jump (9) - >> 72 PopBlock - 73 ReturnConst (None) + 2 >> 66 PopBlock + 67 EnterFinally + >> 68 WithCleanupStart + 69 WithCleanupFinish + 70 Jump (9) + >> 71 PopBlock + 72 ReturnConst (None) diff --git a/crates/compiler-core/src/bytecode.rs b/crates/compiler-core/src/bytecode.rs index d1482b8c8a..609ec4a632 100644 --- a/crates/compiler-core/src/bytecode.rs +++ b/crates/compiler-core/src/bytecode.rs @@ -12,18 +12,76 @@ use num_complex::Complex64; use rustpython_wtf8::{Wtf8, Wtf8Buf}; use std::{collections::BTreeSet, fmt, hash, marker::PhantomData, mem, num::NonZeroU8, ops::Deref}; +/// Oparg values for [`Instruction::ConvertValue`]. +/// +/// ## See also +/// +/// - [CPython FVC_* flags](https://github.com/python/cpython/blob/8183fa5e3f78ca6ab862de7fb8b14f3d929421e0/Include/ceval.h#L129-L132) +#[repr(u8)] #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -#[repr(i8)] -#[allow(clippy::cast_possible_wrap)] -pub enum ConversionFlag { - /// No conversion - None = -1, // CPython uses -1 +pub enum ConvertValueOparg { + /// No conversion. + /// + /// ```python + /// f"{x}" + /// f"{x:4}" + /// ``` + None = 0, /// Converts by calling `str()`. - Str = b's' as i8, - /// Converts by calling `ascii()`. - Ascii = b'a' as i8, + /// + /// ```python + /// f"{x!s}" + /// f"{x!s:2}" + /// ``` + Str = 1, /// Converts by calling `repr()`. - Repr = b'r' as i8, + /// + /// ```python + /// f"{x!r}" + /// f"{x!r:2}" + /// ``` + Repr = 2, + /// Converts by calling `ascii()`. + /// + /// ```python + /// f"{x!a}" + /// f"{x!a:2}" + /// ``` + Ascii = 3, +} + +impl fmt::Display for ConvertValueOparg { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let out = match self { + Self::Str => "1 (str)", + Self::Repr => "2 (repr)", + Self::Ascii => "3 (ascii)", + // We should never reach this. `FVC_NONE` are being handled by `Instruction::FormatSimple` + Self::None => "", + }; + + write!(f, "{out}") + } +} + +impl OpArgType for ConvertValueOparg { + #[inline] + fn from_op_arg(x: u32) -> Option { + Some(match x { + // Ruff `ConversionFlag::None` is `-1i8`, + // when its converted to `u8` its value is `u8::MAX` + 0 | 255 => Self::None, + 1 => Self::Str, + 2 => Self::Repr, + 3 => Self::Ascii, + _ => return None, + }) + } + + #[inline] + fn to_op_arg(self) -> u32 { + self as u32 + } } /// Resume type for the RESUME instruction @@ -476,24 +534,6 @@ impl fmt::Display for Label { } } -impl OpArgType for ConversionFlag { - #[inline] - fn from_op_arg(x: u32) -> Option { - match x as u8 { - b's' => Some(Self::Str), - b'a' => Some(Self::Ascii), - b'r' => Some(Self::Repr), - std::u8::MAX => Some(Self::None), - _ => None, - } - } - - #[inline] - fn to_op_arg(self) -> u32 { - self as i8 as u8 as u32 - } -} - op_arg_enum!( /// The kind of Raise that occurred. #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -620,6 +660,18 @@ pub enum Instruction { Continue { target: Arg