repo_path: split RepoPathComponent into owned and borrowed types

This is a step towards introducing a borrowed RepoPath type. The current
RepoPath type is inefficient as each component String is usually short. We
could apply short-string optimization, but still each inlined component would
consume 24 bytes just for e.g. "src", and increase the chance of random memory
access. If the owned RepoPath type is backed by String, we can implement cheap
cast from &str to borrowed &RepoPath type.

lib/src/backend.rs

@@ -384,7 +384,7 @@ impl Tree {
     }
 
     pub fn names(&self) -> impl Iterator<Item = &RepoPathComponent> {
-        self.entries.keys()
+        self.entries.keys().map(|name| name.as_ref())
     }
 
     pub fn entries(&self) -> TreeEntriesNonRecursiveIterator {
@@ -407,7 +407,7 @@ impl Tree {
                 self.entries.remove(name);
             }
             Some(value) => {
-                self.entries.insert(name.clone(), value);
+                self.entries.insert(name.to_owned(), value);
             }
         }
     }

lib/src/local_working_copy.rs

@@ -783,7 +783,7 @@ impl TreeState {
                 if name == ".jj" || name == ".git" {
                     return Ok(());
                 }
-                let path = dir.join(&RepoPathComponent::from(name));
+                let path = dir.join(RepoPathComponent::new(name));
                 if let Some(file_state) = file_states.get(&path) {
                     if file_state.file_type == FileType::GitSubmodule {
                         return Ok(());

lib/src/repo_path.rs

@@ -14,25 +14,46 @@
 
 #![allow(missing_docs)]
 
+use std::borrow::Borrow;
 use std::fmt::{Debug, Error, Formatter};
+use std::ops::Deref;
 use std::path::{Component, Path, PathBuf};
 
 use itertools::Itertools;
+use ref_cast::{ref_cast_custom, RefCastCustom};
 use thiserror::Error;
 
 use crate::file_util;
 
-// TODO: make RepoPathComponent a borrowed type
-pub type RepoPathComponentBuf = RepoPathComponent;
-
 content_hash! {
+    /// Owned `RepoPath` component.
     #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
-    pub struct RepoPathComponent {
+    pub struct RepoPathComponentBuf {
+        // Don't add more fields. Eq, Hash, and Ord must be compatible with the
+        // borrowed RepoPathComponent type.
         value: String,
     }
 }
 
+/// Borrowed `RepoPath` component.
+#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Hash, RefCastCustom)]
+#[repr(transparent)]
+pub struct RepoPathComponent {
+    value: str,
+}
+
 impl RepoPathComponent {
+    /// Wraps `value` as `RepoPathComponent`.
+    ///
+    /// The input `value` must not be empty and not contain path separator.
+    pub fn new(value: &str) -> &Self {
+        assert!(is_valid_repo_path_component_str(value));
+        Self::new_unchecked(value)
+    }
+
+    #[ref_cast_custom]
+    const fn new_unchecked(value: &str) -> &Self;
+
     pub fn as_str(&self) -> &str {
         &self.value
     }
@@ -51,6 +72,45 @@ impl From<String> for RepoPathComponentBuf {
     }
 }
 
+impl AsRef<RepoPathComponent> for RepoPathComponent {
+    fn as_ref(&self) -> &RepoPathComponent {
+        self
+    }
+}
+
+impl AsRef<RepoPathComponent> for RepoPathComponentBuf {
+    fn as_ref(&self) -> &RepoPathComponent {
+        self
+    }
+}
+
+impl Borrow<RepoPathComponent> for RepoPathComponentBuf {
+    fn borrow(&self) -> &RepoPathComponent {
+        self
+    }
+}
+
+impl Deref for RepoPathComponentBuf {
+    type Target = RepoPathComponent;
+
+    fn deref(&self) -> &Self::Target {
+        RepoPathComponent::new_unchecked(&self.value)
+    }
+}
+
+impl ToOwned for RepoPathComponent {
+    type Owned = RepoPathComponentBuf;
+
+    fn to_owned(&self) -> Self::Owned {
+        let value = self.value.to_owned();
+        RepoPathComponentBuf { value }
+    }
+
+    fn clone_into(&self, target: &mut Self::Owned) {
+        self.value.clone_into(&mut target.value);
+    }
+}
+
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct RepoPath {
     components: Vec<RepoPathComponentBuf>,
@@ -189,7 +249,8 @@ impl RepoPath {
     }
 
     pub fn join(&self, entry: &RepoPathComponent) -> RepoPath {
-        let components = self.components.iter().chain([entry]).cloned().collect();
+        let components =
+            itertools::chain(self.components.iter().cloned(), [entry.to_owned()]).collect();
         RepoPath { components }
     }
 }
@@ -259,12 +320,12 @@ mod tests {
     #[test]
     fn test_join() {
         let root = RepoPath::root();
-        let dir = root.join(&RepoPathComponent::from("dir"));
+        let dir = root.join(RepoPathComponent::new("dir"));
         assert_eq!(dir, repo_path("dir"));
-        let subdir = dir.join(&RepoPathComponent::from("subdir"));
+        let subdir = dir.join(RepoPathComponent::new("subdir"));
         assert_eq!(subdir, repo_path("dir/subdir"));
         assert_eq!(
-            subdir.join(&RepoPathComponent::from("file")),
+            subdir.join(RepoPathComponent::new("file")),
             repo_path("dir/subdir/file")
         );
     }
@@ -272,8 +333,8 @@ mod tests {
     #[test]
     fn test_parent() {
         let root = RepoPath::root();
-        let dir_component = &RepoPathComponent::from("dir");
+        let dir_component = RepoPathComponent::new("dir");
-        let subdir_component = &RepoPathComponent::from("subdir");
+        let subdir_component = RepoPathComponent::new("subdir");
 
         let dir = root.join(dir_component);
         let subdir = dir.join(subdir_component);
@@ -286,8 +347,8 @@ mod tests {
     #[test]
     fn test_split() {
         let root = RepoPath::root();
-        let dir_component = &RepoPathComponent::from("dir");
+        let dir_component = RepoPathComponent::new("dir");
-        let file_component = &RepoPathComponent::from("file");
+        let file_component = RepoPathComponent::new("file");
 
         let dir = root.join(dir_component);
         let file = dir.join(file_component);

lib/src/tree_builder.rs

@@ -82,7 +82,7 @@ impl TreeBuilder {
             let tree = trees_to_write.get_mut(&dir).unwrap();
             match file_override {
                 Override::Replace(value) => {
-                    tree.set(basename.clone(), value);
+                    tree.set(basename.to_owned(), value);
                 }
                 Override::Tombstone => {
                     tree.remove(basename);
@@ -104,7 +104,7 @@ impl TreeBuilder {
                     }
                 } else {
                     let tree = store.write_tree(&dir, tree).unwrap();
-                    parent_tree.set(basename.clone(), TreeValue::Tree(tree.id().clone()));
+                    parent_tree.set(basename.to_owned(), TreeValue::Tree(tree.id().clone()));
                 }
             } else {
                 // We're writing the root tree. Write it even if empty. Return its id.

lib/tests/test_local_working_copy.rs

@@ -145,7 +145,7 @@ fn test_checkout_file_transitions(backend: TestRepoBackend) {
                 Merge::normal(TreeValue::Symlink(id))
             }
             Kind::Tree => {
-                let file_path = path.join(&RepoPathComponent::from("file"));
+                let file_path = path.join(RepoPathComponent::new("file"));
                 let id = testutils::write_file(store, &file_path, "normal file contents");
                 let value = TreeValue::File {
                     id,
@@ -348,7 +348,7 @@ fn test_conflicting_changes_on_disk() {
         &[
             (&file_file_path, "committed contents"),
             (
-                &file_dir_path.join(&RepoPathComponent::from("file")),
+                &file_dir_path.join(RepoPathComponent::new("file")),
                 "committed contents",
             ),
             (&dir_file_path, "committed contents"),

lib/tests/test_merge_trees.rs

@@ -83,36 +83,36 @@ fn test_same_type() {
 
     // Check that the simple, non-conflicting cases were resolved correctly
     assert_eq!(
-        merged_tree.value(&RepoPathComponent::from("__a")),
+        merged_tree.value(RepoPathComponent::new("__a")),
-        side2_tree.value(&RepoPathComponent::from("__a"))
+        side2_tree.value(RepoPathComponent::new("__a"))
     );
     assert_eq!(
-        merged_tree.value(&RepoPathComponent::from("_a_")),
+        merged_tree.value(RepoPathComponent::new("_a_")),
-        side1_tree.value(&RepoPathComponent::from("_a_"))
+        side1_tree.value(RepoPathComponent::new("_a_"))
     );
     assert_eq!(
-        merged_tree.value(&RepoPathComponent::from("_aa")),
+        merged_tree.value(RepoPathComponent::new("_aa")),
-        side1_tree.value(&RepoPathComponent::from("_aa"))
+        side1_tree.value(RepoPathComponent::new("_aa"))
     );
     assert_eq!(
-        merged_tree.value(&RepoPathComponent::from("aaa")),
+        merged_tree.value(RepoPathComponent::new("aaa")),
-        side1_tree.value(&RepoPathComponent::from("aaa"))
+        side1_tree.value(RepoPathComponent::new("aaa"))
     );
     assert_eq!(
-        merged_tree.value(&RepoPathComponent::from("aab")),
+        merged_tree.value(RepoPathComponent::new("aab")),
-        side2_tree.value(&RepoPathComponent::from("aab"))
+        side2_tree.value(RepoPathComponent::new("aab"))
     );
     assert_eq!(
-        merged_tree.value(&RepoPathComponent::from("aba")),
+        merged_tree.value(RepoPathComponent::new("aba")),
-        side1_tree.value(&RepoPathComponent::from("aba"))
+        side1_tree.value(RepoPathComponent::new("aba"))
     );
     assert_eq!(
-        merged_tree.value(&RepoPathComponent::from("abb")),
+        merged_tree.value(RepoPathComponent::new("abb")),
-        side1_tree.value(&RepoPathComponent::from("abb"))
+        side1_tree.value(RepoPathComponent::new("abb"))
     );
 
     // Check the conflicting cases
-    let component = &RepoPathComponent::from("_ab");
+    let component = RepoPathComponent::new("_ab");
     match merged_tree.value(component).unwrap() {
         TreeValue::Conflict(id) => {
             let conflict = store
@@ -129,7 +129,7 @@ fn test_same_type() {
         }
         _ => panic!("unexpected value"),
     };
-    let component = &RepoPathComponent::from("a_b");
+    let component = RepoPathComponent::new("a_b");
     match merged_tree.value(component).unwrap() {
         TreeValue::Conflict(id) => {
             let conflict = store
@@ -146,7 +146,7 @@ fn test_same_type() {
         }
         _ => panic!("unexpected value"),
     };
-    let component = &RepoPathComponent::from("ab_");
+    let component = RepoPathComponent::new("ab_");
     match merged_tree.value(component).unwrap() {
         TreeValue::Conflict(id) => {
             let conflict = store
@@ -163,7 +163,7 @@ fn test_same_type() {
         }
         _ => panic!("unexpected value"),
     };
-    let component = &RepoPathComponent::from("abc");
+    let component = RepoPathComponent::new("abc");
     match merged_tree.value(component).unwrap() {
         TreeValue::Conflict(id) => {
             let conflict = store
@@ -221,16 +221,16 @@ fn test_executable() {
     let merged_tree = merge_trees(&side1_tree, &base_tree, &side2_tree).unwrap();
 
     // Check that the merged tree has the correct executable bits
-    let norm = base_tree.value(&RepoPathComponent::from("nnn"));
+    let norm = base_tree.value(RepoPathComponent::new("nnn"));
-    let exec = base_tree.value(&RepoPathComponent::from("xxx"));
+    let exec = base_tree.value(RepoPathComponent::new("xxx"));
-    assert_eq!(merged_tree.value(&RepoPathComponent::from("nnn")), norm);
+    assert_eq!(merged_tree.value(RepoPathComponent::new("nnn")), norm);
-    assert_eq!(merged_tree.value(&RepoPathComponent::from("nnx")), exec);
+    assert_eq!(merged_tree.value(RepoPathComponent::new("nnx")), exec);
-    assert_eq!(merged_tree.value(&RepoPathComponent::from("nxn")), exec);
+    assert_eq!(merged_tree.value(RepoPathComponent::new("nxn")), exec);
-    assert_eq!(merged_tree.value(&RepoPathComponent::from("nxx")), exec);
+    assert_eq!(merged_tree.value(RepoPathComponent::new("nxx")), exec);
-    assert_eq!(merged_tree.value(&RepoPathComponent::from("xnn")), norm);
+    assert_eq!(merged_tree.value(RepoPathComponent::new("xnn")), norm);
-    assert_eq!(merged_tree.value(&RepoPathComponent::from("xnx")), norm);
+    assert_eq!(merged_tree.value(RepoPathComponent::new("xnx")), norm);
-    assert_eq!(merged_tree.value(&RepoPathComponent::from("xxn")), norm);
+    assert_eq!(merged_tree.value(RepoPathComponent::new("xxn")), norm);
-    assert_eq!(merged_tree.value(&RepoPathComponent::from("xxx")), exec);
+    assert_eq!(merged_tree.value(RepoPathComponent::new("xxx")), exec);
 }
 
 #[test]
@@ -411,7 +411,7 @@ fn test_types() {
     let merged_tree = merge_trees(&side1_tree, &base_tree, &side2_tree).unwrap();
 
     // Check the conflicting cases
-    let component = &RepoPathComponent::from("normal_executable_symlink");
+    let component = RepoPathComponent::new("normal_executable_symlink");
     match merged_tree.value(component).unwrap() {
         TreeValue::Conflict(id) => {
             let conflict = store
@@ -431,7 +431,7 @@ fn test_types() {
         }
         _ => panic!("unexpected value"),
     };
-    let component = &RepoPathComponent::from("tree_normal_symlink");
+    let component = RepoPathComponent::new("tree_normal_symlink");
     match merged_tree.value(component).unwrap() {
         TreeValue::Conflict(id) => {
             let conflict = store
@@ -456,7 +456,7 @@ fn test_simplify_conflict() {
     let repo = &test_repo.repo;
     let store = repo.store();
 
-    let component = &RepoPathComponent::from("file");
+    let component = RepoPathComponent::new("file");
     let path = RepoPath::from_internal_string("file");
     let write_tree = |contents: &str| -> Tree { create_single_tree(repo, &[(&path, contents)]) };
 
@@ -495,7 +495,7 @@ fn test_simplify_conflict() {
     match further_rebased_tree.value(component).unwrap() {
         TreeValue::Conflict(id) => {
             let conflict = store
-                .read_conflict(&RepoPath::from_components(vec![component.clone()]), id)
+                .read_conflict(&RepoPath::from_components(vec![component.to_owned()]), id)
                 .unwrap();
             assert_eq!(
                 conflict.removes().map(|v| v.as_ref()).collect_vec(),

lib/tests/test_merged_tree.rs

@@ -123,10 +123,10 @@ fn test_from_legacy_tree() {
     tree_builder.set(file5_path.clone(), TreeValue::Conflict(file5_conflict_id));
 
     // dir1: directory without conflicts
-    let dir1_basename = &RepoPathComponent::from("dir1");
+    let dir1_basename = RepoPathComponent::new("dir1");
     let dir1_filename = RepoPath::root()
         .join(dir1_basename)
-        .join(&RepoPathComponent::from("file"));
+        .join(RepoPathComponent::new("file"));
     let dir1_filename_id = write_file(store.as_ref(), &dir1_filename, "file5_v2");
     tree_builder.set(dir1_filename.clone(), file_value(&dir1_filename_id));
 
@@ -135,7 +135,7 @@ fn test_from_legacy_tree() {
 
     let merged_tree = MergedTree::from_legacy_tree(tree.clone()).unwrap();
     assert_eq!(
-        merged_tree.value(&RepoPathComponent::from("missing")),
+        merged_tree.value(RepoPathComponent::new("missing")),
         MergedTreeVal::Resolved(None)
     );
     // file1: regular file without conflicts
@@ -373,11 +373,11 @@ fn test_resolve_success() {
     let trivial_file_path = RepoPath::from_internal_string("trivial-file");
     let trivial_hunk_path = RepoPath::from_internal_string("trivial-hunk");
     let both_added_dir_path = RepoPath::from_internal_string("added-dir");
-    let both_added_dir_file1_path = both_added_dir_path.join(&RepoPathComponent::from("file1"));
+    let both_added_dir_file1_path = both_added_dir_path.join(RepoPathComponent::new("file1"));
-    let both_added_dir_file2_path = both_added_dir_path.join(&RepoPathComponent::from("file2"));
+    let both_added_dir_file2_path = both_added_dir_path.join(RepoPathComponent::new("file2"));
     let emptied_dir_path = RepoPath::from_internal_string("to-become-empty");
-    let emptied_dir_file1_path = emptied_dir_path.join(&RepoPathComponent::from("file1"));
+    let emptied_dir_file1_path = emptied_dir_path.join(RepoPathComponent::new("file1"));
-    let emptied_dir_file2_path = emptied_dir_path.join(&RepoPathComponent::from("file2"));
+    let emptied_dir_file2_path = emptied_dir_path.join(RepoPathComponent::new("file2"));
     let base1 = create_single_tree(
         repo,
         &[
@@ -503,7 +503,7 @@ fn test_conflict_iterator() {
             (&dir_file_path, "base"),
             // no added_dir_path
             (
-                &modify_delete_dir_path.join(&RepoPathComponent::from("base")),
+                &modify_delete_dir_path.join(RepoPathComponent::new("base")),
                 "base",
             ),
         ],
@@ -520,11 +520,11 @@ fn test_conflict_iterator() {
             (&different_add_path, "side1"),
             (&dir_file_path, "side1"),
             (
-                &added_dir_path.join(&RepoPathComponent::from("side1")),
+                &added_dir_path.join(RepoPathComponent::new("side1")),
                 "side1",
             ),
             (
-                &modify_delete_dir_path.join(&RepoPathComponent::from("side1")),
+                &modify_delete_dir_path.join(RepoPathComponent::new("side1")),
                 "side1",
             ),
         ],
@@ -539,9 +539,9 @@ fn test_conflict_iterator() {
             // no modify_delete_path
             (&same_add_path, "same"),
             (&different_add_path, "side2"),
-            (&dir_file_path.join(&RepoPathComponent::from("dir")), "new"),
+            (&dir_file_path.join(RepoPathComponent::new("dir")), "new"),
             (
-                &added_dir_path.join(&RepoPathComponent::from("side2")),
+                &added_dir_path.join(RepoPathComponent::new("side2")),
                 "side2",
             ),
             // no modify_delete_dir_path
@@ -859,7 +859,7 @@ fn test_diff_dir_file() {
     let path4 = RepoPath::from_internal_string("path4");
     let path5 = RepoPath::from_internal_string("path5");
     let path6 = RepoPath::from_internal_string("path6");
-    let file = &RepoPathComponent::from("file");
+    let file = RepoPathComponent::new("file");
     let left_base = create_single_tree(
         repo,
         &[