diff --git a/Cargo.lock b/Cargo.lock index 301e9022..f4fc2b4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,12 +11,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "allocator-api2" -version = "0.2.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" - [[package]] name = "anstream" version = "0.6.15" @@ -223,8 +217,6 @@ dependencies = [ "byteorder", "candid_derive 0.6.6", "candid_parser 0.2.0-beta.4", - "foldhash", - "hashbrown 0.15.2", "hex", "ic_principal 0.1.1", "leb128", @@ -615,12 +607,6 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" -[[package]] -name = "foldhash" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a0d2fde1f7b3d48b8395d5f2de76c18a528bd6a9cdde438df747bfcba3e05d6f" - [[package]] name = "generic-array" version = "0.14.7" @@ -688,17 +674,6 @@ version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" -[[package]] -name = "hashbrown" -version = "0.15.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf151400ff0baff5465007dd2f3e717f3fe502074ca563069ce3a6629d07b289" -dependencies = [ - "allocator-api2", - "equivalent", - "foldhash", -] - [[package]] name = "heck" version = "0.5.0" @@ -754,7 +729,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" dependencies = [ "equivalent", - "hashbrown 0.14.5", + "hashbrown", ] [[package]] diff --git a/Changelog.md b/Changelog.md index c41230ee..4b0e4cd2 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,14 +1,6 @@ # Changelog -## Unreleased - -### Candid - -* [BREAKING]: type representation was optimized to improve performance: - * In `Type::Var(var)` `var` now has type `TypeKey` instead of `String`. Calling `var.as_str()` returns `&str` and `var.to_string()` returns a `String`. The string representation of indexed variables remains `table{index}` to maintain compatibility with previous versions. - * `TypeEnv` now contains a `HashMap` instead of `BTreeMap`. Code that relied on the iteration order of the map (e.g. `env.0.iter()`) should make use of the newly added `TypeEnv::to_sorted_iter()` method which returns types sorted by their keys. - ## 2025-01-15 ### Candid 0.10.12 diff --git a/clippy.toml b/clippy.toml deleted file mode 100644 index db551b3a..00000000 --- a/clippy.toml +++ /dev/null @@ -1 +0,0 @@ -ignore-interior-mutability = ["candid::types::type_key::TypeKey"] diff --git a/rust/candid/Cargo.toml b/rust/candid/Cargo.toml index 0e39f40e..d91d13ce 100644 --- a/rust/candid/Cargo.toml +++ b/rust/candid/Cargo.toml @@ -19,8 +19,6 @@ candid_derive = { path = "../candid_derive", version = "=0.6.6" } ic_principal = { path = "../ic_principal", version = "0.1.0" } binread = { version = "2.2", features = ["debug_template"] } byteorder = "1.5.0" -foldhash = "0.1.3" -hashbrown = "0.15" leb128 = "0.2.5" paste = "1.0" hex.workspace = true diff --git a/rust/candid/src/binary_parser.rs b/rust/candid/src/binary_parser.rs index f2663c9b..d0c1ea0e 100644 --- a/rust/candid/src/binary_parser.rs +++ b/rust/candid/src/binary_parser.rs @@ -1,5 +1,4 @@ -use crate::types::internal::{Field, Function, Label, Type, TypeInner, TypeKey}; -use crate::types::type_env::TypeMap; +use crate::types::internal::{Field, Function, Label, Type, TypeInner}; use crate::types::{FuncMode, TypeEnv}; use anyhow::{anyhow, Context, Result}; use binread::io::{Read, Seek}; @@ -136,6 +135,9 @@ pub struct PrincipalBytes { pub inner: Vec, } +fn index_to_var(ind: i64) -> String { + format!("table{ind}") +} impl IndexType { fn to_type(&self, len: u64) -> Result { Ok(match self.index { @@ -143,7 +145,7 @@ impl IndexType { if v >= len as i64 { return Err(anyhow!("type index {} out of range", v)); } - TypeInner::Var(TypeKey::indexed(v)) + TypeInner::Var(index_to_var(v)) } -1 => TypeInner::Null, -2 => TypeInner::Bool, @@ -231,12 +233,13 @@ impl ConsType { } impl Table { fn to_env(&self, len: u64) -> Result { - let mut env = TypeMap::default(); + use std::collections::BTreeMap; + let mut env = BTreeMap::new(); for (i, t) in self.table.iter().enumerate() { let ty = t .to_type(len) .with_context(|| format!("Invalid table entry {i}: {t:?}"))?; - env.insert(TypeKey::indexed(i as i64), ty); + env.insert(index_to_var(i as i64), ty); } // validate method has func type for t in env.values() { diff --git a/rust/candid/src/lib.rs b/rust/candid/src/lib.rs index 329f9dcc..e83807b4 100644 --- a/rust/candid/src/lib.rs +++ b/rust/candid/src/lib.rs @@ -275,7 +275,7 @@ pub mod pretty; // Candid hash function comes from // https://caml.inria.fr/pub/papers/garrigue-polymorphic_variants-ml98.pdf -// Not public API. +// Not public API. Only used by tests. // Remember to update the same function in candid_derive if you change this function. #[doc(hidden)] #[inline] diff --git a/rust/candid/src/pretty/candid.rs b/rust/candid/src/pretty/candid.rs index 6f83bb93..5f67607d 100644 --- a/rust/candid/src/pretty/candid.rs +++ b/rust/candid/src/pretty/candid.rs @@ -95,7 +95,7 @@ pub fn pp_ty_inner(ty: &TypeInner) -> RcDoc { Text => str("text"), Reserved => str("reserved"), Empty => str("empty"), - Var(ref s) => str(s.as_str()), + Var(ref s) => str(s), Principal => str("principal"), Opt(ref t) => kwd("opt").append(pp_ty(t)), Vec(ref t) if matches!(t.as_ref(), Nat8) => str("blob"), @@ -116,7 +116,7 @@ pub fn pp_ty_inner(ty: &TypeInner) -> RcDoc { let doc = pp_args(args).append(" ->").append(RcDoc::space()); match t.as_ref() { Service(ref serv) => doc.append(pp_service(serv)), - Var(ref s) => doc.append(s.as_str()), + Var(ref s) => doc.append(s), _ => unreachable!(), } } @@ -188,9 +188,9 @@ fn pp_service(serv: &[(String, Type)]) -> RcDoc { } fn pp_defs(env: &TypeEnv) -> RcDoc { - lines(env.to_sorted_iter().map(|(id, ty)| { + lines(env.0.iter().map(|(id, ty)| { kwd("type") - .append(ident(id.as_str())) + .append(ident(id)) .append(kwd("=")) .append(pp_ty(ty)) .append(";") diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index 7ea00523..c6d2b983 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -1,13 +1,8 @@ use super::CandidType; use crate::idl_hash; -use crate::types::type_env::TypeMap; use std::cell::RefCell; use std::collections::BTreeMap; use std::fmt; -use std::fmt::Debug; -use std::hash::Hash; - -pub use crate::types::type_key::TypeKey; // This is a re-implementation of std::any::TypeId to get rid of 'static constraint. // The current TypeId doesn't consider lifetime while computing the hash, which is @@ -29,7 +24,7 @@ impl TypeId { impl std::fmt::Display for TypeId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let name = NAME.with(|n| n.borrow_mut().get(self)); - f.write_str(&name) + write!(f, "{name}") } } pub fn type_of(_: &T) -> TypeId { @@ -118,9 +113,8 @@ impl TypeContainer { } let id = ID.with(|n| n.borrow().get(t).cloned()); if let Some(id) = id { - let type_key = TypeKey::from(id.to_string()); - self.env.0.insert(type_key.clone(), res); - TypeInner::Var(type_key) + self.env.0.insert(id.to_string(), res); + TypeInner::Var(id.to_string()) } else { // if the type is part of an enum, the id won't be recorded. // we want to inline the type in this case. @@ -139,18 +133,17 @@ impl TypeContainer { .into(); let id = ID.with(|n| n.borrow().get(t).cloned()); if let Some(id) = id { - let type_key = TypeKey::from(id.to_string()); - self.env.0.insert(type_key.clone(), res); - TypeInner::Var(type_key) + self.env.0.insert(id.to_string(), res); + TypeInner::Var(id.to_string()) } else { return res; } } TypeInner::Knot(id) => { + let name = id.to_string(); let ty = ENV.with(|e| e.borrow().get(id).unwrap().clone()); - let type_key = TypeKey::from(id.to_string()); - self.env.0.insert(type_key.clone(), ty); - TypeInner::Var(type_key) + self.env.0.insert(id.to_string(), ty); + TypeInner::Var(name) } TypeInner::Func(func) => TypeInner::Func(Function { modes: func.modes.clone(), @@ -194,7 +187,7 @@ pub enum TypeInner { Reserved, Empty, Knot(TypeId), // For recursive types from Rust - Var(TypeKey), // For variables from Candid file + Var(String), // For variables from Candid file Unknown, Opt(Type), Vec(Type), @@ -255,12 +248,12 @@ impl Type { pub fn is_blob(&self, env: &crate::TypeEnv) -> bool { self.as_ref().is_blob(env) } - pub fn subst(&self, tau: &TypeMap) -> Self { + pub fn subst(&self, tau: &std::collections::BTreeMap) -> Self { use TypeInner::*; match self.as_ref() { Var(id) => match tau.get(id) { - None => Var(id.clone()), - Some(new_id) => Var(new_id.clone()), + None => Var(id.to_string()), + Some(new_id) => Var(new_id.to_string()), }, Opt(t) => Opt(t.subst(tau)), Vec(t) => Vec(t.subst(tau)), @@ -337,7 +330,7 @@ pub fn text_size(t: &Type, limit: i32) -> Result { Reserved => 8, Principal => 9, Knot(_) => 10, - Var(id) => id.as_str().len() as i32, + Var(id) => id.len() as i32, Opt(t) => 4 + text_size(t, limit - 4)?, Vec(t) => 4 + text_size(t, limit - 4)?, Record(fs) | Variant(fs) => { diff --git a/rust/candid/src/types/mod.rs b/rust/candid/src/types/mod.rs index a286bdb1..49af606d 100644 --- a/rust/candid/src/types/mod.rs +++ b/rust/candid/src/types/mod.rs @@ -29,7 +29,6 @@ pub mod result; pub mod arc; pub mod rc; -mod type_key; pub trait CandidType { // memoized type derivation diff --git a/rust/candid/src/types/type_env.rs b/rust/candid/src/types/type_env.rs index 5f5aef1c..0723f276 100644 --- a/rust/candid/src/types/type_env.rs +++ b/rust/candid/src/types/type_env.rs @@ -1,22 +1,17 @@ -use crate::types::internal::TypeKey; use crate::types::{Function, Type, TypeInner}; use crate::{Error, Result}; -use foldhash::fast::FixedState; -use hashbrown::HashMap; - -pub type TypeMap = HashMap; +use std::collections::BTreeMap; #[derive(Debug, Clone, Default)] -pub struct TypeEnv(pub TypeMap); +pub struct TypeEnv(pub BTreeMap); impl TypeEnv { pub fn new() -> Self { - TypeEnv(TypeMap::default()) + TypeEnv(BTreeMap::new()) } - pub fn merge<'a>(&'a mut self, env: &TypeEnv) -> Result<&'a mut Self> { - for (k, v) in env.0.iter() { - let entry = self.0.entry(k.clone()).or_insert_with(|| v.clone()); + for (k, v) in &env.0 { + let entry = self.0.entry(k.to_string()).or_insert_with(|| v.clone()); if *entry != *v { return Err(Error::msg("inconsistent binding")); } @@ -24,11 +19,11 @@ impl TypeEnv { Ok(self) } pub fn merge_type(&mut self, env: TypeEnv, ty: Type) -> Type { - let tau: TypeMap = env + let tau: BTreeMap = env .0 .keys() .filter(|k| self.0.contains_key(*k)) - .map(|k| (k.clone(), format!("{k}/1").into())) + .map(|k| (k.clone(), format!("{k}/1"))) .collect(); for (k, t) in env.0 { let t = t.subst(&tau); @@ -40,14 +35,13 @@ impl TypeEnv { } ty.subst(&tau) } - pub fn find_type(&self, key: &TypeKey) -> Result<&Type> { - match self.0.get(key) { - None => Err(Error::msg(format!("Unbound type identifier {key}"))), + pub fn find_type(&self, name: &str) -> Result<&Type> { + match self.0.get(name) { + None => Err(Error::msg(format!("Unbound type identifier {name}"))), Some(t) => Ok(t), } } - - pub fn rec_find_type(&self, name: &TypeKey) -> Result<&Type> { + pub fn rec_find_type(&self, name: &str) -> Result<&Type> { let t = self.find_type(name)?; match t.as_ref() { TypeInner::Var(id) => self.rec_find_type(id), @@ -88,8 +82,8 @@ impl TypeEnv { } fn is_empty<'a>( &'a self, - res: &mut HashMap<&'a TypeKey, Option, FixedState>, - id: &'a TypeKey, + res: &mut BTreeMap<&'a str, Option>, + id: &'a str, ) -> Result { match res.get(id) { None => { @@ -122,34 +116,24 @@ impl TypeEnv { } } pub fn replace_empty(&mut self) -> Result<()> { - let mut res = HashMap::default(); - for key in self.0.keys() { - self.is_empty(&mut res, key)?; + let mut res = BTreeMap::new(); + for name in self.0.keys() { + self.is_empty(&mut res, name)?; } let ids: Vec<_> = res - .into_iter() + .iter() .filter(|(_, v)| matches!(v, Some(true))) - .map(|(id, _)| id.to_owned()) + .map(|(id, _)| id.to_string()) .collect(); for id in ids { self.0.insert(id, TypeInner::Empty.into()); } Ok(()) } - - /// Creates an iterator that iterates over the types in the order of keys. - /// - /// The implementation collects elements into a temporary vector and sorts the vector. - pub fn to_sorted_iter(&self) -> impl Iterator { - let mut vec: Vec<_> = self.0.iter().collect(); - vec.sort_unstable_by_key(|elem| elem.0); - vec.into_iter() - } } - impl std::fmt::Display for TypeEnv { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for (k, v) in self.to_sorted_iter() { + for (k, v) in &self.0 { writeln!(f, "type {k} = {v}")?; } Ok(()) diff --git a/rust/candid/src/types/type_key.rs b/rust/candid/src/types/type_key.rs deleted file mode 100644 index 238f37b2..00000000 --- a/rust/candid/src/types/type_key.rs +++ /dev/null @@ -1,230 +0,0 @@ -use crate::idl_hash; -use std::cell::OnceCell; -use std::cmp::Ordering; -use std::fmt; -use std::fmt::{Debug, Display, Formatter}; -use std::hash::{Hash, Hasher}; -use std::str::FromStr; - -#[derive(Clone)] -enum TypeKeyInner { - Indexed { index: i64, name: OnceCell }, - Named { name: String, hash: u32 }, -} - -impl TypeKeyInner { - pub fn as_str(&self) -> &str { - match self { - TypeKeyInner::Indexed { index, name } => { - name.get_or_init(|| TypeKey::name_from_index(*index)) - } - TypeKeyInner::Named { name, .. } => name, - } - } -} - -impl Debug for TypeKeyInner { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) - } -} - -/// A `TypeKey` represents a type from a Candid schema. The type can be string-based or index-based. -#[derive(Debug, Clone)] -pub struct TypeKey(TypeKeyInner); - -impl TypeKey { - const INDEXED_PREFIX: &'static str = "table"; - - pub fn indexed(idx: i64) -> Self { - TypeKeyInner::Indexed { - index: idx, - name: Default::default(), - } - .into() - } - - pub fn as_str(&self) -> &str { - self.0.as_str() - } - - fn name_from_index(i: i64) -> String { - format!("{}{}", Self::INDEXED_PREFIX, i) - } - - fn maybe_indexed(value: &str) -> Option { - let index = i64::from_str(value.strip_prefix(Self::INDEXED_PREFIX)?).ok()?; - Some(TypeKey::indexed(index)) - } -} - -impl From for TypeKey { - fn from(value: TypeKeyInner) -> Self { - Self(value) - } -} - -impl PartialOrd for TypeKey { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for TypeKey { - fn cmp(&self, other: &Self) -> Ordering { - // If the performance of this function ever becomes too slow, the function can be optimized - // by specializing comparison of two TypeKeyInner::Indexed objects. - self.as_str().cmp(other.as_str()) - } -} - -impl PartialEq for TypeKey { - fn eq(&self, other: &Self) -> bool { - match (&self.0, &other.0) { - ( - TypeKeyInner::Indexed { index: index1, .. }, - TypeKeyInner::Indexed { index: index2, .. }, - ) => *index1 == *index2, - ( - TypeKeyInner::Named { - hash: hash1, - name: name1, - }, - TypeKeyInner::Named { - hash: hash2, - name: name2, - }, - ) => *hash1 == *hash2 && name1 == name2, - _ => false, - } - } -} - -impl Eq for TypeKey {} - -impl Hash for TypeKey { - fn hash(&self, state: &mut H) { - match &self.0 { - TypeKeyInner::Indexed { index, .. } => index.hash(state), - TypeKeyInner::Named { hash, .. } => hash.hash(state), - } - } -} - -impl Display for TypeKey { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) - } -} - -impl From for TypeKey { - fn from(value: String) -> Self { - Self::maybe_indexed(&value).unwrap_or_else(|| { - TypeKeyInner::Named { - hash: idl_hash(&value), - name: value, - } - .into() - }) - } -} - -impl From<&str> for TypeKey { - fn from(value: &str) -> Self { - Self::maybe_indexed(value).unwrap_or_else(|| { - TypeKeyInner::Named { - hash: idl_hash(value), - name: value.to_string(), - } - .into() - }) - } -} - -impl AsRef for TypeKey { - fn as_ref(&self) -> &str { - self.as_str() - } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::collections::HashMap; - - #[test] - fn type_key_indexed_name() { - assert_eq!(TypeKey::indexed(42).as_str(), "table42"); - } - - #[test] - fn type_key_from_string() { - assert_eq!(TypeKey::from("foobar"), TypeKey::from("foobar".to_string())); - assert_eq!(TypeKey::from("foobar").as_str(), "foobar"); - assert_eq!(TypeKey::from("table").as_str(), "table"); - // Check that indexed keys with a "broken" index are parsed correctly. - assert_eq!(TypeKey::from("table3.5").as_str(), "table3.5"); - assert_eq!(TypeKey::from("table-33").as_str(), "table-33"); - } - - #[test] - fn type_key_indexed() { - assert_eq!(TypeKey::from("table33"), TypeKey::indexed(33)); - } - - #[test] - fn type_key_hash() { - let mut map = HashMap::new(); - map.insert(TypeKey::from("a"), 1); - map.insert(TypeKey::from("bar".to_string()), 2); - map.insert(TypeKey::from("table1"), 3); - map.insert(TypeKey::from("table97"), 4); - map.insert(TypeKey::indexed(33), 5); - - assert_eq!(map.get(&"a".to_string().into()), Some(&1)); - assert_eq!(map.get(&"bar".into()), Some(&2)); - assert_eq!(map.get(&TypeKey::indexed(1)), Some(&3)); - assert_eq!(map.get(&TypeKey::indexed(97)), Some(&4)); - assert_eq!(map.get(&TypeKey::from("table33")), Some(&5)); - } - - #[test] - fn type_key_ord() { - let mut keys = vec![ - TypeKey::indexed(4), - TypeKey::indexed(24), - TypeKey::from("table3"), - TypeKey::from("table"), - TypeKey::indexed(1), - TypeKey::from("table23"), - TypeKey::from("table4.3"), - TypeKey::from("zzz"), - TypeKey::from("a"), - ]; - - let expected = vec![ - TypeKey::from("a"), - TypeKey::from("table"), - TypeKey::indexed(1), - TypeKey::from("table23"), - // Note that even though 3 < 24 and 4 < 24, they're ordered afterward because we use - // string-based ordering to maintain consistency between named and indexed TypeKeys. - TypeKey::indexed(24), - TypeKey::from("table3"), - TypeKey::indexed(4), - TypeKey::from("table4.3"), - TypeKey::from("zzz"), - ]; - keys.sort_unstable(); - assert_eq!(keys, expected); - } - - #[test] - fn type_key_to_string() { - assert_eq!(TypeKey::indexed(32).to_string(), "table32"); - assert_eq!(TypeKey::from("foo").to_string(), "foo"); - // debug string - assert_eq!(format!("{:?}", TypeKey::indexed(32)), "TypeKey(table32)"); - assert_eq!(format!("{:?}", TypeKey::from("foo")), "TypeKey(foo)"); - } -} diff --git a/rust/candid_parser/src/bindings/analysis.rs b/rust/candid_parser/src/bindings/analysis.rs index ec76b5c2..edd316bc 100644 --- a/rust/candid_parser/src/bindings/analysis.rs +++ b/rust/candid_parser/src/bindings/analysis.rs @@ -1,5 +1,4 @@ use crate::{Error, Result}; -use candid::types::internal::TypeKey; use candid::types::{Type, TypeEnv, TypeInner}; use std::collections::{BTreeMap, BTreeSet}; @@ -44,10 +43,10 @@ pub fn chase_type<'a>( use TypeInner::*; match t.as_ref() { Var(id) => { - if seen.insert(id.as_str()) { + if seen.insert(id) { let t = env.find_type(id)?; chase_type(seen, res, env, t)?; - res.push(id.as_str()); + res.push(id); } } Opt(ty) | Vec(ty) => chase_type(seen, res, env, ty)?, @@ -149,8 +148,8 @@ pub fn infer_rec<'a>(_env: &'a TypeEnv, def_list: &'a [&'a str]) -> Result { - if seen.insert(id.as_str()) { - res.insert(id.as_str()); + if seen.insert(id) { + res.insert(id); } } Opt(ty) | Vec(ty) => go(seen, res, _env, ty)?, @@ -179,8 +178,8 @@ pub fn infer_rec<'a>(_env: &'a TypeEnv, def_list: &'a [&'a str]) -> Result RcDoc { Text => str("IDL.Text"), Reserved => str("IDL.Reserved"), Empty => str("IDL.Empty"), - Var(ref s) => ident(s.as_str()), + Var(ref s) => ident(s), Principal => str("IDL.Principal"), Opt(ref t) => str("IDL.Opt").append(enclose("(", pp_ty(t), ")")), Vec(ref t) => str("IDL.Vec").append(enclose("(", pp_ty(t), ")")), @@ -199,8 +199,8 @@ fn pp_defs<'a>( recs.iter() .map(|id| kwd("const").append(ident(id)).append(" = IDL.Rec();")), ); - let defs = lines(def_list.iter().map(|&id| { - let ty = env.find_type(&id.into()).unwrap(); + let defs = lines(def_list.iter().map(|id| { + let ty = env.find_type(id).unwrap(); if recs.contains(id) { ident(id) .append(".fill") @@ -220,10 +220,10 @@ fn pp_actor<'a>(ty: &'a Type, recs: &'a BTreeSet<&'a str>) -> RcDoc<'a> { match ty.as_ref() { TypeInner::Service(_) => pp_ty(ty), TypeInner::Var(id) => { - if recs.contains(id.as_str()) { - str(id.as_str()).append(".getType()") + if recs.contains(&*id.clone()) { + str(id).append(".getType()") } else { - str(id.as_str()) + str(id) } } TypeInner::Class(_, t) => pp_actor(t, recs), @@ -234,7 +234,7 @@ fn pp_actor<'a>(ty: &'a Type, recs: &'a BTreeSet<&'a str>) -> RcDoc<'a> { pub fn compile(env: &TypeEnv, actor: &Option) -> String { match actor { None => { - let def_list: Vec<_> = env.to_sorted_iter().map(|pair| pair.0.as_str()).collect(); + let def_list: Vec<_> = env.0.iter().map(|pair| pair.0.as_ref()).collect(); let recs = infer_rec(env, &def_list).unwrap(); let doc = pp_defs(env, &def_list, &recs); doc.pretty(LINE_WIDTH).to_string() diff --git a/rust/candid_parser/src/bindings/motoko.rs b/rust/candid_parser/src/bindings/motoko.rs index ba1fe5d6..d0fcdbd4 100644 --- a/rust/candid_parser/src/bindings/motoko.rs +++ b/rust/candid_parser/src/bindings/motoko.rs @@ -112,7 +112,7 @@ fn pp_ty(ty: &Type) -> RcDoc { Text => str("Text"), Reserved => str("Any"), Empty => str("None"), - Var(ref s) => escape(s.as_str(), false), + Var(ref s) => escape(s, false), Principal => str("Principal"), Opt(ref t) => str("?").append(pp_ty(t)), Vec(ref t) if matches!(t.as_ref(), Nat8) => str("Blob"), @@ -140,7 +140,7 @@ fn pp_ty(ty: &Type) -> RcDoc { let doc = pp_args(args).append(" -> async "); match t.as_ref() { Service(ref serv) => doc.append(pp_service(serv)), - Var(ref s) => doc.append(s.as_str()), + Var(ref s) => doc.append(s), _ => unreachable!(), } } @@ -220,9 +220,9 @@ fn pp_service(serv: &[(String, Type)]) -> RcDoc { } fn pp_defs(env: &TypeEnv) -> RcDoc { - lines(env.to_sorted_iter().map(|(id, ty)| { + lines(env.0.iter().map(|(id, ty)| { kwd("public type") - .append(escape(id.as_str(), false)) + .append(escape(id, false)) .append(" = ") .append(pp_ty(ty)) .append(";") diff --git a/rust/candid_parser/src/bindings/rust.rs b/rust/candid_parser/src/bindings/rust.rs index 651c722a..c2f58fbb 100644 --- a/rust/candid_parser/src/bindings/rust.rs +++ b/rust/candid_parser/src/bindings/rust.rs @@ -220,7 +220,7 @@ fn test_{test_name}() {{ self.state.update_stats("name"); res } else { - ident(id.as_str(), Some(Case::Pascal)) + ident(id, Some(Case::Pascal)) }; if !is_ref && self.recs.contains(id.as_str()) { str("Box<").append(name).append(">") @@ -393,13 +393,13 @@ fn test_{test_name}() {{ } fn pp_defs(&mut self, def_list: &'a [&'a str]) -> RcDoc<'a> { let mut res = Vec::with_capacity(def_list.len()); - for &id in def_list { + for id in def_list { let old = self.state.push_state(&StateElem::Label(id)); if self.state.config.use_type.is_some() { self.state.pop_state(old, StateElem::Label(id)); continue; } - let ty = self.state.env.find_type(&id.into()).unwrap(); + let ty = self.state.env.find_type(id).unwrap(); let name = self .state .config @@ -662,9 +662,7 @@ pub fn emit_bindgen(tree: &Config, env: &TypeEnv, actor: &Option) -> (Outp let def_list = if let Some(actor) = &actor { chase_actor(&env, actor).unwrap() } else { - env.to_sorted_iter() - .map(|pair| pair.0.as_str()) - .collect::>() + env.0.iter().map(|pair| pair.0.as_ref()).collect::>() }; let recs = infer_rec(&env, &def_list).unwrap(); let mut state = State { @@ -847,8 +845,8 @@ impl NominalState<'_> { &mut vec![TypePath::Id(new_var.clone())], &TypeInner::Record(fs.to_vec()).into(), ); - env.0.insert(new_var.clone().into(), ty); - TypeInner::Var(new_var.into()) + env.0.insert(new_var.clone(), ty); + TypeInner::Var(new_var) } } TypeInner::Variant(fs) => { @@ -885,8 +883,8 @@ impl NominalState<'_> { &mut vec![TypePath::Id(new_var.clone())], &TypeInner::Variant(fs.to_vec()).into(), ); - env.0.insert(new_var.clone().into(), ty); - TypeInner::Var(new_var.into()) + env.0.insert(new_var.clone(), ty); + TypeInner::Var(new_var) } } TypeInner::Func(func) => match path.last() { @@ -947,8 +945,8 @@ impl NominalState<'_> { &mut vec![TypePath::Id(new_var.clone())], &TypeInner::Func(func.clone()).into(), ); - env.0.insert(new_var.clone().into(), ty); - TypeInner::Var(new_var.into()) + env.0.insert(new_var.clone(), ty); + TypeInner::Var(new_var) } }, TypeInner::Service(serv) => match path.last() { @@ -978,8 +976,8 @@ impl NominalState<'_> { &mut vec![TypePath::Id(new_var.clone())], &TypeInner::Service(serv.clone()).into(), ); - env.0.insert(new_var.clone().into(), ty); - TypeInner::Var(new_var.into()) + env.0.insert(new_var.clone(), ty); + TypeInner::Var(new_var) } }, TypeInner::Class(args, ty) => TypeInner::Class( @@ -1007,15 +1005,11 @@ impl NominalState<'_> { fn nominalize_all(&mut self, actor: &Option) -> (TypeEnv, Option) { let mut res = TypeEnv(Default::default()); - for (id, ty) in self.state.env.to_sorted_iter() { - let elem = StateElem::Label(id.as_str()); + for (id, ty) in self.state.env.0.iter() { + let elem = StateElem::Label(id); let old = self.state.push_state(&elem); - let ty = self.nominalize( - &mut res, - &mut vec![TypePath::Id(id.as_str().to_string())], - ty, - ); - res.0.insert(id.clone(), ty); + let ty = self.nominalize(&mut res, &mut vec![TypePath::Id(id.clone())], ty); + res.0.insert(id.to_string(), ty); self.state.pop_state(old, elem); } let actor = actor diff --git a/rust/candid_parser/src/bindings/typescript.rs b/rust/candid_parser/src/bindings/typescript.rs index 264c53d2..1f4f3bfc 100644 --- a/rust/candid_parser/src/bindings/typescript.rs +++ b/rust/candid_parser/src/bindings/typescript.rs @@ -29,10 +29,10 @@ fn pp_ty<'a>(env: &'a TypeEnv, ty: &'a Type, is_ref: bool) -> RcDoc<'a> { if matches!(ty.as_ref(), Service(_) | Func(_)) { pp_ty(env, ty, false) } else { - ident(id.as_str()) + ident(id) } } else { - ident(id.as_str()) + ident(id) } } Principal => str("Principal"), @@ -142,8 +142,8 @@ fn pp_service<'a>(env: &'a TypeEnv, serv: &'a [(String, Type)]) -> RcDoc<'a> { } fn pp_defs<'a>(env: &'a TypeEnv, def_list: &'a [&'a str]) -> RcDoc<'a> { - lines(def_list.iter().map(|&id| { - let ty = env.find_type(&id.into()).unwrap(); + lines(def_list.iter().map(|id| { + let ty = env.find_type(id).unwrap(); let export = match ty.as_ref() { TypeInner::Record(_) if !ty.is_tuple() => kwd("export interface") .append(ident(id)) @@ -174,7 +174,7 @@ fn pp_actor<'a>(env: &'a TypeEnv, ty: &'a Type) -> RcDoc<'a> { kwd("export interface _SERVICE").append(pp_service(env, serv)) } TypeInner::Var(id) => kwd("export interface _SERVICE extends") - .append(str(id.as_str())) + .append(str(id)) .append(str(" {}")), TypeInner::Class(_, t) => pp_actor(env, t), _ => unreachable!(), @@ -186,7 +186,7 @@ pub fn compile(env: &TypeEnv, actor: &Option) -> String { import type { ActorMethod } from '@dfinity/agent'; import type { IDL } from '@dfinity/candid'; "#; - let def_list: Vec<_> = env.to_sorted_iter().map(|pair| pair.0.as_str()).collect(); + let def_list: Vec<_> = env.0.iter().map(|pair| pair.0.as_ref()).collect(); let defs = pp_defs(env, &def_list); let actor = match actor { None => RcDoc::nil(), diff --git a/rust/candid_parser/src/configs.rs b/rust/candid_parser/src/configs.rs index e50d80a1..4b6a57cd 100644 --- a/rust/candid_parser/src/configs.rs +++ b/rust/candid_parser/src/configs.rs @@ -393,7 +393,7 @@ fn path_name(t: &Type) -> String { TypeInner::Text => "text", TypeInner::Reserved => "reserved", TypeInner::Empty => "empty", - TypeInner::Var(id) => id.as_str(), + TypeInner::Var(id) => id, TypeInner::Knot(id) => id.name, TypeInner::Principal => "principal", TypeInner::Opt(_) => "opt", diff --git a/rust/candid_parser/src/lib.rs b/rust/candid_parser/src/lib.rs index 780b92a9..9853c3d0 100644 --- a/rust/candid_parser/src/lib.rs +++ b/rust/candid_parser/src/lib.rs @@ -69,7 +69,7 @@ //! //! let method = env.get_method(&actor, "g").unwrap(); //! assert_eq!(method.is_query(), true); -//! assert_eq!(method.args, vec![TypeInner::Var("List".into()).into()]); +//! assert_eq!(method.args, vec![TypeInner::Var("List".to_string()).into()]); //! # Ok(()) //! # } //! ``` diff --git a/rust/candid_parser/src/random.rs b/rust/candid_parser/src/random.rs index 31ca3bbd..8a38e586 100644 --- a/rust/candid_parser/src/random.rs +++ b/rust/candid_parser/src/random.rs @@ -300,7 +300,7 @@ fn size_helper(env: &TypeEnv, seen: &mut HashSet, t: &Type) -> Option Result { match t { IDLType::PrimT(prim) => Ok(check_prim(prim)), IDLType::VarT(id) => { - let key = id.as_str().into(); - env.te.find_type(&key)?; - Ok(TypeInner::Var(key).into()) + env.te.find_type(id)?; + Ok(TypeInner::Var(id.to_string()).into()) } IDLType::OptT(t) => { let t = check_type(env, t)?; @@ -135,7 +134,7 @@ fn check_defs(env: &mut Env, decs: &[Dec]) -> Result<()> { match dec { Dec::TypD(Binding { id, typ }) => { let t = check_type(env, typ)?; - env.te.0.insert(id.clone().into(), t); + env.te.0.insert(id.to_string(), t); } Dec::ImportType(_) | Dec::ImportServ(_) => (), } @@ -147,7 +146,7 @@ fn check_cycle(env: &TypeEnv) -> Result<()> { fn has_cycle<'a>(seen: &mut BTreeSet<&'a str>, env: &'a TypeEnv, t: &'a Type) -> Result { match t.as_ref() { TypeInner::Var(id) => { - if seen.insert(id.as_str()) { + if seen.insert(id) { let ty = env.find_type(id)?; has_cycle(seen, env, ty) } else { @@ -169,10 +168,7 @@ fn check_cycle(env: &TypeEnv) -> Result<()> { fn check_decs(env: &mut Env, decs: &[Dec]) -> Result<()> { for dec in decs.iter() { if let Dec::TypD(Binding { id, typ: _ }) = dec { - let duplicate = env - .te - .0 - .insert(id.as_str().into(), TypeInner::Unknown.into()); + let duplicate = env.te.0.insert(id.to_string(), TypeInner::Unknown.into()); if duplicate.is_some() { return Err(Error::msg(format!("duplicate binding for {id}"))); } diff --git a/rust/candid_parser/src/utils.rs b/rust/candid_parser/src/utils.rs index f591d3c5..6e24b432 100644 --- a/rust/candid_parser/src/utils.rs +++ b/rust/candid_parser/src/utils.rs @@ -1,5 +1,4 @@ use crate::{check_prog, pretty_check_file, pretty_parse, Error, Result}; -use candid::types::internal::TypeKey; use candid::types::TypeInner; use candid::{types::Type, TypeEnv}; use std::path::Path; @@ -71,8 +70,8 @@ pub fn get_metadata(env: &TypeEnv, serv: &Option) -> Option { let def_list = crate::bindings::analysis::chase_actor(env, &serv).ok()?; let mut filtered = TypeEnv::new(); for d in def_list { - if let Some(t) = env.0.get(&TypeKey::from(d)) { - filtered.0.insert(d.to_string().into(), t.clone()); + if let Some(t) = env.0.get(d) { + filtered.0.insert(d.to_string(), t.clone()); } } Some(candid::pretty::candid::compile(&filtered, &Some(serv))) diff --git a/rust/candid_parser/tests/parse_type.rs b/rust/candid_parser/tests/parse_type.rs index 9e190b64..ad956940 100644 --- a/rust/candid_parser/tests/parse_type.rs +++ b/rust/candid_parser/tests/parse_type.rs @@ -6,7 +6,6 @@ use candid_parser::types::IDLProg; use candid_parser::typing::{check_file, check_prog}; use goldenfile::Mint; use std::io::Write; -use std::panic::AssertUnwindSafe; use std::path::Path; #[test] @@ -52,10 +51,9 @@ fn compiler_test(resource: &str) { } { match filename.file_name().unwrap().to_str().unwrap() { - "unicode.did" | "escape.did" => check_error( - AssertUnwindSafe(|| motoko::compile(&env, &actor)), - "not a valid Motoko id", - ), + "unicode.did" | "escape.did" => { + check_error(|| motoko::compile(&env, &actor), "not a valid Motoko id") + } _ => { let mut output = mint.new_goldenfile(filename.with_extension("mo")).unwrap();