diff --git a/CHANGELOG.md b/CHANGELOG.md index e34020d061..2f8e0365a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Replaced unsound `ptr::read` with safe unbox in panic recovery, removing UB from potential double-drop ([#2934](https://github.com/0xMiden/miden-vm/pull/2934)). - Reverted `InvokeKind::ProcRef` back to `InvokeKind::Exec` in `visit_mut_procref` and added an explanatory comment (#2893). - Fixed the release dry-run publish cycle between `miden-air` and `miden-ace-codegen`, and preserved leaf-only DAG imports with explicit snapshots ([#2931](https://github.com/0xMiden/miden-vm/pull/2931)). +- Canonicalized `PathBuf::try_from(String)` to match `TryFrom<&str>`/`FromStr`, so semantically equivalent quoted path components compare and hash consistently. #### Changes diff --git a/crates/assembly-syntax/src/ast/path/path_buf.rs b/crates/assembly-syntax/src/ast/path/path_buf.rs index f9250c27ae..0ce13803f5 100644 --- a/crates/assembly-syntax/src/ast/path/path_buf.rs +++ b/crates/assembly-syntax/src/ast/path/path_buf.rs @@ -278,12 +278,50 @@ impl<'a> TryFrom<&'a str> for PathBuf { } } +fn is_canonical_path(path: &Path) -> bool { + let mut is_absolute = false; + let mut saw_normal_component = false; + + for component in path.components() { + let component = match component { + Ok(component) => component, + Err(_) => return false, + }; + + match component { + PathComponent::Root => is_absolute = true, + component @ PathComponent::Normal(_) => { + let is_quoted = component.is_quoted(); + let component = component.as_str(); + let is_special = matches!(component, Path::KERNEL_PATH | Path::EXEC_PATH); + let requires_quoting = !is_special && Ident::requires_quoting(component); + + if !saw_normal_component && !is_absolute && is_special { + return false; + } + + if requires_quoting != is_quoted { + return false; + } + + saw_normal_component = true; + }, + } + } + + true +} + impl TryFrom for PathBuf { type Error = PathError; fn try_from(value: String) -> Result { - Path::validate(&value)?; - Ok(PathBuf { inner: value }) + let path = Path::validate(&value)?; + if is_canonical_path(path) { + Ok(Self { inner: value }) + } else { + path.canonicalize() + } } } @@ -574,6 +612,28 @@ mod tests { assert_eq!(parent.parent().map(|p| p.as_str()), Some("::\"root_ns:root@1.0.0\"")); } + #[test] + fn try_from_string_canonicalizes_like_str() { + let from_str = PathBuf::try_from("foo::\"bar\"").unwrap(); + let from_string = PathBuf::try_from(alloc::string::String::from("foo::\"bar\"")).unwrap(); + + assert_eq!(from_string, from_str); + assert_eq!(from_string.as_str(), "foo::bar"); + } + + #[test] + fn try_from_string_reuses_canonical_allocation() { + let value = alloc::string::String::from("foo::bar"); + let original_ptr = value.as_ptr(); + let original_capacity = value.capacity(); + + let path = PathBuf::try_from(value).unwrap(); + + assert_eq!(path.as_str(), "foo::bar"); + assert_eq!(path.inner.as_ptr(), original_ptr); + assert_eq!(path.inner.capacity(), original_capacity); + } + #[test] fn exec_path() { let path = PathBuf::new("$exec::bar::baz").unwrap();