From de40124adb3c7bd064e9dbdcb4d82db6843889c3 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 30 Nov 2023 13:49:03 +0100 Subject: [PATCH] Fix content hashing Fixes #2800 --- crates/typst-macros/src/elem.rs | 7 +++++-- crates/typst/src/foundations/content.rs | 9 ++------- crates/typst/src/foundations/element.rs | 8 +++++++- crates/typst/src/foundations/styles.rs | 18 +++++++++++------- crates/typst/src/foundations/value.rs | 14 +++++--------- 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/crates/typst-macros/src/elem.rs b/crates/typst-macros/src/elem.rs index e0306b706..e531a7edb 100644 --- a/crates/typst-macros/src/elem.rs +++ b/crates/typst-macros/src/elem.rs @@ -964,8 +964,11 @@ fn create_native_elem_impl(element: &Elem) -> TokenStream { #foundations::Element::of::() } - fn dyn_hash(&self, mut hasher: &mut dyn ::std::hash::Hasher) { - ::hash(self, &mut hasher); + fn dyn_hash(&self, mut state: &mut dyn ::std::hash::Hasher) { + // Also hash the TypeId since values with different types but + // equal data should be different. + ::std::hash::Hash::hash(&::std::any::TypeId::of::(), &mut state); + ::std::hash::Hash::hash(self, &mut state); } fn dyn_eq(&self, other: &#foundations::Content) -> bool { diff --git a/crates/typst/src/foundations/content.rs b/crates/typst/src/foundations/content.rs index fc9a49e21..d8e7570d4 100644 --- a/crates/typst/src/foundations/content.rs +++ b/crates/typst/src/foundations/content.rs @@ -66,7 +66,8 @@ use crate::util::fat; /// elements the content is composed of and what fields they have. /// Alternatively, you can inspect the output of the [`repr`]($repr) function. #[ty(scope)] -#[derive(Clone)] +#[derive(Clone, Hash)] +#[allow(clippy::derived_hash_with_manual_eq)] pub struct Content(Arc); impl Content { @@ -607,12 +608,6 @@ impl From> for Content { } } -impl std::hash::Hash for Content { - fn hash(&self, state: &mut H) { - self.0.dyn_hash(state); - } -} - impl PartialEq for Content { fn eq(&self, other: &Self) -> bool { // Additional short circuit for different elements. diff --git a/crates/typst/src/foundations/element.rs b/crates/typst/src/foundations/element.rs index 64c92be97..301a4003f 100644 --- a/crates/typst/src/foundations/element.rs +++ b/crates/typst/src/foundations/element.rs @@ -2,7 +2,7 @@ use std::any::{Any, TypeId}; use std::borrow::Cow; use std::cmp::Ordering; use std::fmt::{self, Debug}; -use std::hash::Hasher; +use std::hash::{Hash, Hasher}; use std::sync::Arc; use ecow::EcoString; @@ -275,6 +275,12 @@ pub trait NativeElement: Debug + Repr + Construct + Set + Send + Sync + 'static fn fields(&self) -> Dict; } +impl Hash for dyn NativeElement { + fn hash(&self, state: &mut H) { + self.dyn_hash(state); + } +} + /// An element's constructor function. pub trait Construct { /// Construct an element from the arguments. diff --git a/crates/typst/src/foundations/styles.rs b/crates/typst/src/foundations/styles.rs index f1ed13f45..c85cab413 100644 --- a/crates/typst/src/foundations/styles.rs +++ b/crates/typst/src/foundations/styles.rs @@ -1,4 +1,4 @@ -use std::any::Any; +use std::any::{Any, TypeId}; use std::borrow::Cow; use std::fmt::{self, Debug, Formatter}; use std::hash::{Hash, Hasher}; @@ -258,6 +258,7 @@ impl Debug for Property { /// We're using a `Box` since values will either be contained in an `Arc` and /// therefore already on the heap or they will be small enough that we can just /// clone them. +#[derive(Hash)] struct Block(Box); impl Block { @@ -278,12 +279,6 @@ impl Debug for Block { } } -impl Hash for Block { - fn hash(&self, state: &mut H) { - self.0.dyn_hash(state); - } -} - impl Clone for Block { fn clone(&self) -> Self { self.0.dyn_clone() @@ -318,6 +313,9 @@ impl Blockable for T { } fn dyn_hash(&self, mut state: &mut dyn Hasher) { + // Also hash the TypeId since values with different types but + // equal data should be different. + TypeId::of::().hash(&mut state); self.hash(&mut state); } @@ -326,6 +324,12 @@ impl Blockable for T { } } +impl Hash for dyn Blockable { + fn hash(&self, state: &mut H) { + self.dyn_hash(state); + } +} + /// A show rule recipe. #[derive(Clone, PartialEq, Hash)] pub struct Recipe { diff --git a/crates/typst/src/foundations/value.rs b/crates/typst/src/foundations/value.rs index a1660e85d..d45c8412e 100644 --- a/crates/typst/src/foundations/value.rs +++ b/crates/typst/src/foundations/value.rs @@ -1,4 +1,4 @@ -use std::any::Any; +use std::any::{Any, TypeId}; use std::cmp::Ordering; use std::fmt::{self, Debug, Formatter}; use std::hash::{Hash, Hasher}; @@ -8,7 +8,6 @@ use ecow::{eco_format, EcoString}; use serde::de::value::{MapAccessDeserializer, SeqAccessDeserializer}; use serde::de::{Error, MapAccess, SeqAccess, Visitor}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use siphasher::sip128::{Hasher128, SipHasher13}; use crate::diag::StrResult; use crate::eval::ops; @@ -544,7 +543,7 @@ trait Bounds: Debug + Repr + Sync + Send + 'static { fn as_any(&self) -> &dyn Any; fn dyn_eq(&self, other: &Dynamic) -> bool; fn dyn_ty(&self) -> Type; - fn hash128(&self) -> u128; + fn dyn_hash(&self, state: &mut dyn Hasher); } impl Bounds for T @@ -564,20 +563,17 @@ where Type::of::() } - #[tracing::instrument(skip_all)] - fn hash128(&self) -> u128 { + fn dyn_hash(&self, mut state: &mut dyn Hasher) { // Also hash the TypeId since values with different types but // equal data should be different. - let mut state = SipHasher13::new(); - self.type_id().hash(&mut state); + TypeId::of::().hash(&mut state); self.hash(&mut state); - state.finish128().as_u128() } } impl Hash for dyn Bounds { fn hash(&self, state: &mut H) { - state.write_u128(self.hash128()); + self.dyn_hash(state); } }