use PartialEq and Hash instead of a freestanding function

Author: pascalkuthe

helix-core/src/syntax.rs

@@ -18,7 +18,7 @@ use std::{
     cell::RefCell,
     collections::{HashMap, VecDeque},
     fmt,
-    hash::{BuildHasher, Hash, Hasher},
+    hash::{Hash, Hasher},
     mem::{replace, transmute},
     path::Path,
     str::FromStr,
@@ -711,8 +711,15 @@ thread_local! {
 
 pub struct Syntax {
     layers: HopSlotMap<LayerId, LanguageLayer>,
-    layers_lut: RawTable<LayerId>,
-    layers_lut_hasher: RandomState,
+    /// This table allows inverse indexing of `layers`.
+    /// That is by hashing a `Layer` you can find
+    /// the `LayerId` of an existing equivalent `Layer` in `layers`.
+    ///
+    /// This hashtable is populated inside the `update` function
+    /// and used to determine if a new layer exists for an injection
+    /// or if an existing layer needs to be updated.
+    layers_tabel: RawTable<LayerId>,
+    layers_hasher: RandomState,
     root: LayerId,
     loader: Arc<Loader>,
 }
@@ -756,8 +763,8 @@ impl Syntax {
             root,
             layers,
             loader,
-            layers_lut: RawTable::new(),
-            layers_lut_hasher: RandomState::new(),
+            layers_tabel: RawTable::new(),
+            layers_hasher: RandomState::new(),
         };
 
         syntax
@@ -803,11 +810,10 @@ impl Syntax {
                 }
             }
 
-            // Ensure lut is large enough to hold all layers.
-            // The lut should always be empty at this point because it is only
-            // kept to avoid realloctions so rehashing is never requied (hence unreachable).
-            assert!(self.layers_lut.is_empty());
-            self.layers_lut
+            self.layers_tabel.clear_no_drop();
+            // Ensure table is large enough to hold all layers.
+            // Because the table is cleared first, rehashing is never requied (hence unreachable).
+            self.layers_tabel
                 .reserve(self.layers.len(), |_| unreachable!());
 
             for (layer_id, layer) in self.layers.iter_mut() {
@@ -881,17 +887,12 @@ impl Syntax {
                     }
                 }
 
-                let hash = hash_injection_layer(
-                    &self.layers_lut_hasher,
-                    layer.depth,
-                    &layer.config,
-                    &layer.ranges,
-                );
+                let hash = self.layers_hasher.hash_one(layer);
                 // Safety: insert_no_grow is unsafe because it assumes that the table
                 // has enough capacity to hold additional elements.
                 // This is always the case as we reserved enough capacity above.
                 unsafe {
-                    self.layers_lut.insert_no_grow(hash, layer_id);
+                    self.layers_tabel.insert_no_grow(hash, layer_id);
                 }
             }
         }
@@ -1018,31 +1019,24 @@ impl Syntax {
                 let depth = layer.depth + 1;
                 // TODO: can't inline this since matches borrows self.layers
                 for (config, ranges) in injections {
-                    // Find an existing layer
+                    let new_layer = LanguageLayer {
+                        tree: None,
+                        config,
+                        depth,
+                        ranges,
+                        flags: LayerUpdateFlags::empty(),
+                    };
 
-                    let hash =
-                        hash_injection_layer(&self.layers_lut_hasher, depth, &config, &ranges);
+                    // Find an identical existing layer
                     let layer = self
-                        .layers_lut
-                        .get(hash, |&it| {
-                            let layer = &self.layers[it];
-                            layer.depth == depth && // TODO: track parent id instead
-                            layer.config.language == config.language &&
-                            layer.ranges == ranges
+                        .layers_tabel
+                        .get(self.layers_hasher.hash_one(&new_layer), |&it| {
+                            self.layers[it] == new_layer
                         })
                         .copied();
 
                     // ...or insert a new one.
-                    let layer_id = layer.unwrap_or_else(|| {
-                        self.layers.insert(LanguageLayer {
-                            tree: None,
-                            config,
-                            depth,
-                            ranges,
-                            // set the modified flag to ensure the layer is parsed
-                            flags: LayerUpdateFlags::empty(),
-                        })
-                    });
+                    let layer_id = layer.unwrap_or_else(|| self.layers.insert(new_layer));
 
                     queue.push_back(layer_id);
                 }
@@ -1060,8 +1054,6 @@ impl Syntax {
                     .contains(LayerUpdateFlags::TOUCHED)
             });
 
-            self.layers_lut.clear_no_drop();
-
             Ok(())
         })
     }
@@ -1181,23 +1173,32 @@ pub struct LanguageLayer {
     flags: LayerUpdateFlags,
 }
 
-fn hash_injection_layer(
-    state: &RandomState,
-    depth: u32,
-    config: &HighlightConfiguration,
-    ranges: &[Range],
-) -> u64 {
-    let mut state = state.build_hasher();
-    depth.hash(&mut state);
-    // The transmute is necessary here because tree_sitter::Language does not derive Hash at the moment.
-    // However it does use #[repr] transparent so the transmute here is safe
-    // as `Language` (which `Grammar` is an alias for) is just a newtype wrapper around a (thin) pointer.
-    // This is also compatible with the PartialEq implementation of language
-    // as that is just a pointer comparison.
-    let language: *const () = unsafe { transmute(config.language) };
-    language.hash(&mut state);
-    ranges.hash(&mut state);
-    state.finish()
+/// This PartialEq implementation only checks if that
+/// two layers are theoretically identical (meaning they highlight the same text range with the same language).
+/// It does not check whether the layers have the same internal treesitter
+/// state.
+impl PartialEq for LanguageLayer {
+    fn eq(&self, other: &Self) -> bool {
+        self.depth == other.depth
+            && self.config.language == other.config.language
+            && self.ranges == other.ranges
+    }
+}
+
+/// Hash implementation belongs to PartialEq implementation above.
+/// See its documentation for details.
+impl Hash for LanguageLayer {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.depth.hash(state);
+        // The transmute is necessary here because tree_sitter::Language does not derive Hash at the moment.
+        // However it does use #[repr] transparent so the transmute here is safe
+        // as `Language` (which `Grammar` is an alias for) is just a newtype wrapper around a (thin) pointer.
+        // This is also compatible with the PartialEq implementation of language
+        // as that is just a pointer comparison.
+        let language: *const () = unsafe { transmute(self.config.language) };
+        language.hash(state);
+        self.ranges.hash(state);
+    }
 }
 
 impl LanguageLayer {