Skip to content

Commit 26d5307

Browse files
ShaharNavehShaharNaveh
authored
Align f-string related bytecodes with 3.13 (#6321)
* Align `f-string` related bytecodes with 3.13 * Resolve name collision * Adjust for ruff return value
1 parent d287d1e commit 26d5307

File tree

5 files changed

+187
-96
lines changed

5 files changed

+187
-96
lines changed

‎crates/codegen/src/compile.rs‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ use rustpython_compiler_core::{
3636
Mode, OneIndexed, PositionEncoding, SourceFile, SourceLocation,
3737
bytecode::{
3838
self, Arg as OpArgMarker, BinaryOperator, BuildSliceArgCount, CodeObject,
39-
ComparisonOperator, ConstantData, Instruction, Invert, OpArg, OpArgType, UnpackExArgs,
39+
ComparisonOperator, ConstantData, ConvertValueOparg, Instruction, Invert, OpArg, OpArgType,
40+
UnpackExArgs,
4041
},
4142
};
4243
use rustpython_wtf8::Wtf8Buf;
@@ -5636,7 +5637,12 @@ impl Compiler {
56365637
}
56375638
}
56385639
InterpolatedStringElement::Interpolation(fstring_expr) => {
5639-
let mut conversion = fstring_expr.conversion;
5640+
let mut conversion = match fstring_expr.conversion {
5641+
ConversionFlag::None => ConvertValueOparg::None,
5642+
ConversionFlag::Str => ConvertValueOparg::Str,
5643+
ConversionFlag::Repr => ConvertValueOparg::Repr,
5644+
ConversionFlag::Ascii => ConvertValueOparg::Ascii,
5645+
};
56405646

56415647
if let Some(DebugText { leading, trailing }) = &fstring_expr.debug_text {
56425648
let range = fstring_expr.expression.range();
@@ -5645,35 +5651,39 @@ impl Compiler {
56455651

56465652
self.emit_load_const(ConstantData::Str { value: text.into() });
56475653
element_count += 1;
5654+
5655+
// Match CPython behavior: If debug text is present, apply repr conversion.
5656+
// if no `format_spec` specified.
5657+
// See: https://github.com/python/cpython/blob/f61afca262d3a0aa6a8a501db0b1936c60858e35/Parser/action_helpers.c#L1456
5658+
if matches!(
5659+
(conversion, &fstring_expr.format_spec),
5660+
(ConvertValueOparg::None, None)
5661+
) {
5662+
conversion = ConvertValueOparg::Repr;
5663+
}
56485664
}
56495665

5650-
match &fstring_expr.format_spec {
5651-
None => {
5652-
self.emit_load_const(ConstantData::Str {
5653-
value: Wtf8Buf::new(),
5654-
});
5655-
// Match CPython behavior: If debug text is present, apply repr conversion.
5656-
// See: https://github.com/python/cpython/blob/f61afca262d3a0aa6a8a501db0b1936c60858e35/Parser/action_helpers.c#L1456
5657-
if conversion == ConversionFlag::None
5658-
&& fstring_expr.debug_text.is_some()
5659-
{
5660-
conversion = ConversionFlag::Repr;
5661-
}
5666+
self.compile_expression(&fstring_expr.expression)?;
5667+
5668+
match conversion {
5669+
ConvertValueOparg::None => {}
5670+
ConvertValueOparg::Str
5671+
| ConvertValueOparg::Repr
5672+
| ConvertValueOparg::Ascii => {
5673+
emit!(self, Instruction::ConvertValue { oparg: conversion })
56625674
}
5675+
}
5676+
5677+
match &fstring_expr.format_spec {
56635678
Some(format_spec) => {
56645679
self.compile_fstring_elements(flags, &format_spec.elements)?;
5680+
5681+
emit!(self, Instruction::FormatWithSpec);
5682+
}
5683+
None => {
5684+
emit!(self, Instruction::FormatSimple);
56655685
}
56665686
}
5667-
5668-
self.compile_expression(&fstring_expr.expression)?;
5669-
5670-
let conversion = match conversion {
5671-
ConversionFlag::None => bytecode::ConversionFlag::None,
5672-
ConversionFlag::Str => bytecode::ConversionFlag::Str,
5673-
ConversionFlag::Ascii => bytecode::ConversionFlag::Ascii,
5674-
ConversionFlag::Repr => bytecode::ConversionFlag::Repr,
5675-
};
5676-
emit!(self, Instruction::FormatValue { conversion });
56775687
}
56785688
}
56795689
}

‎crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap‎

Lines changed: 16 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/compiler-core/src/bytecode.rs‎

Lines changed: 106 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,76 @@ use num_complex::Complex64;
1212
use rustpython_wtf8::{Wtf8, Wtf8Buf};
1313
use std::{collections::BTreeSet, fmt, hash, marker::PhantomData, mem, num::NonZeroU8, ops::Deref};
1414

15+
/// Oparg values for [`Instruction::ConvertValue`].
16+
///
17+
/// ## See also
18+
///
19+
/// - [CPython FVC_* flags](https://github.com/python/cpython/blob/8183fa5e3f78ca6ab862de7fb8b14f3d929421e0/Include/ceval.h#L129-L132)
20+
#[repr(u8)]
1521
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
16-
#[repr(i8)]
17-
#[allow(clippy::cast_possible_wrap)]
18-
pub enum ConversionFlag {
19-
/// No conversion
20-
None = -1, // CPython uses -1
22+
pub enum ConvertValueOparg {
23+
/// No conversion.
24+
///
25+
/// ```python
26+
/// f"{x}"
27+
/// f"{x:4}"
28+
/// ```
29+
None = 0,
2130
/// Converts by calling `str(<value>)`.
22-
Str = b's' as i8,
23-
/// Converts by calling `ascii(<value>)`.
24-
Ascii = b'a' as i8,
31+
///
32+
/// ```python
33+
/// f"{x!s}"
34+
/// f"{x!s:2}"
35+
/// ```
36+
Str = 1,
2537
/// Converts by calling `repr(<value>)`.
26-
Repr = b'r' as i8,
38+
///
39+
/// ```python
40+
/// f"{x!r}"
41+
/// f"{x!r:2}"
42+
/// ```
43+
Repr = 2,
44+
/// Converts by calling `ascii(<value>)`.
45+
///
46+
/// ```python
47+
/// f"{x!a}"
48+
/// f"{x!a:2}"
49+
/// ```
50+
Ascii = 3,
51+
}
52+
53+
impl fmt::Display for ConvertValueOparg {
54+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
55+
let out = match self {
56+
Self::Str => "1 (str)",
57+
Self::Repr => "2 (repr)",
58+
Self::Ascii => "3 (ascii)",
59+
// We should never reach this. `FVC_NONE` are being handled by `Instruction::FormatSimple`
60+
Self::None => "",
61+
};
62+
63+
write!(f, "{out}")
64+
}
65+
}
66+
67+
impl OpArgType for ConvertValueOparg {
68+
#[inline]
69+
fn from_op_arg(x: u32) -> Option<Self> {
70+
Some(match x {
71+
// Ruff `ConversionFlag::None` is `-1i8`,
72+
// when its converted to `u8` its value is `u8::MAX`
73+
0 | 255 => Self::None,
74+
1 => Self::Str,
75+
2 => Self::Repr,
76+
3 => Self::Ascii,
77+
_ => return None,
78+
})
79+
}
80+
81+
#[inline]
82+
fn to_op_arg(self) -> u32 {
83+
self as u32
84+
}
2785
}
2886

2987
/// Resume type for the RESUME instruction
@@ -476,24 +534,6 @@ impl fmt::Display for Label {
476534
}
477535
}
478536

479-
impl OpArgType for ConversionFlag {
480-
#[inline]
481-
fn from_op_arg(x: u32) -> Option<Self> {
482-
match x as u8 {
483-
b's' => Some(Self::Str),
484-
b'a' => Some(Self::Ascii),
485-
b'r' => Some(Self::Repr),
486-
std::u8::MAX => Some(Self::None),
487-
_ => None,
488-
}
489-
}
490-
491-
#[inline]
492-
fn to_op_arg(self) -> u32 {
493-
self as i8 as u8 as u32
494-
}
495-
}
496-
497537
op_arg_enum!(
498538
/// The kind of Raise that occurred.
499539
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
@@ -620,6 +660,18 @@ pub enum Instruction {
620660
Continue {
621661
target: Arg<Label>,
622662
},
663+
/// Convert value to a string, depending on `oparg`:
664+
///
665+
/// ```python
666+
/// value = STACK.pop()
667+
/// result = func(value)
668+
/// STACK.append(result)
669+
/// ```
670+
///
671+
/// Used for implementing formatted string literals (f-strings).
672+
ConvertValue {
673+
oparg: Arg<ConvertValueOparg>,
674+
},
623675
CopyItem {
624676
index: Arg<u32>,
625677
},
@@ -635,24 +687,40 @@ pub enum Instruction {
635687
index: Arg<u32>,
636688
},
637689
EndAsyncFor,
638-
639690
/// Marker bytecode for the end of a finally sequence.
640691
/// When this bytecode is executed, the eval loop does one of those things:
641692
/// - Continue at a certain bytecode position
642693
/// - Propagate the exception
643694
/// - Return from a function
644695
/// - Do nothing at all, just continue
645696
EndFinally,
646-
647697
/// Enter a finally block, without returning, excepting, just because we are there.
648698
EnterFinally,
649699
ExtendedArg,
650700
ForIter {
651701
target: Arg<Label>,
652702
},
653-
FormatValue {
654-
conversion: Arg<ConversionFlag>,
655-
},
703+
/// Formats the value on top of stack:
704+
///
705+
/// ```python
706+
/// value = STACK.pop()
707+
/// result = value.__format__("")
708+
/// STACK.append(result)
709+
/// ```
710+
///
711+
/// Used for implementing formatted string literals (f-strings).
712+
FormatSimple,
713+
/// Formats the given value with the given format spec:
714+
///
715+
/// ```python
716+
/// spec = STACK.pop()
717+
/// value = STACK.pop()
718+
/// result = value.__format__(spec)
719+
/// STACK.append(result)
720+
/// ```
721+
///
722+
/// Used for implementing formatted string literals (f-strings).
723+
FormatWithSpec,
656724
GetAIter,
657725
GetANext,
658726
GetAwaitable,
@@ -727,12 +795,10 @@ pub enum Instruction {
727795
PopJumpIfTrue {
728796
target: Arg<Label>,
729797
},
730-
731798
PrintExpr,
732799
Raise {
733800
kind: Arg<RaiseKind>,
734801
},
735-
736802
/// Resume execution (e.g., at function start, after yield, etc.)
737803
Resume {
738804
arg: Arg<u32>,
@@ -750,7 +816,6 @@ pub enum Instruction {
750816
SetFunctionAttribute {
751817
attr: Arg<MakeFunctionFlags>,
752818
},
753-
754819
SetupAnnotation,
755820
SetupAsyncWith {
756821
end: Arg<Label>,
@@ -759,7 +824,6 @@ pub enum Instruction {
759824
SetupExcept {
760825
handler: Arg<Label>,
761826
},
762-
763827
/// Setup a finally handler, which will be called whenever one of this events occurs:
764828
/// - the block is popped
765829
/// - the function returns
@@ -1656,6 +1720,9 @@ impl Instruction {
16561720
CallMethodKeyword { nargs } => -1 - (nargs.get(arg) as i32) - 3 + 1,
16571721
CallFunctionEx { has_kwargs } => -1 - (has_kwargs.get(arg) as i32) - 1 + 1,
16581722
CallMethodEx { has_kwargs } => -1 - (has_kwargs.get(arg) as i32) - 3 + 1,
1723+
ConvertValue { .. } => 0,
1724+
FormatSimple => 0,
1725+
FormatWithSpec => -1,
16591726
LoadMethod { .. } => -1 + 3,
16601727
ForIter { .. } => {
16611728
if jump {
@@ -1709,7 +1776,6 @@ impl Instruction {
17091776
let UnpackExArgs { before, after } = args.get(arg);
17101777
-1 + before as i32 + 1 + after as i32
17111778
}
1712-
FormatValue { .. } => -1,
17131779
PopException => 0,
17141780
Reverse { .. } => 0,
17151781
GetAwaitable => 0,
@@ -1824,6 +1890,7 @@ impl Instruction {
18241890
CompareOperation { op } => w!(CompareOperation, ?op),
18251891
ContainsOp(inv) => w!(CONTAINS_OP, ?inv),
18261892
Continue { target } => w!(Continue, target),
1893+
ConvertValue { oparg } => write!(f, "{:pad$}{}", "CONVERT_VALUE", oparg.get(arg)),
18271894
CopyItem { index } => w!(CopyItem, index),
18281895
DeleteAttr { idx } => w!(DeleteAttr, name = idx),
18291896
DeleteDeref(idx) => w!(DeleteDeref, cell_name = idx),
@@ -1837,7 +1904,8 @@ impl Instruction {
18371904
EnterFinally => w!(EnterFinally),
18381905
ExtendedArg => w!(ExtendedArg, Arg::<u32>::marker()),
18391906
ForIter { target } => w!(ForIter, target),
1840-
FormatValue { conversion } => w!(FormatValue, ?conversion),
1907+
FormatSimple => w!(FORMAT_SIMPLE),
1908+
FormatWithSpec => w!(FORMAT_WITH_SPEC),
18411909
GetAIter => w!(GetAIter),
18421910
GetANext => w!(GetANext),
18431911
GetAwaitable => w!(GetAwaitable),

0 commit comments

Comments
 (0)