Skip to content

Commit

Permalink
[red-knot] fix re-hashing in Files and SymbolTable (#11327)
Browse files Browse the repository at this point in the history
  • Loading branch information
carljm authored May 8, 2024
1 parent 22639c5 commit caf0147
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 10 deletions.
30 changes: 26 additions & 4 deletions crates/red_knot/src/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ impl FilesInner {
/// Inserts the path and returns a new id for it or returns the id if it is an existing path.
// TODO should this accept Path or PathBuf?
pub(crate) fn intern(&mut self, path: &Path) -> FileId {
let mut hasher = FxHasher::default();
path.hash(&mut hasher);
let hash = hasher.finish();
let hash = FilesInner::hash_path(path);

let entry = self
.by_path
Expand All @@ -94,12 +92,20 @@ impl FilesInner {
RawEntryMut::Occupied(entry) => *entry.key(),
RawEntryMut::Vacant(entry) => {
let id = self.by_id.push(Arc::from(path));
entry.insert_with_hasher(hash, id, (), |_| hash);
entry.insert_with_hasher(hash, id, (), |file| {
FilesInner::hash_path(&self.by_id[*file])
});
id
}
}
}

fn hash_path(path: &Path) -> u64 {
let mut hasher = FxHasher::default();
path.hash(&mut hasher);
hasher.finish()
}

pub(crate) fn try_get(&self, path: &Path) -> Option<FileId> {
let mut hasher = FxHasher::default();
path.hash(&mut hasher);
Expand Down Expand Up @@ -155,4 +161,20 @@ mod tests {
let id2 = files.intern(&path2);
assert_ne!(id1, id2);
}

#[test]
fn four_files() {
let files = Files::default();
let foo_path = PathBuf::from("foo");
let foo_id = files.intern(&foo_path);
let bar_path = PathBuf::from("bar");
files.intern(&bar_path);
let baz_path = PathBuf::from("baz");
files.intern(&baz_path);
let qux_path = PathBuf::from("qux");
files.intern(&qux_path);

let foo_id_2 = files.try_get(&foo_path).expect("foo_path to be found");
assert_eq!(foo_id_2, foo_id);
}
}
32 changes: 26 additions & 6 deletions crates/red_knot/src/symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,13 @@ impl SymbolTable {
let scope = &self.scopes_by_id[scope_id];
let hash = SymbolTable::hash_name(name);
let name = Name::new(name);
scope
.symbols_by_name
.raw_entry()
.from_hash(hash, |symid| self.symbols_by_id[*symid].name == name)
.map(|(symbol_id, ())| *symbol_id)
Some(
*scope
.symbols_by_name
.raw_entry()
.from_hash(hash, |symid| self.symbols_by_id[*symid].name == name)?
.0,
)
}

pub(crate) fn symbol_by_name(&self, scope_id: ScopeId, name: &str) -> Option<&Symbol> {
Expand Down Expand Up @@ -344,7 +346,9 @@ impl SymbolTable {
flags,
scope_id,
});
entry.insert_with_hasher(hash, id, (), |_| hash);
entry.insert_with_hasher(hash, id, (), |symid| {
SymbolTable::hash_name(&self.symbols_by_id[*symid].name)
});
id
}
}
Expand Down Expand Up @@ -953,4 +957,20 @@ mod tests {
let symbol = foo_symbol_id.symbol(&table);
assert_eq!(symbol.name.as_str(), "foo");
}

#[test]
fn bigger_symbol_table() {
let mut table = SymbolTable::new();
let root_scope_id = SymbolTable::root_scope_id();
let foo_symbol_id = table.add_or_update_symbol(root_scope_id, "foo", SymbolFlags::empty());
table.add_or_update_symbol(root_scope_id, "bar", SymbolFlags::empty());
table.add_or_update_symbol(root_scope_id, "baz", SymbolFlags::empty());
table.add_or_update_symbol(root_scope_id, "qux", SymbolFlags::empty());

let foo_symbol_id_2 = table
.root_symbol_id_by_name("foo")
.expect("foo symbol to be found");

assert_eq!(foo_symbol_id_2, foo_symbol_id);
}
}

0 comments on commit caf0147

Please sign in to comment.