Skip to content

Commit

Permalink
refactor: make PackageExport::name to be QualifiedProcedureName,
Browse files Browse the repository at this point in the history
Implement `Serializable/Deserializable` and `Arbitrary`(proptest)
for `QualifiedProcedureName`. Add serialization roundtrip tests for
`LibraryPath` and `ProcedureName`.
  • Loading branch information
greenhat committed Jan 20, 2025
1 parent a6f78bf commit 915c093
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 38 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion air/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ winter-prover = { package = "winter-prover", version = "0.11", default-features

[dev-dependencies]
criterion = "0.5"
proptest = "1.5"
proptest = "1.6"
rand-utils = { package = "winter-rand-utils", version = "0.11" }
5 changes: 4 additions & 1 deletion assembly/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ doctest = false
[features]
default = ["std"]
std = ["aho-corasick/std", "miette/fancy", "miette/std", "vm-core/std", "thiserror/std"]
testing = ["dep:regex"]
testing = ["dep:regex", "dep:proptest", "dep:proptest-derive"]

[dependencies]
aho-corasick = { version = "1.1", default-features = false }
Expand All @@ -29,6 +29,8 @@ miette = { package = "miden-miette", version = "8.0", default-features = false,
"fancy-no-syscall",
"derive"
] }
proptest = { version = "1.6", optional = true }
proptest-derive = { version = "0.5", optional = true }
regex = { version = "1.10", optional = true, default-features = false, features = ["unicode", "perf"] }
smallvec = { version = "1.13", features = ["union", "const_generics", "const_new"] }
thiserror = { workspace = true }
Expand All @@ -41,6 +43,7 @@ vm-core = { package = "miden-core", path = "../core", version = "0.11", default-
[dev-dependencies]
pretty_assertions = "1.4"
regex = { version = "1.10", default-features = false, features = [ "unicode", "perf"] }
proptest = "1.6"

[build-dependencies]
lalrpop = { version = "0.20", default-features = false }
Expand Down
75 changes: 58 additions & 17 deletions assembly/src/ast/procedure/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ use crate::{
/// A qualified procedure name can be context-sensitive, i.e. the module path might refer
/// to an imported
#[derive(Clone)]
#[cfg_attr(feature = "testing", derive(proptest_derive::Arbitrary))]
pub struct QualifiedProcedureName {
/// The source span associated with this identifier.
#[cfg_attr(feature = "testing", proptest(value = "SourceSpan::default()"))]
pub span: SourceSpan,
/// The module path for this procedure.
pub module: LibraryPath,
Expand Down Expand Up @@ -121,6 +123,21 @@ impl fmt::Display for QualifiedProcedureName {
}
}

impl Serializable for QualifiedProcedureName {
fn write_into<W: ByteWriter>(&self, target: &mut W) {
self.module.write_into(target);
self.name.write_into(target);
}
}

impl Deserializable for QualifiedProcedureName {
fn read_from<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
let module = LibraryPath::read_from(source)?;
let name = ProcedureName::read_from(source)?;
Ok(Self::new(module, name))
}
}

// PROCEDURE NAME
// ================================================================================================

Expand Down Expand Up @@ -349,31 +366,55 @@ impl Deserializable for ProcedureName {
}
}

// ARBITRARY IMPLEMENTATION
// ================================================================================================

#[cfg(feature = "testing")]
impl proptest::prelude::Arbitrary for ProcedureName {
type Parameters = ();

fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
use proptest::prelude::*;
// see https://doc.rust-lang.org/rustc/symbol-mangling/v0.html#symbol-grammar-summary
let all_possible_chars_in_mangled_name =
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.$";
let mangled_rustc_name = ProcedureName::new_unchecked(Ident::new_unchecked(Span::new(
SourceSpan::UNKNOWN,
all_possible_chars_in_mangled_name.into(),
)));
let plain = ProcedureName::new_unchecked(Ident::new_unchecked(Span::new(
SourceSpan::UNKNOWN,
"userfunc".into(),
)));
let quoted = any::<String>().prop_map(|s| {
ProcedureName::new_unchecked(Ident::new_unchecked(Span::new(
SourceSpan::UNKNOWN,
format!("\"{s}\"").into(),
)))
});
prop_oneof![Just(mangled_rustc_name), Just(plain), quoted].boxed()
}

type Strategy = proptest::prelude::BoxedStrategy<Self>;
}

// TESTS
// ================================================================================================

/// Tests
#[cfg(test)]
mod tests {
use vm_core::{
debuginfo::Span,
utils::{Deserializable, Serializable},
};
use proptest::prelude::*;
use vm_core::utils::{Deserializable, Serializable};

use super::ProcedureName;
use crate::ast::Ident;

#[test]
fn procedure_name_serialization_rustc_mangled() {
// Tests that rustc mangled symbols are serialized and deserialized
// see https://doc.rust-lang.org/rustc/symbol-mangling/v0.html#symbol-grammar-summary
let all_possible_chars_in_mangled_name =
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.$";
let proc_name = ProcedureName::new_unchecked(Ident::new_unchecked(Span::unknown(
all_possible_chars_in_mangled_name.into(),
)));
let bytes = proc_name.to_bytes();
let deserialized = ProcedureName::read_from_bytes(&bytes).unwrap();
assert_eq!(proc_name, deserialized);
proptest! {
#[test]
fn procedure_name_serialization_roundtrip(path in any::<ProcedureName>()) {
let bytes = path.to_bytes();
let deserialized = ProcedureName::read_from_bytes(&bytes).unwrap();
assert_eq!(path, deserialized);
}
}
}
56 changes: 41 additions & 15 deletions assembly/src/library/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,22 +551,53 @@ fn validate_component(component: &str) -> Result<(), PathError> {
}
}

// ARBITRARY IMPLEMENTATION
// ================================================================================================

#[cfg(feature = "testing")]
impl proptest::prelude::Arbitrary for LibraryPath {
type Parameters = ();

fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
use proptest::prelude::*;
let wasm_cm_style = LibraryPath::new_from_components(
LibraryNamespace::Anon,
[Ident::new_unchecked(Span::new(
SourceSpan::UNKNOWN,
"namespace:package/[email protected]".into(),
))],
);
let path_len_2 = LibraryPath::new_from_components(
LibraryNamespace::User("userns".into()),
[Ident::new_unchecked(Span::new(SourceSpan::UNKNOWN, "usermodule".into()))],
);
let path_len_3 = LibraryPath::new_from_components(
LibraryNamespace::User("userns".into()),
[
Ident::new_unchecked(Span::new(SourceSpan::UNKNOWN, "userns2".into())),
Ident::new_unchecked(Span::new(SourceSpan::UNKNOWN, "usermodule".into())),
],
);
prop_oneof![Just(wasm_cm_style), Just(path_len_2), Just(path_len_3)].boxed()
}

type Strategy = proptest::prelude::BoxedStrategy<Self>;
}

// TESTS
// ================================================================================================

/// Tests
#[cfg(test)]
mod tests {
use std::sync::Arc;

use proptest::prelude::*;
use vm_core::{
assert_matches,
debuginfo::{SourceSpan, Span},
utils::{Deserializable, Serializable},
};

use super::{super::LibraryNamespaceError, IdentError, LibraryPath, PathError};
use crate::{alloc::string::ToString, ast::Ident, LibraryNamespace};

#[test]
fn new_path() {
Expand Down Expand Up @@ -616,17 +647,12 @@ mod tests {
);
}

#[test]
fn path_serialization_wasm_cm_style_module_name() {
// Tests that Wasm Component Model names are serialized and deserialized correctly
let wasm_cm_style_module_name = "namespace:package/[email protected]";
let module_id = Ident::new_unchecked(Span::new(
SourceSpan::default(),
Arc::from(wasm_cm_style_module_name.to_string()),
));
let path = LibraryPath::new_from_components(LibraryNamespace::Anon, [module_id]);
let bytes = path.to_bytes();
let deserialized = LibraryPath::read_from_bytes(&bytes).unwrap();
assert_eq!(path, deserialized);
proptest! {
#[test]
fn path_serialization_roundtrip(path in any::<LibraryPath>()) {
let bytes = path.to_bytes();
let deserialized = LibraryPath::read_from_bytes(&bytes).unwrap();
assert_eq!(path, deserialized);
}
}
}
2 changes: 1 addition & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ winter-utils = { package = "winter-utils", version = "0.11", default-features =

[dev-dependencies]
loom = "0.7"
proptest = "1.5"
proptest = "1.6"
rand-utils = { package = "winter-rand-utils", version = "0.11" }

[target.'cfg(loom)'.dependencies]
Expand Down
2 changes: 1 addition & 1 deletion package/src/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub struct PackageManifest {
#[cfg_attr(test, derive(proptest_derive::Arbitrary))]
pub struct PackageExport {
/// The fully-qualified name of the procedure exported by this package
pub name: String,
pub name: QualifiedProcedureName,
/// The digest of the procedure exported by this package
#[cfg_attr(test, proptest(value = "Digest::default()"))]
pub digest: Digest,
Expand Down
4 changes: 2 additions & 2 deletions package/src/package/serialization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
use alloc::{collections::BTreeSet, format, string::String, sync::Arc, vec::Vec};

use assembly::Library;
use assembly::{ast::QualifiedProcedureName, Library};
use vm_core::{
utils::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable},
Program,
Expand Down Expand Up @@ -205,7 +205,7 @@ impl Serializable for PackageExport {

impl Deserializable for PackageExport {
fn read_from<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
let name = String::read_from(source)?;
let name = QualifiedProcedureName::read_from(source)?;
let digest = Digest::read_from(source)?;
Ok(Self { name, digest })
}
Expand Down

0 comments on commit 915c093

Please sign in to comment.