From 13914e0739e4d26ecac976c63257269863284324 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 2 Jan 2020 17:53:27 +0900 Subject: [PATCH] objstr::get_value -> objstr::clone_value because the previous naming gives unclear performance estimation to users especially comparing to objstr::borrow_value --- vm/src/frame.rs | 6 +++--- vm/src/obj/objdict.rs | 2 +- vm/src/obj/objstr.rs | 28 ++++++++++++++-------------- vm/src/py_serde.rs | 2 +- vm/src/stdlib/csv.rs | 20 ++++++++------------ vm/src/stdlib/imp.rs | 4 ++-- vm/src/stdlib/io.rs | 32 +++++++++++++++----------------- vm/src/stdlib/pystruct.rs | 4 ++-- vm/src/stdlib/subprocess.rs | 2 +- vm/src/stdlib/tokenize.rs | 4 ++-- vm/src/vm.rs | 2 +- 11 files changed, 50 insertions(+), 56 deletions(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index d3590260a..66de05318 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -301,7 +301,7 @@ impl Frame { let s = self .pop_multiple(*size) .into_iter() - .map(|pyobj| objstr::get_value(&pyobj)) + .map(|pyobj| objstr::clone_value(&pyobj)) .collect::(); let str_obj = vm.ctx.new_str(s); self.push_value(str_obj); @@ -929,7 +929,7 @@ impl Frame { let kwarg_names = vm .extract_elements(&kwarg_names)? .iter() - .map(|pyobj| objstr::get_value(pyobj)) + .map(|pyobj| objstr::clone_value(pyobj)) .collect(); PyFuncArgs::new(args, kwarg_names) } @@ -939,7 +939,7 @@ impl Frame { self.pop_value().downcast().expect("Kwargs must be a dict."); kw_dict .into_iter() - .map(|elem| (objstr::get_value(&elem.0), elem.1)) + .map(|elem| (objstr::clone_value(&elem.0), elem.1)) .collect() } else { IndexMap::new() diff --git a/vm/src/obj/objdict.rs b/vm/src/obj/objdict.rs index c25c68b71..eb61d7b2b 100644 --- a/vm/src/obj/objdict.rs +++ b/vm/src/obj/objdict.rs @@ -332,7 +332,7 @@ impl PyDictRef { pub fn to_attributes(self) -> PyAttributes { let mut attrs = PyAttributes::new(); for (key, value) in self { - let key = objstr::get_value(&key); + let key = objstr::clone_value(&key); attrs.insert(key, value); } attrs diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 0e0f3fb22..15d09a16c 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -214,7 +214,7 @@ impl PyString { #[pymethod(name = "__add__")] fn add(&self, rhs: PyObjectRef, vm: &VirtualMachine) -> PyResult { if objtype::isinstance(&rhs, &vm.ctx.str_type()) { - Ok(format!("{}{}", self.value, get_value(&rhs))) + Ok(format!("{}{}", self.value, borrow_value(&rhs))) } else { Err(vm.new_type_error(format!("Cannot add {} and {}", self, rhs))) } @@ -228,7 +228,7 @@ impl PyString { #[pymethod(name = "__eq__")] fn eq(&self, rhs: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef { if objtype::isinstance(&rhs, &vm.ctx.str_type()) { - vm.new_bool(self.value == get_value(&rhs)) + vm.new_bool(self.value == borrow_value(&rhs)) } else { vm.ctx.not_implemented() } @@ -237,7 +237,7 @@ impl PyString { #[pymethod(name = "__ne__")] fn ne(&self, rhs: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef { if objtype::isinstance(&rhs, &vm.ctx.str_type()) { - vm.new_bool(self.value != get_value(&rhs)) + vm.new_bool(self.value != borrow_value(&rhs)) } else { vm.ctx.not_implemented() } @@ -623,8 +623,8 @@ impl PyString { actual_type ))); } - let format_string_text = get_value(zelf); - match FormatString::from_str(format_string_text.as_str()) { + let format_string_text = borrow_value(zelf); + match FormatString::from_str(format_string_text) { Ok(format_string) => perform_format(vm, &format_string, &args), Err(err) => match err { FormatParseError::UnmatchedBracket => { @@ -649,8 +649,8 @@ impl PyString { } let zelf = &args.args[0]; - let format_string_text = get_value(zelf); - match FormatString::from_str(format_string_text.as_str()) { + let format_string_text = borrow_value(zelf); + match FormatString::from_str(format_string_text) { Ok(format_string) => perform_format_map(vm, &format_string, &args.args[1]), Err(err) => match err { FormatParseError::UnmatchedBracket => { @@ -1288,7 +1288,7 @@ pub fn init(ctx: &PyContext) { PyStringReverseIterator::extend_class(ctx, &ctx.types.strreverseiterator_type); } -pub fn get_value(obj: &PyObjectRef) -> String { +pub fn clone_value(obj: &PyObjectRef) -> String { obj.payload::().unwrap().value.clone() } @@ -1341,7 +1341,7 @@ fn do_cformat_specifier( CFormatPreconversor::Ascii => vm.call_method(&obj.clone(), "__repr__", vec![])?, CFormatPreconversor::Bytes => vm.call_method(&obj.clone(), "decode", vec![])?, }; - Ok(format_spec.format_string(get_value(&result))) + Ok(format_spec.format_string(clone_value(&result))) } CFormatType::Number(_) => { if !objtype::isinstance(&obj, &vm.ctx.int_type()) { @@ -1384,7 +1384,7 @@ fn do_cformat_specifier( } } } else if objtype::isinstance(&obj, &vm.ctx.str_type()) { - let s: String = get_value(&obj); + let s = borrow_value(&obj); let num_chars = s.chars().count(); if num_chars != 1 { Err(vm.new_type_error("%c requires int or char".to_string())) @@ -1573,7 +1573,7 @@ fn perform_format( } }; auto_argument_index += 1; - get_value(&result) + clone_value(&result) } FormatPart::IndexSpec(index, format_spec) => { let result = match arguments.args.get(*index + 1) { @@ -1582,7 +1582,7 @@ fn perform_format( return Err(vm.new_index_error("tuple index out of range".to_string())); } }; - get_value(&result) + clone_value(&result) } FormatPart::KeywordSpec(keyword, format_spec) => { let result = match arguments.get_optional_kwarg(&keyword) { @@ -1591,7 +1591,7 @@ fn perform_format( return Err(vm.new_key_error(vm.new_str(keyword.to_string()))); } }; - get_value(&result) + clone_value(&result) } FormatPart::Literal(literal) => literal.clone(), }; @@ -1616,7 +1616,7 @@ fn perform_format_map( FormatPart::KeywordSpec(keyword, format_spec) => { let argument = dict.get_item(keyword, &vm)?; let result = call_object_format(vm, argument.clone(), &format_spec)?; - get_value(&result) + clone_value(&result) } FormatPart::Literal(literal) => literal.clone(), }; diff --git a/vm/src/py_serde.rs b/vm/src/py_serde.rs index 92200da5b..44b9df56e 100644 --- a/vm/src/py_serde.rs +++ b/vm/src/py_serde.rs @@ -67,7 +67,7 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> { seq.end() }; if objtype::isinstance(self.pyobject, &self.vm.ctx.str_type()) { - serializer.serialize_str(&objstr::get_value(&self.pyobject)) + serializer.serialize_str(objstr::borrow_value(&self.pyobject)) } else if objtype::isinstance(self.pyobject, &self.vm.ctx.float_type()) { serializer.serialize_f64(objfloat::get_value(self.pyobject)) } else if objtype::isinstance(self.pyobject, &self.vm.ctx.bool_type()) { diff --git a/vm/src/stdlib/csv.rs b/vm/src/stdlib/csv.rs index d937379bb..709e15a90 100644 --- a/vm/src/stdlib/csv.rs +++ b/vm/src/stdlib/csv.rs @@ -29,12 +29,8 @@ struct ReaderOption { impl ReaderOption { fn new(args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { - let delimiter = { - let bytes = args - .get_optional_kwarg("delimiter") - .map_or(",".to_string(), |pyobj| objstr::get_value(&pyobj)) - .into_bytes(); - + let delimiter = if let Some(delimiter) = args.get_optional_kwarg("delimiter") { + let bytes = objstr::borrow_value(&delimiter).as_bytes(); match bytes.len() { 1 => bytes[0], _ => { @@ -42,14 +38,12 @@ impl ReaderOption { return Err(vm.new_type_error(msg.to_string())); } } + } else { + b',' }; - let quotechar = { - let bytes = args - .get_optional_kwarg("quotechar") - .map_or("\"".to_string(), |pyobj| objstr::get_value(&pyobj)) - .into_bytes(); - + let quotechar = if let Some(quotechar) = args.get_optional_kwarg("quotechar") { + let bytes = objstr::borrow_value("echar).as_bytes(); match bytes.len() { 1 => bytes[0], _ => { @@ -57,6 +51,8 @@ impl ReaderOption { return Err(vm.new_type_error(msg.to_string())); } } + } else { + b'"' }; Ok(ReaderOption { diff --git a/vm/src/stdlib/imp.rs b/vm/src/stdlib/imp.rs index 87f2179f9..e656c12a1 100644 --- a/vm/src/stdlib/imp.rs +++ b/vm/src/stdlib/imp.rs @@ -35,8 +35,8 @@ fn imp_is_frozen(name: PyStringRef, vm: &VirtualMachine) -> bool { fn imp_create_builtin(spec: PyObjectRef, vm: &VirtualMachine) -> PyResult { let sys_modules = vm.get_attribute(vm.sys_module.clone(), "modules").unwrap(); - - let name = &objstr::get_value(&vm.get_attribute(spec.clone(), "name")?); + let spec = vm.get_attribute(spec.clone(), "name")?; + let name = objstr::borrow_value(&spec); if let Ok(module) = sys_modules.get_item(name, vm) { Ok(module) diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs index dc53d053c..1884724a4 100644 --- a/vm/src/stdlib/io.rs +++ b/vm/src/stdlib/io.rs @@ -209,13 +209,11 @@ fn string_io_new( _args: StringIOArgs, vm: &VirtualMachine, ) -> PyResult { - let raw_string = match object { - OptionalArg::Present(Some(ref input)) => objstr::get_value(input), - _ => String::new(), - }; + let flatten = object.flat_option(); + let input = flatten.map_or_else(Vec::new, |v| objstr::borrow_value(&v).as_bytes().to_vec()); PyStringIO { - buffer: RefCell::new(Some(BufferedIO::new(Cursor::new(raw_string.into_bytes())))), + buffer: RefCell::new(Some(BufferedIO::new(Cursor::new(input)))), } .into_ref_with_type(vm, cls) } @@ -816,7 +814,7 @@ fn text_io_wrapper_readline( Ok(rust_string) } -fn split_mode_string(mode_string: String) -> Result<(String, String), String> { +fn split_mode_string(mode_string: &str) -> Result<(String, String), String> { let mut mode: char = '\0'; let mut typ: char = '\0'; let mut plus_is_set = false; @@ -882,7 +880,7 @@ pub fn io_open(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult { ); // mode is optional: 'rt' is the default mode (open from reading text) - let mode_string = mode.map_or("rt".to_string(), objstr::get_value); + let mode_string = mode.map_or("rt", objstr::borrow_value); let (mode, typ) = match split_mode_string(mode_string) { Ok((mode, typ)) => (mode, typ), @@ -1066,7 +1064,7 @@ mod tests { use super::*; fn assert_mode_split_into(mode_string: &str, expected_mode: &str, expected_typ: &str) { - let (mode, typ) = split_mode_string(mode_string.to_string()).unwrap(); + let (mode, typ) = split_mode_string(mode_string).unwrap(); assert_eq!(mode, expected_mode); assert_eq!(typ, expected_typ); } @@ -1085,15 +1083,15 @@ mod tests { #[test] fn test_invalid_mode() { assert_eq!( - split_mode_string("rbsss".to_string()), + split_mode_string("rbsss"), Err("invalid mode: 'rbsss'".to_string()) ); assert_eq!( - split_mode_string("rrb".to_string()), + split_mode_string("rrb"), Err("invalid mode: 'rrb'".to_string()) ); assert_eq!( - split_mode_string("rbb".to_string()), + split_mode_string("rbb"), Err("invalid mode: 'rbb'".to_string()) ); } @@ -1101,21 +1099,21 @@ mod tests { #[test] fn test_mode_not_specified() { assert_eq!( - split_mode_string("".to_string()), + split_mode_string(""), Err( "Must have exactly one of create/read/write/append mode and at most one plus" .to_string() ) ); assert_eq!( - split_mode_string("b".to_string()), + split_mode_string("b"), Err( "Must have exactly one of create/read/write/append mode and at most one plus" .to_string() ) ); assert_eq!( - split_mode_string("t".to_string()), + split_mode_string("t"), Err( "Must have exactly one of create/read/write/append mode and at most one plus" .to_string() @@ -1126,7 +1124,7 @@ mod tests { #[test] fn test_text_and_binary_at_once() { assert_eq!( - split_mode_string("rbt".to_string()), + split_mode_string("rbt"), Err("can't have text and binary mode at once".to_string()) ); } @@ -1134,7 +1132,7 @@ mod tests { #[test] fn test_exactly_one_mode() { assert_eq!( - split_mode_string("rwb".to_string()), + split_mode_string("rwb"), Err("must have exactly one of create/read/write/append mode".to_string()) ); } @@ -1142,7 +1140,7 @@ mod tests { #[test] fn test_at_most_one_plus() { assert_eq!( - split_mode_string("a++".to_string()), + split_mode_string("a++"), Err("invalid mode: 'a++'".to_string()) ); } diff --git a/vm/src/stdlib/pystruct.rs b/vm/src/stdlib/pystruct.rs index f7d17a53a..a5ca6f498 100644 --- a/vm/src/stdlib/pystruct.rs +++ b/vm/src/stdlib/pystruct.rs @@ -213,7 +213,7 @@ fn struct_pack(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult { } else { let fmt_arg = args.args[0].clone(); if objtype::isinstance(&fmt_arg, &vm.ctx.str_type()) { - let fmt_str = objstr::get_value(&fmt_arg); + let fmt_str = objstr::clone_value(&fmt_arg); let format_spec = parse_format_string(fmt_str).map_err(|e| vm.new_value_error(e))?; @@ -364,7 +364,7 @@ fn struct_unpack(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult { ] ); - let fmt_str = objstr::get_value(&fmt); + let fmt_str = objstr::clone_value(&fmt); let format_spec = parse_format_string(fmt_str).map_err(|e| vm.new_value_error(e))?; let data = objbytes::get_value(buffer).to_vec(); diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index bf6738694..87f552d18 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -109,7 +109,7 @@ impl PopenRef { Either::A(command) => vec![command.as_str().to_string()], Either::B(command_list) => objsequence::get_elements_list(command_list.as_object()) .iter() - .map(|x| objstr::get_value(x)) + .map(|x| objstr::clone_value(x)) .collect(), }; let cwd = args.cwd.map(|x| OsString::from(x.as_str())); diff --git a/vm/src/stdlib/tokenize.rs b/vm/src/stdlib/tokenize.rs index 000dd5e1a..f0160b939 100644 --- a/vm/src/stdlib/tokenize.rs +++ b/vm/src/stdlib/tokenize.rs @@ -13,10 +13,10 @@ use crate::vm::VirtualMachine; fn tokenize_tokenize(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult { arg_check!(vm, args, required = [(readline, Some(vm.ctx.str_type()))]); - let source = objstr::get_value(readline); + let source = objstr::borrow_value(readline); // TODO: implement generator when the time has come. - let lexer1 = lexer::make_tokenizer(&source); + let lexer1 = lexer::make_tokenizer(source); let tokens = lexer1.map(|st| vm.ctx.new_str(format!("{:?}", st.unwrap().1))); let tokens = Vec::from_iter(tokens); diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 488f168fb..6779be9d5 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -1523,7 +1523,7 @@ mod tests { let a = vm.ctx.new_str(String::from("Hello ")); let b = vm.ctx.new_int(4_i32); let res = vm._mul(a, b).unwrap(); - let value = objstr::get_value(&res); + let value = objstr::borrow_value(&res); assert_eq!(value, String::from("Hello Hello Hello Hello ")) } }