Pass owned paths to id_from_canonical

id_from_canonical ideally takes owned strings
instead of references to avoid a copy.

4 files changed, 14 insertions, 28 deletions

diff --git a/benches/parse.rs b/benches/parse.rs
index 71abf60..bdd7323 100644
--- a/benches/parse.rs
+++ b/benches/parse.rs
@@ -1,5 +1,4 @@
 use criterion::{criterion_group, criterion_main, Criterion};
-use n2::parse::Loader;
 use std::{io::Write, path::PathBuf, str::FromStr};
 
 pub fn bench_canon(c: &mut Criterion) {
diff --git a/src/eval.rs b/src/eval.rs
index c5f511b..5adf6ce 100644
--- a/src/eval.rs
+++ b/src/eval.rs
@@ -3,7 +3,8 @@
 
 use crate::smallmap::SmallMap;
 use std::borrow::Borrow;
-use std::{borrow::Cow, collections::HashMap};
+use std::borrow::Cow;
+use std::collections::HashMap;
 
 /// An environment providing a mapping of variable name to variable value.
 /// This represents one "frame" of evaluation context, a given EvalString may
diff --git a/src/load.rs b/src/load.rs
index 8297b4b..edcf49d 100644
--- a/src/load.rs
+++ b/src/load.rs
@@ -56,17 +56,6 @@ pub struct Loader {
     builddir: Option<String>,
 }
 
-impl parse::Loader for Loader {
-    type Path = FileId;
-    fn path(&mut self, path: &mut str) -> Self::Path {
-        // Perf: this is called while parsing build.ninja files.  We go to
-        // some effort to avoid allocating in the common case of a path that
-        // refers to a file that is already known.
-        let len = canon_path_fast(path);
-        self.graph.files.id_from_canonical(&path[..len])
-    }
-}
-
 impl Loader {
     pub fn new() -> Self {
         let mut loader = Loader::default();
@@ -76,10 +65,19 @@ impl Loader {
         loader
     }
 
+    /// Convert a path string to a FileId.  For performance reasons
+    /// this requires an owned 'path' param.
+    fn path(&mut self, mut path: String) -> FileId {
+        // Perf: this is called while parsing build.ninja files.  We go to
+        // some effort to avoid allocating in the common case of a path that
+        // refers to a file that is already known.
+        let len = canon_path_fast(&mut path);
+        path.truncate(len);
+        self.graph.files.id_from_canonical(path)
+    }
+
     fn evaluate_path(&mut self, path: EvalString<&str>, envs: &[&dyn eval::Env]) -> FileId {
-        use parse::Loader;
-        let mut evaluated = path.evaluate(envs);
-        self.path(&mut evaluated)
+        self.path(path.evaluate(envs))
     }
 
     fn evaluate_paths(
diff --git a/src/parse.rs b/src/parse.rs
index a8ee60f..04a8397 100644
--- a/src/parse.rs
+++ b/src/parse.rs
@@ -57,18 +57,6 @@ pub struct Parser<'text> {
     eval_buf: Vec<EvalPart<&'text str>>,
 }
 
-/// Loader maps path strings (as found in build.ninja files) into an arbitrary
-/// "Path" type.  This allows us to canonicalize and convert path strings to
-/// more efficient integer identifiers while we parse, rather than needing to
-/// buffer up many intermediate strings; in fact, parsing uses a single buffer
-/// for all of these.
-pub trait Loader {
-    type Path;
-    /// Convert a path string to a Self::Path type.  For performance reasons
-    /// this may mutate the 'path' param.
-    fn path(&mut self, path: &mut str) -> Self::Path;
-}
-
 impl<'text> Parser<'text> {
     pub fn new(buf: &'text [u8]) -> Parser<'text> {
         Parser {