From d5ed27cce63b1f724e39665dc8294487a009c6b2 Mon Sep 17 00:00:00 2001
From: Jakob Stoklund Olesen <stoklund@2pi.dk>
Date: Fri, 16 Sep 2016 09:36:34 -0700
Subject: [PATCH] Use Cow<str> for the values of filecheck variables.

Often, an implementation of VariableMap can return references to
internal strings, and Cow::Borrow() allows that without making any
copies.

We still want to allow VariableMap implementations to return owned
strings in case they have to manufacture variable values on demand. The
Cow::Owned() variant does exactly that.

Switch the internal VariableMap implementations over to Cows. It turns
out they can simply store references to substrings of the input test,
completely avoiding string allocation for script-defined variables.
---
 cranelift/src/libfilecheck/checker.rs  | 11 ++++++-----
 cranelift/src/libfilecheck/pattern.rs  | 21 ++++++++-------------
 cranelift/src/libfilecheck/variable.rs |  8 +++++---
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/cranelift/src/libfilecheck/checker.rs b/cranelift/src/libfilecheck/checker.rs
index 6bb458b55e..a2af0815e7 100644
--- a/cranelift/src/libfilecheck/checker.rs
+++ b/cranelift/src/libfilecheck/checker.rs
@@ -2,6 +2,7 @@ use error::{Error, Result};
 use variable::{VariableMap, Value, varname_prefix};
 use pattern::Pattern;
 use regex::{Regex, Captures};
+use std::borrow::Cow;
 use std::collections::HashMap;
 use std::cmp::max;
 use std::fmt::{self, Display, Formatter};
@@ -183,7 +184,7 @@ impl Checker {
                 Directive::Regex(ref var, ref rx) => {
                     state.vars.insert(var.clone(),
                                       VarDef {
-                                          value: Value::Regex(rx.clone()),
+                                          value: Value::Regex(Cow::Borrowed(rx)),
                                           offset: 0,
                                       });
                     continue;
@@ -234,9 +235,9 @@ impl Checker {
 }
 
 /// A local definition of a variable.
-pub struct VarDef {
+pub struct VarDef<'a> {
     /// The value given to the variable.
-    value: Value,
+    value: Value<'a>,
     /// Offset in input text from where the variable is available.
     offset: usize,
 }
@@ -246,7 +247,7 @@ struct State<'a> {
     env_vars: &'a VariableMap,
     recorder: &'a mut Recorder,
 
-    vars: HashMap<String, VarDef>,
+    vars: HashMap<String, VarDef<'a>>,
     // Offset after the last ordered match. This does not include recent unordered matches.
     last_ordered: usize,
     // Largest offset following a positive match, including unordered matches.
@@ -331,7 +332,7 @@ impl<'a> State<'a> {
                     let txtval = caps.name(var).unwrap_or("");
                     self.recorder.defined_var(var, txtval);
                     let vardef = VarDef {
-                        value: Value::Text(txtval.to_string()),
+                        value: Value::Text(Cow::Borrowed(txtval)),
                         // This offset is the end of the whole matched pattern, not just the text
                         // defining the variable.
                         offset: range.0 + matched_range.1,
diff --git a/cranelift/src/libfilecheck/pattern.rs b/cranelift/src/libfilecheck/pattern.rs
index 69464bd7c8..88c47011f9 100644
--- a/cranelift/src/libfilecheck/pattern.rs
+++ b/cranelift/src/libfilecheck/pattern.rs
@@ -330,19 +330,14 @@ impl Pattern {
                 Part::DefLit { ref regex, .. } => out.push_str(regex),
                 Part::DefVar { def, ref var } => {
                     // Wrap regex in a named capture group.
-                    write!(out,
-                           "(?P<{}>{})",
-                           self.defs[def],
-                           match vmap.lookup(var) {
-                               None => {
-                                   return Err(Error::UndefVariable(format!("undefined variable \
-                                                                            ${}",
-                                                                           var)))
-                               }
-                               Some(Value::Text(s)) => quote(&s),
-                               Some(Value::Regex(rx)) => rx,
-                           })
-                        .unwrap()
+                    write!(out, "(?P<{}>", self.defs[def]).unwrap();
+                    match vmap.lookup(var) {
+                        None => {
+                            return Err(Error::UndefVariable(format!("undefined variable ${}", var)))
+                        }
+                        Some(Value::Text(s)) => write!(out, "{})", quote(&s[..])).unwrap(),
+                        Some(Value::Regex(rx)) => write!(out, "{})", rx).unwrap(),
+                    }
                 }
             }
 
diff --git a/cranelift/src/libfilecheck/variable.rs b/cranelift/src/libfilecheck/variable.rs
index 7238249146..51bc38fc07 100644
--- a/cranelift/src/libfilecheck/variable.rs
+++ b/cranelift/src/libfilecheck/variable.rs
@@ -1,3 +1,5 @@
+use std::borrow::Cow;
+
 /// A variable name is one or more ASCII alphanumerical characters, including underscore.
 /// Note that numerical variable names like `$45` are allowed too.
 ///
@@ -16,9 +18,9 @@ pub fn varname_prefix(s: &str) -> usize {
 
 /// A variable can contain either a regular expression or plain text.
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub enum Value {
-    Text(String),
-    Regex(String),
+pub enum Value<'a> {
+    Text(Cow<'a, str>),
+    Regex(Cow<'a, str>),
 }
 
 /// Resolve variables by name.