Skip to content

Commit

Permalink
fix: resource deduping (#190)
Browse files Browse the repository at this point in the history
* fix: resource deduping

* fixup
  • Loading branch information
guybedford committed Oct 13, 2023
1 parent b85f6bc commit 6de98be
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 5 deletions.
17 changes: 13 additions & 4 deletions crates/js-component-bindgen/src/function_bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub enum ErrHandling {
pub struct ResourceTable {
pub id: u32,
pub imported: bool,
pub local_name: String,
}
pub type ResourceMap = BTreeMap<TypeId, ResourceTable>;

Expand Down Expand Up @@ -1118,7 +1119,11 @@ impl Bindgen for FunctionBindgen<'_> {

Instruction::HandleLift { handle, name, .. } => {
let (Handle::Own(ty) | Handle::Borrow(ty)) = handle;
let ResourceTable { id, imported } = self.resource_map[ty];
let ResourceTable {
id,
imported,
local_name,
} = &self.resource_map[ty];
let is_own = matches!(handle, Handle::Own(_));
let rsc = format!("rsc{}", self.tmp());

Expand All @@ -1128,7 +1133,7 @@ impl Bindgen for FunctionBindgen<'_> {
let resource_symbol = self.intrinsic(Intrinsic::ResourceSymbol);
uwrite!(
self.src,
"const {rsc} = new.target === {class_name} ? this : Object.create({class_name}.prototype);
"const {rsc} = new.target === {local_name} ? this : Object.create({local_name}.prototype);
Object.defineProperty({rsc}, {resource_symbol}, {{ writable: true, value: handleTable{id}.get({}).rep }});
",
operands[0],
Expand Down Expand Up @@ -1168,7 +1173,11 @@ impl Bindgen for FunctionBindgen<'_> {
Instruction::HandleLower { handle, name, .. } => {
let (Handle::Own(ty) | Handle::Borrow(ty)) = handle;
let is_own = matches!(handle, Handle::Own(_));
let ResourceTable { id, imported } = self.resource_map[ty];
let ResourceTable {
id,
imported,
local_name,
} = &self.resource_map[ty];
let class_name = name.to_upper_camel_case();
let handle = format!("handle{}", self.tmp());
if !imported {
Expand Down Expand Up @@ -1204,7 +1213,7 @@ impl Bindgen for FunctionBindgen<'_> {
// their assigned rep is deduped across usage though
uwriteln!(
self.src,
"if (!({} instanceof {class_name})) {{
"if (!({} instanceof {local_name})) {{
throw new Error('Not a valid \"{class_name}\" resource.');
}}
const {handle} = handleCnt{id}++;
Expand Down
15 changes: 14 additions & 1 deletion crates/js-component-bindgen/src/transpile_bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,10 +807,23 @@ impl<'a> Instantiator<'a, '_> {
.defined_resource_index(self.types[*t2].ty)
.is_none();
self.ensure_resource_table(*t2);

let ty = &self.resolve.types[*t1];
let resource = ty.name.as_ref().unwrap();
let (local_name, _) = self.gen.local_names.get_or_create(
&if imported {
format!("import_resource:{resource}")
} else {
format!("resource:{resource}")
},
&resource.to_upper_camel_case(),
);

map.insert(
*t1,
ResourceTable {
id: t2.as_u32(),
local_name: local_name.to_string(),
imported,
},
);
Expand Down Expand Up @@ -1175,7 +1188,7 @@ impl<'a> Instantiator<'a, '_> {
let ty = &self.resolve.types[ty];
let resource = ty.name.as_ref().unwrap();
self.gen.local_names.get_or_create(
&format!("resource:{export_name}:{resource}"),
&format!("resource:{resource}"),
&resource.to_upper_camel_case(),
)
} else {
Expand Down
19 changes: 19 additions & 0 deletions test/codegen.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { readFile } from 'node:fs/promises';
import { exec, jcoPath } from './helpers.js';
import { strictEqual } from 'node:assert';
import { componentNew, componentEmbed, transpile } from '@bytecodealliance/jco';
import { ok } from 'node:assert';

const eslintPath = `node_modules/eslint/bin/eslint.js`;

Expand Down Expand Up @@ -39,4 +41,21 @@ export async function codegenTest (fixtures) {
});
}
});

suite(`Naming`, () => {
test(`Resource deduping`, async () => {
const component = await componentNew(await componentEmbed({
witSource: await readFile(`test/fixtures/wits/resource-naming/resource-naming.wit`, 'utf8'),
dummy: true,
metadata: [['language', [['javascript', '']]], ['processed-by', [['dummy-gen', 'test']]]]
}));

const { files } = await transpile(component, { name: 'resource-naming' });

const bindingsSource = new TextDecoder().decode(files['resource-naming.js']);

ok(bindingsSource.includes('const Thing$1 = class Thing'));
ok(bindingsSource.includes('Thing: Thing$1'));
});
});
}
17 changes: 17 additions & 0 deletions test/fixtures/wits/resource-naming.wit
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package test:resource-naming

interface resource-import-and-export {
resource thing {
constructor(v: u32)

foo: func() -> u32
bar: func(v: u32)

baz: static func(a: thing, b: thing) -> thing
}
}

world resource-naming {
import resource-import-and-export
export resource-import-and-export
}
17 changes: 17 additions & 0 deletions test/fixtures/wits/resource-naming/resource-naming.wit
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package test:test

interface resource-import-and-export {
resource thing {
constructor(v: u32)

foo: func() -> u32
bar: func(v: u32)

baz: static func(a: thing, b: thing) -> thing
}
}

world test {
import resource-import-and-export
export resource-import-and-export
}

0 comments on commit 6de98be

Please sign in to comment.