Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions crates/wasmparser/src/validator/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4663,6 +4663,18 @@ impl ComponentNameContext {
}
}

if let Some(_) = version_suffix {
require_feature::cm_canon_names(
*features,
"the `cm-canon-names` feature is not active",
offset,
)?;
match ty {
ComponentEntityType::Instance(_) => {}
_ => bail!(offset, "only instances can have an `versionsuffix`"),
}
}

if let Some(implements) = implements {
require_feature::cm_implements(
*features,
Expand All @@ -4682,22 +4694,21 @@ impl ComponentNameContext {
let implements = ComponentName::new_with_features(implements, offset, *features)
.with_context(|| format!("`{implements}` is not a valid name"))?;
match implements.kind() {
ComponentNameKind::Interface(_) => {}
ComponentNameKind::Interface(_) => {
if let Some(version) = version_suffix {
if let ComponentNameKind::Interface(iface) = implements.kind() {
if let Err(e) = iface.version(Some(version)) {
bail!(offset, "invalid interface version: {e}");
}
}
}
}
_ => bail!(offset, "name `{implements}` must be an interface"),
}
}

if let Some(_) = version_suffix {
require_feature::cm_canon_names(
*features,
"the `cm-canon-names` feature is not active",
offset,
)?;
match ty {
ComponentEntityType::Instance(_) => {}
_ => bail!(offset, "only instances can have an `versionsuffix`"),
}
}
Some(implements)
} else {
None
};

if let Some(_) = external_id {
require_feature::cm_implements(
Expand Down
72 changes: 62 additions & 10 deletions crates/wit-component/src/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,15 +609,39 @@ impl<'a> EncodingState<'a> {
let instance_type_idx = self
.component
.type_instance(Some(&format!("ty-{name}")), &ty);
let instance_idx = self.component.import(

let extern_name = if self.info.encoder.emit_canonical_names {
let name = resolve
.canonicalized_id_of(interface_id)
.unwrap_or_else(|| name.to_string());
let implements = info
.implements
.map(|id| resolve.canonicalized_id_of(id).unwrap());
let suffix_id = if let Some(id) = info.implements {
id
} else {
interface_id
};
wasm_encoder::ComponentExternName {
name: name.into(),
implements: implements.map(|s| s.into()),
external_id: info.external_id.as_deref().map(|s| s.into()),
version_suffix: resolve.version_suffix_of(suffix_id).map(|s| s.into()),
}
} else {
wasm_encoder::ComponentExternName {
name: name.into(),
implements: info.implements.as_deref().map(|s| s.into()),
implements: info
.implements
.as_ref()
.map(|s| resolve.id_of(*s).unwrap().into()),
external_id: info.external_id.as_deref().map(|s| s.into()),
version_suffix: None,
},
ComponentTypeRef::Instance(instance_type_idx),
);
}
};
let instance_idx = self
.component
.import(extern_name, ComponentTypeRef::Instance(instance_type_idx));
let prev = self.instances.insert(interface_id, instance_idx);
assert!(prev.is_none());
Ok(())
Expand Down Expand Up @@ -762,7 +786,11 @@ impl<'a> EncodingState<'a> {
let world = &resolve.worlds[self.info.encoder.metadata.world];

for export_name in exports {
let export_string = resolve.name_world_key(export_name);
let export_string = if self.info.encoder.emit_canonical_names {
resolve.name_canonicalized_world_key(export_name)
} else {
resolve.name_world_key(export_name)
};
match &world.exports[export_name] {
WorldItem::Function(func) => {
let ty = self
Expand Down Expand Up @@ -993,13 +1021,24 @@ impl<'a> EncodingState<'a> {
component_index,
imports,
);
let idx = self.component.export(
let implements = resolve.implements_interface(key, item);
let extern_name = if self.info.encoder.emit_canonical_names {
wasm_encoder::ComponentExternName {
name: export_name.into(),
implements: resolve.implements_value(key, item).map(|s| s.into()),
implements: implements.map(|id| resolve.canonicalized_id_of(id).unwrap().into()),
external_id: resolve.external_id_value(key, item).map(|s| s.into()),
version_suffix: resolve.version_suffix_value(key, item).map(|s| s.into()),
}
} else {
wasm_encoder::ComponentExternName {
name: export_name.into(),
implements: implements.map(|id| resolve.id_of(id).unwrap().into()),
external_id: resolve.external_id_value(key, item).map(|s| s.into()),
version_suffix: None,
},
}
};
let idx = self.component.export(
extern_name,
ComponentExportKind::Instance,
instance_index,
None,
Expand Down Expand Up @@ -3290,6 +3329,7 @@ pub struct ComponentEncoder {
pub(super) reject_legacy_names: bool,
debug_names: bool,
shim_return_call_ref: bool,
emit_canonical_names: bool,
}

impl ComponentEncoder {
Expand Down Expand Up @@ -3357,6 +3397,18 @@ impl ComponentEncoder {
self
}

/// Sets whether to emit canonical interface names in the component binary.
///
/// When enabled, import/export names use canonical version prefixes (e.g.,
/// `wasi:cli/exit@0.2` instead of `wasi:cli/exit@0.2.1`) and the
/// `version_suffix` field is populated.
///
/// This is disabled by default.
pub fn emit_canonical_names(&mut self, emit: bool) -> &mut Self {
self.emit_canonical_names = emit;
self
}

/// Sets whether to reject the historical mangling/name scheme for core wasm
/// imports/exports as they map to the component model.
///
Expand Down Expand Up @@ -3671,7 +3723,7 @@ world test {

let mut module = dummy_module(&resolve, world, ManglingAndAbi::Standard32);

embed_component_metadata(&mut module, &resolve, world, StringEncoding::UTF8).unwrap();
embed_component_metadata(&mut module, &resolve, world, StringEncoding::UTF8, true).unwrap();

let encoded = ComponentEncoder::default()
.import_name_map(HashMap::from([
Expand Down
76 changes: 57 additions & 19 deletions crates/wit-component/src/encoding/wit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use wit_parser::*;
///
/// The binary returned can be [`decode`d](crate::decode) to recover the WIT
/// package provided.
pub fn encode(resolve: &Resolve, package: PackageId) -> Result<Vec<u8>> {
let mut component = encode_component(resolve, package)?;
pub fn encode(resolve: &Resolve, package: PackageId, canonical_names: bool) -> Result<Vec<u8>> {
let mut component = encode_component(resolve, package, canonical_names)?;
component.raw_custom_section(&crate::base_producers().raw_custom_section());
Ok(component.finish())
}
Expand All @@ -48,11 +48,16 @@ pub fn encode(resolve: &Resolve, package: PackageId) -> Result<Vec<u8>> {
///
/// The binary returned can be [`decode`d](crate::decode) to recover the WIT
/// package provided.
pub fn encode_component(resolve: &Resolve, package: PackageId) -> Result<ComponentBuilder> {
pub fn encode_component(
resolve: &Resolve,
package: PackageId,
canonical_names: bool,
) -> Result<ComponentBuilder> {
let mut encoder = Encoder {
component: ComponentBuilder::default(),
resolve,
package,
canonical_names,
};
encoder.run()?;

Expand All @@ -66,7 +71,11 @@ pub fn encode_component(resolve: &Resolve, package: PackageId) -> Result<Compone
}

/// Encodes a `world` as a component type.
pub fn encode_world(resolve: &Resolve, world_id: WorldId) -> Result<ComponentType> {
pub fn encode_world(
resolve: &Resolve,
world_id: WorldId,
canonical_names: bool,
) -> Result<ComponentType> {
let mut component = InterfaceEncoder::new(resolve);
let world = &resolve.worlds[world_id];
log::trace!("encoding world {}", world.name);
Expand All @@ -93,9 +102,10 @@ pub fn encode_world(resolve: &Resolve, world_id: WorldId) -> Result<ComponentTyp
continue;
}
};
component
.outer
.import(component_extern_name(resolve, key, import), ty);
component.outer.import(
component_extern_name(resolve, key, import, canonical_names),
ty,
);
}
// Encode the exports
for (key, export) in world.exports.iter() {
Expand All @@ -113,9 +123,10 @@ pub fn encode_world(resolve: &Resolve, world_id: WorldId) -> Result<ComponentTyp
}
WorldItem::Type { .. } => unreachable!(),
};
component
.outer
.export(component_extern_name(resolve, key, export), ty);
component.outer.export(
component_extern_name(resolve, key, export, canonical_names),
ty,
);
}

Ok(component.outer)
Expand All @@ -125,19 +136,31 @@ fn component_extern_name(
resolve: &Resolve,
key: &WorldKey,
item: &WorldItem,
canonical_names: bool,
) -> wasm_encoder::ComponentExternName<'static> {
ComponentExternName {
name: resolve.name_world_key(key).into(),
implements: resolve.implements_value(key, item).map(|s| s.into()),
external_id: resolve.external_id_value(key, item).map(|s| s.into()),
version_suffix: None,
let implements = resolve.implements_interface(key, item);
if canonical_names {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In a previous review I was curiuos if it would be possible to deduplicate the number of places that a canonical-names option was taken into account and a ComponentExternName were created. I count currently four different locations doing very similar things:

  1. here
  2. below in this file in for interface in interfaces
  3. in encode_interface_import in encoding.rs
  4. in encode_interface_export in encoding.rs (split across two functions)

Were you able to take a look and see if these locations could be unified? Is there perhaps one, or maybe two at most, helpers that could be used to construct these names?

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.

Will think about it tomorrow.

ComponentExternName {
name: resolve.name_canonicalized_world_key(key).into(),
implements: implements.map(|id| resolve.canonicalized_id_of(id).unwrap().into()),
external_id: resolve.external_id_value(key, item).map(|s| s.into()),
version_suffix: resolve.version_suffix_value(key, item).map(|s| s.into()),
}
} else {
ComponentExternName {
name: resolve.name_world_key(key).into(),
implements: implements.map(|id| resolve.id_of(id).unwrap().into()),
external_id: resolve.external_id_value(key, item).map(|s| s.into()),
version_suffix: None,
}
}
}

struct Encoder<'a> {
component: ComponentBuilder,
resolve: &'a Resolve,
package: PackageId,
canonical_names: bool,
}

impl Encoder<'_> {
Expand All @@ -153,7 +176,7 @@ impl Encoder<'_> {
// For each `world` encode it directly as a component and then create a
// wrapper component that exports that component.
for (name, &world) in self.resolve.packages[self.package].worlds.iter() {
let component_ty = encode_world(self.resolve, world)?;
let component_ty = encode_world(self.resolve, world, self.canonical_names)?;

let world = &self.resolve.worlds[world];
let mut wrapper = ComponentType::new();
Expand Down Expand Up @@ -197,11 +220,24 @@ impl Encoder<'_> {
for interface in interfaces {
encoder.interface = Some(interface);
let iface = &self.resolve.interfaces[interface];
let name = self.resolve.id_of(interface).unwrap();
let extern_name = if self.canonical_names {
let name = self.resolve.canonicalized_id_of(interface).unwrap();
let version_suffix = self.resolve.version_suffix_of(interface);
ComponentExternName {
name: name.into(),
implements: None,
external_id: None,
version_suffix: version_suffix.map(|s| s.into()),
}
} else {
ComponentExternName::from(self.resolve.id_of(interface).unwrap())
};
if interface == id {
let idx = encoder.encode_instance(interface)?;
log::trace!("exporting self as {idx}");
encoder.outer.export(name, ComponentTypeRef::Instance(idx));
encoder
.outer
.export(extern_name, ComponentTypeRef::Instance(idx));
} else {
encoder.push_instance();
for (_, id) in iface.types.iter() {
Expand All @@ -212,7 +248,9 @@ impl Encoder<'_> {
encoder.outer.ty().instance(&instance);
encoder.import_map.insert(interface, encoder.instances);
encoder.instances += 1;
encoder.outer.import(name, ComponentTypeRef::Instance(idx));
encoder
.outer
.import(extern_name, ComponentTypeRef::Instance(idx));
}
}

Expand Down
6 changes: 3 additions & 3 deletions crates/wit-component/src/encoding/world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub struct ComponentWorld<'a> {
pub struct ImportedInterface {
pub lowerings: IndexMap<(String, AbiVariant), Lowering>,
pub interface: Option<InterfaceId>,
pub implements: Option<String>,
pub implements: Option<InterfaceId>,
pub external_id: Option<String>,
}

Expand Down Expand Up @@ -293,7 +293,7 @@ impl<'a> ComponentWorld<'a> {
WorldItem::Function(_) | WorldItem::Type { .. } => None,
WorldItem::Interface { id, .. } => Some(*id),
};
let implements = resolve.implements_value(key, item);
let implements = resolve.implements_interface(key, item);
// Note that `external_id` is only tracked for interface imports
// here. World-level functions and types all share the `None` entry
// in `import_map` but each item can have its own `external-id`
Expand All @@ -307,7 +307,7 @@ impl<'a> ComponentWorld<'a> {
.or_insert_with(|| ImportedInterface {
interface: interface_id,
lowerings: Default::default(),
implements: implements.clone(),
implements,
external_id: external_id.clone(),
});
assert_eq!(interface.interface, interface_id);
Expand Down
5 changes: 3 additions & 2 deletions crates/wit-component/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ pub fn embed_component_metadata(
wit_resolver: &Resolve,
world: WorldId,
encoding: StringEncoding,
canonical_names: bool,
) -> Result<()> {
let encoded = metadata::encode(&wit_resolver, world, encoding, None)?;
let encoded = metadata::encode(&wit_resolver, world, encoding, None, canonical_names)?;

let section = wasm_encoder::CustomSection {
name: "component-type".into(),
Expand Down Expand Up @@ -151,7 +152,7 @@ world test-world {}
let world = resolver.select_world(&[pkg], Some("test-world"))?;

// Embed component metadata
embed_component_metadata(&mut bytes, &resolver, world, StringEncoding::UTF8)?;
embed_component_metadata(&mut bytes, &resolver, world, StringEncoding::UTF8, true)?;

// Re-retrieve custom section count, and search for the component-type custom section along the way
let mut found_component_section = false;
Expand Down
3 changes: 2 additions & 1 deletion crates/wit-component/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,9 @@ pub fn encode(
world: WorldId,
string_encoding: StringEncoding,
extra_producers: Option<&Producers>,
canonical_names: bool,
) -> Result<Vec<u8>> {
let ty = crate::encoding::encode_world(resolve, world)?;
let ty = crate::encoding::encode_world(resolve, world, canonical_names)?;

let world = &resolve.worlds[world];
let mut outer_ty = ComponentType::new();
Expand Down
Loading
Loading