Skip to content

fix(assembly-syntax): canonicalize PathBuf::try_from(String)#2991

Open
KitaroMoha563 wants to merge 2 commits into0xMiden:nextfrom
KitaroMoha563:fix/assembly-syntax-tryfrom-string-canonicalize
Open

fix(assembly-syntax): canonicalize PathBuf::try_from(String)#2991
KitaroMoha563 wants to merge 2 commits into0xMiden:nextfrom
KitaroMoha563:fix/assembly-syntax-tryfrom-string-canonicalize

Conversation

@KitaroMoha563
Copy link
Copy Markdown
Contributor

Describe your changes

  • Canonicalize PathBuf::try_from(String) by delegating to PathBuf::new, so it matches TryFrom<&str> and FromStr.
  • Add a regression test in path_buf.rs to ensure PathBuf::try_from("foo::\"bar\"".to_string()) is equal to PathBuf::try_from("foo::\"bar\"") and canonicalizes to foo::bar.
  • This fixes inconsistent equality/hashing behavior for semantically equivalent paths created from different conversion APIs.

Checklist before requesting a review

  • Repo forked and branch created from next according to naming convention.
  • Commit messages and codestyle follow conventions.
  • Commits are signed.
  • Relevant issues are linked in the PR description.
  • Tests added for new functionality.
  • Documentation/comments updated according to changes.
  • Updated `CHANGELOG.md'

@github-actions
Copy link
Copy Markdown

This PR contains unsigned commits. All commits must be cryptographically signed (GPG or SSH).

Unsigned commits:

  • e747f98b fix(assembly-syntax): canonicalize PathBuf::try_from(String)

For instructions on setting up commit signing and re-signing existing commits, see:
https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

@github-actions
Copy link
Copy Markdown

Automated check (CONTRIBUTING.md)

Findings:

Recommendations:

  • Consider adding a Test plan or clear review steps.

Next steps:

  • Link a relevant issue (e.g., "Fixes Implement SHA256 in Miden Assembly #123") and ensure it is assigned to you.
  • See CONTRIBUTING.md for expectations.
  • If this is a false positive, comment: /quality-review.

@KitaroMoha563 KitaroMoha563 force-pushed the fix/assembly-syntax-tryfrom-string-canonicalize branch from e747f98 to 0977c61 Compare April 10, 2026 15:29
fn try_from(value: String) -> Result<Self, Self::Error> {
Path::validate(&value)?;
Ok(PathBuf { inner: value })
PathBuf::new(&value)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always reallocate, we only want to reallocate if we need to

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bitwalker Updated. Please take another look, thanks for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants