From 3514594c566c3b2bb35667c91a459c7cb0986201 Mon Sep 17 00:00:00 2001
From: Anand Krishnamoorthi <35780660+anakrish@users.noreply.github.com>
Date: Fri, 24 Nov 2023 17:45:49 -0800
Subject: [PATCH] Use Rc<str> instead of string. (#52)

This reduces std::mem::size_of::<Value>() to 16 bytes.
String values are also efficiently copied like Objects, arrays, sets etc.

When multiple function rules are evaluated, ensure that constant argument values
match before running the rule. If actual parameter does not match the constant parameter,
then the rule is skipped.

Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
---
 src/builtins/encoding.rs |  8 +++--
 src/builtins/strings.rs  | 67 ++++++++++++++++++++--------------------
 src/builtins/types.rs    |  2 +-
 src/builtins/units.rs    |  4 +--
 src/builtins/utils.rs    |  4 +--
 src/interpreter.rs       | 61 ++++++++++++++++++++++++++----------
 src/value.rs             | 14 ++++-----
 tests/interpreter/mod.rs |  2 +-
 tests/parser/mod.rs      | 34 ++++++++++----------
 tests/value/mod.rs       | 10 +++---
 10 files changed, 117 insertions(+), 89 deletions(-)

diff --git a/src/builtins/encoding.rs b/src/builtins/encoding.rs
index 87ad94c..16a7707 100644
--- a/src/builtins/encoding.rs
+++ b/src/builtins/encoding.rs
@@ -29,7 +29,7 @@ fn base64_decode(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Va
     let encoded_str = ensure_string(name, &params[0], &args[0])?;
     let decoded_bytes = BASE64.decode(encoded_str.as_bytes())?;
     Ok(Value::String(
-        String::from_utf8_lossy(&decoded_bytes).to_string(),
+        String::from_utf8_lossy(&decoded_bytes).into(),
     ))
 }
 
@@ -46,7 +46,8 @@ fn yaml_marshal(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Val
     ensure_args_count(span, name, params, args, 1)?;
     Ok(Value::String(
         serde_yaml::to_string(&args[0])
-            .with_context(|| span.error("could not serialize to yaml"))?,
+            .with_context(|| span.error("could not serialize to yaml"))?
+            .into(),
     ))
 }
 
@@ -70,7 +71,8 @@ fn json_marshal(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Val
     ensure_args_count(span, name, params, args, 1)?;
     Ok(Value::String(
         serde_json::to_string(&args[0])
-            .with_context(|| span.error("could not serialize to json"))?,
+            .with_context(|| span.error("could not serialize to json"))?
+            .into(),
     ))
 }
 
diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs
index 0bc8f35..8a4ff4b 100644
--- a/src/builtins/strings.rs
+++ b/src/builtins/strings.rs
@@ -46,7 +46,7 @@ fn concat(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     ensure_args_count(span, name, params, args, 2)?;
     let delimiter = ensure_string(name, &params[0], &args[0])?;
     let collection = ensure_string_collection(name, &params[1], &args[1])?;
-    Ok(Value::String(collection.join(&delimiter)))
+    Ok(Value::String(collection.join(&delimiter).into()))
 }
 
 fn contains(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -54,7 +54,7 @@ fn contains(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value>
     ensure_args_count(span, name, params, args, 2)?;
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
-    Ok(Value::Bool(s1.contains(&s2)))
+    Ok(Value::Bool(s1.contains(s2.as_ref())))
 }
 
 fn endswith(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -62,7 +62,7 @@ fn endswith(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value>
     ensure_args_count(span, name, params, args, 2)?;
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
-    Ok(Value::Bool(s1.ends_with(&s2)))
+    Ok(Value::Bool(s1.ends_with(s2.as_ref())))
 }
 
 fn format_int(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -77,14 +77,15 @@ fn format_int(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value
     let n = n.floor() as u64;
 
     Ok(Value::String(
-        sign.to_owned()
+        (sign.to_owned()
             + &match ensure_numeric(name, &params[1], &args[1])? as u64 {
                 2 => format!("{:b}", n),
                 8 => format!("{:o}", n),
                 10 => format!("{}", n),
                 16 => format!("{:x}", n),
                 _ => return Ok(Value::Undefined),
-            },
+            })
+            .into(),
     ))
 }
 
@@ -93,7 +94,7 @@ fn indexof(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     ensure_args_count(span, name, params, args, 2)?;
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
-    Ok(Value::from_float(match s1.find(&s2) {
+    Ok(Value::from_float(match s1.find(s2.as_ref()) {
         Some(pos) => pos as i64,
         _ => -1,
     } as Float))
@@ -109,7 +110,7 @@ fn indexof_n(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value>
     let mut positions = vec![];
     let mut idx = 0;
     while idx < s1.len() {
-        if let Some(pos) = s1.find(&s2) {
+        if let Some(pos) = s1.find(s2.as_ref()) {
             positions.push(Value::from_float(pos as Float));
             idx = pos + 1;
         } else {
@@ -123,7 +124,7 @@ fn lower(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     let name = "lower";
     ensure_args_count(span, name, params, args, 1)?;
     let s = ensure_string(name, &params[0], &args[0])?;
-    Ok(Value::String(s.to_lowercase()))
+    Ok(Value::String(s.to_lowercase().into()))
 }
 
 fn replace(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -132,7 +133,7 @@ fn replace(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     let s = ensure_string(name, &params[0], &args[0])?;
     let old = ensure_string(name, &params[1], &args[1])?;
     let new = ensure_string(name, &params[2], &args[2])?;
-    Ok(Value::String(s.replace(&old, &new)))
+    Ok(Value::String(s.replace(old.as_ref(), new.as_ref()).into()))
 }
 
 fn split(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -142,8 +143,8 @@ fn split(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     let delimiter = ensure_string(name, &params[1], &args[1])?;
 
     Ok(Value::from_array(
-        s.split(&delimiter)
-            .map(|s| Value::String(s.to_string()))
+        s.split(delimiter.as_ref())
+            .map(|s| Value::String(s.into()))
             .collect(),
     ))
 }
@@ -285,7 +286,7 @@ fn sprintf(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
                 bail!(args_span.error("floating-point number specified for format verb {verb}."));
             }
 
-            ('s', Value::String(sv)) => s += &sv.to_string(),
+            ('s', Value::String(sv)) => s += sv.as_ref(),
             ('s', _) => {
                 bail!(args_span.error("invalid non string argument specified for format verb %s"));
             }
@@ -312,7 +313,7 @@ fn sprintf(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
         ));
     }
 
-    Ok(Value::String(s.to_string()))
+    Ok(Value::String(s.into()))
 }
 
 fn any_prefix_match(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -320,7 +321,7 @@ fn any_prefix_match(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result
     ensure_args_count(span, name, params, args, 2)?;
 
     let search = match &args[0] {
-        Value::String(s) => vec![s.as_str()],
+        Value::String(s) => vec![s.as_ref()],
         Value::Array(_) | Value::Set(_) => ensure_string_collection(name, &params[0], &args[0])?,
         _ => {
             let span = params[0].span();
@@ -331,7 +332,7 @@ fn any_prefix_match(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result
     };
 
     let base = match &args[1] {
-        Value::String(s) => vec![s.as_str()],
+        Value::String(s) => vec![s.as_ref()],
         Value::Array(_) | Value::Set(_) => ensure_string_collection(name, &params[1], &args[1])?,
         _ => {
             let span = params[0].span();
@@ -351,7 +352,7 @@ fn any_suffix_match(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result
     ensure_args_count(span, name, params, args, 2)?;
 
     let search = match &args[0] {
-        Value::String(s) => vec![s.as_str()],
+        Value::String(s) => vec![s.as_ref()],
         Value::Array(_) | Value::Set(_) => ensure_string_collection(name, &params[0], &args[0])?,
         _ => {
             let span = params[0].span();
@@ -362,7 +363,7 @@ fn any_suffix_match(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result
     };
 
     let base = match &args[1] {
-        Value::String(s) => vec![s.as_str()],
+        Value::String(s) => vec![s.as_ref()],
         Value::Array(_) | Value::Set(_) => ensure_string_collection(name, &params[1], &args[1])?,
         _ => {
             let span = params[0].span();
@@ -382,7 +383,7 @@ fn startswith(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value
     ensure_args_count(span, name, params, args, 2)?;
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
-    Ok(Value::Bool(s1.starts_with(&s2)))
+    Ok(Value::Bool(s1.starts_with(s2.as_ref())))
 }
 
 fn replace_n(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -395,7 +396,7 @@ fn replace_n(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value>
     for item in obj.as_ref().iter() {
         match item {
             (Value::String(k), Value::String(v)) => {
-                s = s.replace(k, v);
+                s = s.replace(k.as_ref(), v.as_ref()).into();
             }
             _ => {
                 bail!(span.error(
@@ -405,14 +406,14 @@ fn replace_n(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value>
         }
     }
 
-    Ok(Value::String(s))
+    Ok(Value::String(s.clone()))
 }
 
 fn reverse(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     let name = "reverse";
     ensure_args_count(span, name, params, args, 1)?;
     let s = ensure_string(name, &params[0], &args[0])?;
-    Ok(Value::String(s.chars().rev().collect()))
+    Ok(Value::String(s.chars().rev().collect::<String>().into()))
 }
 
 fn substring(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -437,10 +438,10 @@ fn substring(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value>
     // Also: behavior of
     // x = substring("hello", 20 + 0.0, 25)
     if offset > s.len() || length <= offset {
-        return Ok(Value::String("".to_string()));
+        return Ok(Value::String("".into()));
     }
 
-    Ok(Value::String(s[offset..offset + length].to_string()))
+    Ok(Value::String(s[offset..offset + length].into()))
 }
 
 fn trim(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -448,9 +449,7 @@ fn trim(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     ensure_args_count(span, name, params, args, 2)?;
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
-    Ok(Value::String(
-        s1.trim_matches(|c| s2.contains(c)).to_string(),
-    ))
+    Ok(Value::String(s1.trim_matches(|c| s2.contains(c)).into()))
 }
 
 fn trim_left(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -459,7 +458,7 @@ fn trim_left(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value>
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
     Ok(Value::String(
-        s1.trim_start_matches(|c| s2.contains(c)).to_string(),
+        s1.trim_start_matches(|c| s2.contains(c)).into(),
     ))
 }
 
@@ -468,8 +467,8 @@ fn trim_prefix(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Valu
     ensure_args_count(span, name, params, args, 2)?;
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
-    Ok(Value::String(match s1.strip_prefix(&s2) {
-        Some(s) => s.to_string(),
+    Ok(Value::String(match s1.strip_prefix(s2.as_ref()) {
+        Some(s) => s.into(),
         _ => s1,
     }))
 }
@@ -480,7 +479,7 @@ fn trim_right(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
     Ok(Value::String(
-        s1.trim_end_matches(|c| s2.contains(c)).to_string(),
+        s1.trim_end_matches(|c| s2.contains(c)).into(),
     ))
 }
 
@@ -488,7 +487,7 @@ fn trim_space(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value
     let name = "trim_space";
     ensure_args_count(span, name, params, args, 1)?;
     let s = ensure_string(name, &params[0], &args[0])?;
-    Ok(Value::String(s.trim().to_string()))
+    Ok(Value::String(s.trim().into()))
 }
 
 fn trim_suffix(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
@@ -496,8 +495,8 @@ fn trim_suffix(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Valu
     ensure_args_count(span, name, params, args, 2)?;
     let s1 = ensure_string(name, &params[0], &args[0])?;
     let s2 = ensure_string(name, &params[1], &args[1])?;
-    Ok(Value::String(match s1.strip_suffix(&s2) {
-        Some(s) => s.to_string(),
+    Ok(Value::String(match s1.strip_suffix(s2.as_ref()) {
+        Some(s) => s.into(),
         _ => s1,
     }))
 }
@@ -506,5 +505,5 @@ fn upper(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     let name = "upper";
     ensure_args_count(span, name, params, args, 1)?;
     let s = ensure_string(name, &params[0], &args[0])?;
-    Ok(Value::String(s.to_uppercase()))
+    Ok(Value::String(s.to_uppercase().into()))
 }
diff --git a/src/builtins/types.rs b/src/builtins/types.rs
index 09a9f3b..b58bb6a 100644
--- a/src/builtins/types.rs
+++ b/src/builtins/types.rs
@@ -72,5 +72,5 @@ pub fn get_type(value: &Value) -> &str {
 
 pub fn type_name(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     ensure_args_count(span, "type_name", params, args, 1)?;
-    Ok(Value::String(get_type(&args[0]).to_string()))
+    Ok(Value::String(get_type(&args[0]).into()))
 }
diff --git a/src/builtins/units.rs b/src/builtins/units.rs
index f5d1733..04eb0cb 100644
--- a/src/builtins/units.rs
+++ b/src/builtins/units.rs
@@ -21,7 +21,7 @@ fn parse(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Value> {
     let name = "units.parse";
     ensure_args_count(span, name, params, args, 1)?;
     let string = ensure_string(name, &params[0], &args[0])?;
-    let string = string.as_str();
+    let string = string.as_ref();
 
     // Remove quotes.
     let string = if string.starts_with('"') && string.ends_with('"') && string.len() >= 2 {
@@ -101,7 +101,7 @@ fn parse_bytes(span: &Span, params: &[Ref<Expr>], args: &[Value]) -> Result<Valu
     let name = "units.parse_bytes";
     ensure_args_count(span, name, params, args, 1)?;
     let string = ensure_string(name, &params[0], &args[0])?;
-    let string = string.as_str();
+    let string = string.as_ref();
 
     // Remove quotes.
     let string = if string.starts_with('"') && string.ends_with('"') && string.len() >= 2 {
diff --git a/src/builtins/utils.rs b/src/builtins/utils.rs
index eb5d6ca..467d3cc 100644
--- a/src/builtins/utils.rs
+++ b/src/builtins/utils.rs
@@ -43,7 +43,7 @@ pub fn ensure_numeric(fcn: &str, arg: &Expr, v: &Value) -> Result<Float> {
     })
 }
 
-pub fn ensure_string(fcn: &str, arg: &Expr, v: &Value) -> Result<String> {
+pub fn ensure_string(fcn: &str, arg: &Expr, v: &Value) -> Result<Rc<str>> {
     Ok(match &v {
         Value::String(s) => s.clone(),
         _ => {
@@ -60,7 +60,7 @@ pub fn ensure_string_element<'a>(
     idx: usize,
 ) -> Result<&'a str> {
     Ok(match &v {
-        Value::String(s) => s.as_str(),
+        Value::String(s) => s.as_ref(),
         _ => {
             let span = arg.span();
             bail!(span.error(
diff --git a/src/interpreter.rs b/src/interpreter.rs
index 1c3a45d..efcbd08 100644
--- a/src/interpreter.rs
+++ b/src/interpreter.rs
@@ -42,7 +42,7 @@ pub struct Interpreter {
     active_rules: Vec<Ref<Rule>>,
     builtins_cache: BTreeMap<(&'static str, Vec<Value>), Value>,
     no_rules_lookup: bool,
-    traces: Option<Vec<String>>,
+    traces: Option<Vec<std::rc::Rc<str>>>,
     allow_deprecated: bool,
 }
 
@@ -807,20 +807,20 @@ impl Interpreter {
     fn make_expression_result(span: &Span, v: &Value) -> Value {
         let mut loc = BTreeMap::new();
         loc.insert(
-            Value::String("row".to_string()),
+            Value::String("row".into()),
             Value::from_float(span.line as f64),
         );
         loc.insert(
-            Value::String("col".to_string()),
+            Value::String("col".into()),
             Value::from_float(span.col as f64),
         );
 
         let mut expr = BTreeMap::new();
-        expr.insert(Value::String("value".to_string()), v.clone());
-        expr.insert(Value::String("location".to_string()), Value::from_map(loc));
+        expr.insert(Value::String("value".into()), v.clone());
+        expr.insert(Value::String("location".into()), Value::from_map(loc));
         expr.insert(
-            Value::String("text".to_string()),
-            Value::String(span.text().to_string()),
+            Value::String("text".into()),
+            Value::String(span.text().to_string().into()),
         );
         Value::from_map(expr)
     }
@@ -1196,7 +1196,7 @@ impl Interpreter {
                         result
                             .bindings
                             .as_object_mut()?
-                            .insert(Value::String(name.to_string()), value.clone());
+                            .insert(Value::String(name.to_string().into()), value.clone());
                     }
                 }
                 if result.expressions.iter().all(|v| v != &Value::Undefined)
@@ -1315,7 +1315,7 @@ impl Interpreter {
                         result
                             .bindings
                             .as_object_mut()?
-                            .insert(Value::String(name.to_string()), value.clone());
+                            .insert(Value::String(name.to_string().into()), value.clone());
                     }
                 }
                 if result.expressions.iter().all(|v| v != &Value::Undefined)
@@ -1594,7 +1594,16 @@ impl Interpreter {
 
         let mut results: Vec<Value> = Vec::new();
         let mut errors: Vec<anyhow::Error> = Vec::new();
-        for fcn_rule in fcns {
+        let mut param_values = Vec::with_capacity(params.len());
+        for p in params {
+            let v = self.eval_expr(p)?;
+            if v == Value::Undefined {
+                return Ok(v);
+            }
+            param_values.push(v);
+        }
+
+        'outer: for fcn_rule in fcns {
             let (args, output_expr, bodies) = match fcn_rule.as_ref() {
                 Rule::Spec {
                     head: RuleHead::Func { args, assign, .. },
@@ -1621,11 +1630,25 @@ impl Interpreter {
             for (idx, a) in args.iter().enumerate() {
                 let a = match a.as_ref() {
                     Expr::Var(s) => s.source_str(),
-                    _ => continue,
-                    //                    _ => unimplemented!("destructuring function arguments"),
+                    _ => {
+                        match self.eval_expr(a) {
+                            Ok(a) => {
+                                if a != param_values[idx] {
+                                    // Skip this rule definition.
+                                    continue 'outer;
+                                }
+
+                                continue;
+                            }
+                            _ => {
+                                // TODO: destructuring function arguments.
+                                continue;
+                            }
+                        }
+                    }
                 };
                 //TODO: check call in params
-                args_scope.insert(a, self.eval_expr(&params[idx])?);
+                args_scope.insert(a, param_values[idx].clone());
             }
 
             let ctx = Context {
@@ -1763,6 +1786,7 @@ impl Interpreter {
                 }
             }
         }
+
         Ok(())
     }
 
@@ -1851,7 +1875,7 @@ impl Interpreter {
                     Err(e) => bail!(span.error(format!("invalid string literal. {e}").as_str())),
                 }
             }
-            Expr::RawString(span) => Ok(Value::String(span.text().to_string())),
+            Expr::RawString(span) => Ok(Value::String(span.text().to_string().into())),
 
             // TODO: Handle undefined variables
             Expr::Var(_) => self.eval_chained_ref_dot_or_brack(expr),
@@ -2009,7 +2033,7 @@ impl Interpreter {
 
     fn get_value_chained(mut obj: Value, path: &[&str]) -> Value {
         for p in path {
-            obj = obj[&Value::String(p.to_string())].clone();
+            obj = obj[&Value::String(p.to_string().into())].clone();
         }
         obj
     }
@@ -2020,7 +2044,7 @@ impl Interpreter {
             return Ok(obj);
         }
 
-        let key = Value::String(paths[0].to_owned());
+        let key = Value::String(paths[0].into());
         if obj == &Value::Undefined {
             *obj = Value::new_object();
         }
@@ -2169,6 +2193,8 @@ impl Interpreter {
             span, refr, value, ..
         } = rule.as_ref()
         {
+            let scopes = std::mem::take(&mut self.scopes);
+
             let mut path =
                 Parser::get_path_ref_components(&self.module.clone().unwrap().package.refr)?;
 
@@ -2212,6 +2238,7 @@ impl Interpreter {
                 }
             };
 
+            self.scopes = scopes;
             self.processed.insert(rule.clone());
         }
 
@@ -2289,7 +2316,7 @@ impl Interpreter {
                             Value::Set(_) if special_set => {
                                 let entry = path[path.len() - 1].text();
                                 let mut s = BTreeSet::new();
-                                s.insert(Value::String(entry.to_owned().to_string()));
+                                s.insert(Value::String(entry.to_string().into()));
                                 path = path[0..path.len() - 1].to_vec();
                                 Value::from_set(s)
                             }
diff --git a/src/value.rs b/src/value.rs
index de2d7a7..5ddc80e 100644
--- a/src/value.rs
+++ b/src/value.rs
@@ -90,7 +90,7 @@ pub enum Value {
     Null,
     Bool(bool),
     Number(Number),
-    String(String),
+    String(Rc<str>),
     Array(Rc<Vec<Value>>),
     Object(Rc<BTreeMap<Value, Value>>),
 
@@ -110,7 +110,7 @@ impl Serialize for Value {
         match self {
             Value::Null => serializer.serialize_none(),
             Value::Bool(b) => serializer.serialize_bool(*b),
-            Value::String(s) => serializer.serialize_str(s.as_str()),
+            Value::String(s) => serializer.serialize_str(s.as_ref()),
             Value::Number(n) => n.serialize(serializer),
             Value::Array(a) => a.serialize(serializer),
             Value::Object(fields) => {
@@ -233,14 +233,14 @@ impl Value {
         }
     }
 
-    pub fn as_string(&self) -> Result<&String> {
+    pub fn as_string(&self) -> Result<&Rc<str>> {
         match self {
             Value::String(s) => Ok(s),
             _ => Err(anyhow!("not a string")),
         }
     }
 
-    pub fn as_string_mut(&mut self) -> Result<&mut String> {
+    pub fn as_string_mut(&mut self) -> Result<&mut Rc<str>> {
         match self {
             Value::String(s) => Ok(s),
             _ => Err(anyhow!("not a string")),
@@ -310,7 +310,7 @@ impl Value {
             return Ok(self);
         }
 
-        let key = Value::String(paths[0].to_owned());
+        let key = Value::String(paths[0].into());
         if self == &Value::Undefined {
             *self = Value::new_object();
         }
@@ -379,7 +379,7 @@ impl ops::Index<&str> for Value {
     type Output = Value;
 
     fn index(&self, key: &str) -> &Self::Output {
-        &self[&Value::String(key.to_owned())]
+        &self[&Value::String(key.into())]
     }
 }
 
@@ -387,7 +387,7 @@ impl ops::Index<&String> for Value {
     type Output = Value;
 
     fn index(&self, key: &String) -> &Self::Output {
-        &self[&Value::String(key.clone())]
+        &self[&Value::String(key.clone().into())]
     }
 }
 
diff --git a/tests/interpreter/mod.rs b/tests/interpreter/mod.rs
index c5af5ab..bf10838 100644
--- a/tests/interpreter/mod.rs
+++ b/tests/interpreter/mod.rs
@@ -12,7 +12,7 @@ use test_generator::test_resources;
 pub fn process_value(v: &Value) -> Result<Value> {
     match v {
         // Handle Undefined encoded as a string "#undefined"
-        Value::String(s) if s == "#undefined" => Ok(Value::Undefined),
+        Value::String(s) if s.as_ref() == "#undefined" => Ok(Value::Undefined),
 
         // Handle set encoded as an object
         // set! :
diff --git a/tests/parser/mod.rs b/tests/parser/mod.rs
index b03f765..317b3b7 100644
--- a/tests/parser/mod.rs
+++ b/tests/parser/mod.rs
@@ -20,7 +20,7 @@ macro_rules! my_assert_eq {
 }
 
 fn skip_value(v: &Value) -> bool {
-    matches!(v, Value::String(s) if s == "--skip--")
+    matches!(v, Value::String(s) if s.as_ref() == "--skip--")
 }
 
 fn match_span(s: &Span, v: &Value) -> Result<()> {
@@ -28,7 +28,7 @@ fn match_span(s: &Span, v: &Value) -> Result<()> {
         Value::String(vs) => {
             my_assert_eq!(
                 *s.text(),
-                vs,
+                vs.as_ref(),
                 "{}",
                 s.source
                     .message(s.line, s.col, "match-error", "mismatch happened here.")
@@ -151,7 +151,7 @@ fn match_expr_impl(e: &Expr, v: &Value) -> Result<()> {
         Expr::UnaryExpr { span, expr } => {
             match_span_opt(span, &v["span"])?;
             my_assert_eq!(
-                &Value::String("-".to_owned()),
+                &Value::String("-".into()),
                 &v["op"],
                 "{}",
                 span.source.message(
@@ -324,8 +324,8 @@ fn match_expr_opt(s: &Span, e: &Option<Ref<Expr>>, v: &Value) -> Result<()> {
 
 fn match_bin_op(s: &Span, op: &BinOp, v: &Value) -> Result<()> {
     match (op, v) {
-        (BinOp::And, Value::String(s)) if s == "&" => Ok(()),
-        (BinOp::Or, Value::String(s)) if s == "|" => Ok(()),
+        (BinOp::And, Value::String(s)) if s.as_ref() == "&" => Ok(()),
+        (BinOp::Or, Value::String(s)) if s.as_ref() == "|" => Ok(()),
         _ => bail!(
             "{}",
             s.source.message(
@@ -340,10 +340,10 @@ fn match_bin_op(s: &Span, op: &BinOp, v: &Value) -> Result<()> {
 
 fn match_arith_op(s: &Span, op: &ArithOp, v: &Value) -> Result<()> {
     match (op, v) {
-        (ArithOp::Add, Value::String(s)) if s == "+" => Ok(()),
-        (ArithOp::Sub, Value::String(s)) if s == "-" => Ok(()),
-        (ArithOp::Mul, Value::String(s)) if s == "*" => Ok(()),
-        (ArithOp::Div, Value::String(s)) if s == "/" => Ok(()),
+        (ArithOp::Add, Value::String(s)) if s.as_ref() == "+" => Ok(()),
+        (ArithOp::Sub, Value::String(s)) if s.as_ref() == "-" => Ok(()),
+        (ArithOp::Mul, Value::String(s)) if s.as_ref() == "*" => Ok(()),
+        (ArithOp::Div, Value::String(s)) if s.as_ref() == "/" => Ok(()),
         _ => bail!(
             "{}",
             s.source.message(
@@ -358,11 +358,11 @@ fn match_arith_op(s: &Span, op: &ArithOp, v: &Value) -> Result<()> {
 
 fn match_bool_op(s: &Span, op: &BoolOp, v: &Value) -> Result<()> {
     match (op, v) {
-        (BoolOp::Lt, Value::String(s)) if s == "<" => Ok(()),
-        (BoolOp::Le, Value::String(s)) if s == "<=" => Ok(()),
-        (BoolOp::Eq, Value::String(s)) if s == "==" => Ok(()),
-        (BoolOp::Ge, Value::String(s)) if s == ">=" => Ok(()),
-        (BoolOp::Gt, Value::String(s)) if s == ">" => Ok(()),
+        (BoolOp::Lt, Value::String(s)) if s.as_ref() == "<" => Ok(()),
+        (BoolOp::Le, Value::String(s)) if s.as_ref() == "<=" => Ok(()),
+        (BoolOp::Eq, Value::String(s)) if s.as_ref() == "==" => Ok(()),
+        (BoolOp::Ge, Value::String(s)) if s.as_ref() == ">=" => Ok(()),
+        (BoolOp::Gt, Value::String(s)) if s.as_ref() == ">" => Ok(()),
         _ => bail!(
             "{}",
             s.source.message(
@@ -377,8 +377,8 @@ fn match_bool_op(s: &Span, op: &BoolOp, v: &Value) -> Result<()> {
 
 fn match_assign_op(s: &Span, op: &AssignOp, v: &Value) -> Result<()> {
     match (op, v) {
-        (AssignOp::Eq, Value::String(s)) if s == "=" => Ok(()),
-        (AssignOp::ColEq, Value::String(s)) if s == ":=" => Ok(()),
+        (AssignOp::Eq, Value::String(s)) if s.as_ref() == "=" => Ok(()),
+        (AssignOp::ColEq, Value::String(s)) if s.as_ref() == ":=" => Ok(()),
         _ => bail!(
             "{}",
             s.source.message(
@@ -470,7 +470,7 @@ fn match_literal(l: &Literal, v: &Value) -> Result<()> {
         Literal::NotExpr { expr, span } => {
             let v = &v["notexpr"];
             match &v["op"] {
-                Value::String(s) if s == "not" => (),
+                Value::String(s) if s.as_ref() == "not" => (),
                 _ => {
                     bail!(
                         "{}",
diff --git a/tests/value/mod.rs b/tests/value/mod.rs
index e195a98..b59117c 100644
--- a/tests/value/mod.rs
+++ b/tests/value/mod.rs
@@ -86,7 +86,7 @@ fn display_number() {
 #[test]
 fn serialize_string() -> Result<()> {
     assert_eq!(
-        Value::String("Hello, World\n".to_owned()).to_json_str()?,
+        Value::String("Hello, World\n".into()).to_json_str()?,
         "\"Hello, World\\n\""
     );
     Ok(())
@@ -122,7 +122,7 @@ fn value_as_index() -> Result<()> {
     assert_eq!(&Value::Undefined[&idx], &Value::Undefined);
     assert_eq!(&Value::Null[&idx], &Value::Undefined);
     assert_eq!(&Value::Bool(true)[&idx], &Value::Undefined);
-    assert_eq!(&Value::String("Hello".to_owned())[&idx], &Value::Undefined);
+    assert_eq!(&Value::String("Hello".into())[&idx], &Value::Undefined);
     assert_eq!(&Value::new_set()[&idx], &Value::Undefined);
 
     Ok(())
@@ -151,7 +151,7 @@ fn api() -> Result<()> {
     assert!(&Value::from_json_str("{}")?.as_object()?.is_empty());
     let mut v = Value::new_object();
     v.as_object_mut()?
-        .insert(Value::String("a".to_owned()), Value::from_float(3.145));
+        .insert(Value::String("a".into()), Value::from_float(3.145));
     assert_eq!(v["a"], Value::from_float(3.145));
     assert_eq!(v.as_object()?.len(), 1);
 
@@ -168,8 +168,8 @@ fn api() -> Result<()> {
     assert!(Value::Null.as_set().is_err());
     assert!(Value::Null.as_set_mut().is_err());
 
-    assert!(Value::String("anc".to_owned()).as_array().is_err());
-    assert!(Value::String("anc".to_owned()).as_array_mut().is_err());
+    assert!(Value::String("anc".into()).as_array().is_err());
+    assert!(Value::String("anc".into()).as_array_mut().is_err());
 
     assert!(Value::new_object().as_number().is_err());
     assert!(Value::new_object().as_number_mut().is_err());