diff --git a/bytecode/src/bytecode.rs b/bytecode/src/bytecode.rs index 7f84163b75..24e59a3f93 100644 --- a/bytecode/src/bytecode.rs +++ b/bytecode/src/bytecode.rs @@ -274,7 +274,6 @@ pub enum Instruction { }, FormatValue { conversion: Option, - spec: String, }, PopException, Reverse { @@ -564,7 +563,7 @@ impl Instruction { LoadBuildClass => w!(LoadBuildClass), UnpackSequence { size } => w!(UnpackSequence, size), UnpackEx { before, after } => w!(UnpackEx, before, after), - FormatValue { spec, .. } => w!(FormatValue, spec), // TODO: write conversion + FormatValue { .. } => w!(FormatValue), // TODO: write conversion PopException => w!(PopException), Reverse { amount } => w!(Reverse, amount), GetAwaitable => w!(GetAwaitable), diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index 24df923b7e..d02d3d3878 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -2108,10 +2108,17 @@ impl Compiler { conversion, spec, } => { + match spec { + Some(spec) => self.compile_string(spec)?, + None => self.emit(Instruction::LoadConst { + value: bytecode::Constant::String { + value: String::new(), + }, + }), + }; self.compile_expression(value)?; self.emit(Instruction::FormatValue { conversion: conversion.map(compile_conversion_flag), - spec: spec.clone(), }); } } diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index 2813854661..c3e2099bcf 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -726,8 +726,11 @@ impl SymbolTableBuilder { fn scan_string_group(&mut self, group: &ast::StringGroup) -> SymbolTableResult { match group { ast::StringGroup::Constant { .. } => {} - ast::StringGroup::FormattedValue { value, .. } => { + ast::StringGroup::FormattedValue { value, spec, .. } => { self.scan_expression(value, &ExpressionContext::Load)?; + if let Some(spec) = spec { + self.scan_string_group(spec)?; + } } ast::StringGroup::Joined { values } => { for subgroup in values { diff --git a/parser/src/ast.rs b/parser/src/ast.rs index c211d296ca..f8e104757c 100644 --- a/parser/src/ast.rs +++ b/parser/src/ast.rs @@ -505,7 +505,7 @@ pub enum StringGroup { FormattedValue { value: Box, conversion: Option, - spec: String, + spec: Option>, }, Joined { values: Vec, diff --git a/parser/src/error.rs b/parser/src/error.rs index 7b6e1369a6..86fa763874 100644 --- a/parser/src/error.rs +++ b/parser/src/error.rs @@ -81,6 +81,7 @@ pub enum FStringErrorType { InvalidConversionFlag, EmptyExpression, MismatchedDelimiter, + ExpressionNestedTooDeeply, } impl fmt::Display for FStringErrorType { @@ -94,6 +95,9 @@ impl fmt::Display for FStringErrorType { FStringErrorType::InvalidConversionFlag => write!(f, "Invalid conversion flag"), FStringErrorType::EmptyExpression => write!(f, "Empty expression"), FStringErrorType::MismatchedDelimiter => write!(f, "Mismatched delimiter"), + FStringErrorType::ExpressionNestedTooDeeply => { + write!(f, "expressions nested too deeply") + } } } } diff --git a/parser/src/fstring.rs b/parser/src/fstring.rs index e836c8c4bf..f5a2ad24b6 100644 --- a/parser/src/fstring.rs +++ b/parser/src/fstring.rs @@ -23,7 +23,7 @@ impl<'a> FStringParser<'a> { fn parse_formatted_value(&mut self) -> Result { let mut expression = String::new(); - let mut spec = String::new(); + let mut spec = None; let mut delims = Vec::new(); let mut conversion = None; @@ -43,13 +43,48 @@ impl<'a> FStringParser<'a> { }) } ':' if delims.is_empty() => { + let mut nested = false; + let mut in_nested = false; + let mut spec_expression = String::new(); while let Some(&next) = self.chars.peek() { - if next != '}' { - spec.push(next); - self.chars.next(); - } else { - break; + match next { + '{' => { + if in_nested { + return Err(ExpressionNestedTooDeeply); + } + in_nested = true; + nested = true; + self.chars.next(); + continue; + } + '}' => { + if in_nested { + in_nested = false; + self.chars.next(); + } + break; + } + _ => (), } + spec_expression.push(next); + self.chars.next(); + } + if in_nested { + return Err(UnclosedLbrace); + } + if nested { + spec = Some(Box::new(FormattedValue { + value: Box::new( + parse_expression(spec_expression.trim()) + .map_err(|e| InvalidExpression(Box::new(e.error)))?, + ), + conversion: None, + spec: None, + })) + } else { + spec = Some(Box::new(Constant { + value: spec_expression.trim().to_string(), + })) } } '(' | '{' | '[' => { @@ -194,12 +229,12 @@ mod tests { FormattedValue { value: Box::new(mk_ident("a", 1, 1)), conversion: None, - spec: String::new(), + spec: None, }, FormattedValue { value: Box::new(mk_ident("b", 1, 1)), conversion: None, - spec: String::new(), + spec: None, }, Constant { value: "{foo}".to_owned() @@ -209,6 +244,42 @@ mod tests { ); } + #[test] + fn test_parse_fstring_nested_spec() { + let source = String::from("{foo:{spec}}"); + let parse_ast = parse_fstring(&source).unwrap(); + + assert_eq!( + parse_ast, + FormattedValue { + value: Box::new(mk_ident("foo", 1, 1)), + conversion: None, + spec: Some(Box::new(FormattedValue { + value: Box::new(mk_ident("spec", 1, 1)), + conversion: None, + spec: None, + })), + } + ); + } + + #[test] + fn test_parse_fstring_not_nested_spec() { + let source = String::from("{foo:spec}"); + let parse_ast = parse_fstring(&source).unwrap(); + + assert_eq!( + parse_ast, + FormattedValue { + value: Box::new(mk_ident("foo", 1, 1)), + conversion: None, + spec: Some(Box::new(Constant { + value: "spec".to_string(), + })), + } + ); + } + #[test] fn test_parse_empty_fstring() { assert_eq!( @@ -223,6 +294,9 @@ mod tests { fn test_parse_invalid_fstring() { assert_eq!(parse_fstring("{"), Err(UnclosedLbrace)); assert_eq!(parse_fstring("}"), Err(UnopenedRbrace)); + assert_eq!(parse_fstring("{a:{a:{b}}"), Err(ExpressionNestedTooDeeply)); + assert_eq!(parse_fstring("{a:b}}"), Err(UnopenedRbrace)); + assert_eq!(parse_fstring("{a:{b}"), Err(UnclosedLbrace)); // TODO: check for InvalidExpression enum? assert!(parse_fstring("{class}").is_err()); diff --git a/tests/snippets/fstrings.py b/tests/snippets/fstrings.py index 67da5e1a7d..2d25abf6dc 100644 --- a/tests/snippets/fstrings.py +++ b/tests/snippets/fstrings.py @@ -1,3 +1,4 @@ +from testutils import assert_raises foo = 'bar' assert f"{''}" == '' @@ -16,6 +17,13 @@ assert f'{16:0>+#10x}' == '00000+0x10' assert f"{{{(lambda x: f'hello, {x}')('world}')}" == '{hello, world}' +spec = "0>+#10x" +assert f"{16:{spec}}{foo}" == '00000+0x10bar' + +# TODO: +# spec = "bla" +# assert_raises(ValueError, lambda: f"{16:{spec}}") + # Normally `!` cannot appear outside of delimiters in the expression but # cpython makes an exception for `!=`, so we should too. diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 307f93cc58..0aaa079e00 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -616,7 +616,7 @@ impl Frame { bytecode::Instruction::UnpackEx { before, after } => { self.execute_unpack_ex(vm, *before, *after) } - bytecode::Instruction::FormatValue { conversion, spec } => { + bytecode::Instruction::FormatValue { conversion } => { use bytecode::ConversionFlag::*; let value = match conversion { Some(Str) => vm.to_str(&self.pop_value())?.into_object(), @@ -625,7 +625,7 @@ impl Frame { None => self.pop_value(), }; - let spec = vm.new_str(spec.clone()); + let spec = vm.to_str(&self.pop_value())?.into_object(); let formatted = vm.call_method(&value, "__format__", vec![spec])?; self.push_value(formatted); Ok(None)