Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local function/class name shadows imported host item of same name #338

Closed
jcbhmr opened this issue Jan 15, 2024 · 1 comment · Fixed by #340
Closed

Local function/class name shadows imported host item of same name #338

jcbhmr opened this issue Jan 15, 2024 · 1 comment · Fixed by #340

Comments

@jcbhmr
Copy link

jcbhmr commented Jan 15, 2024

Specific example:

package jcbhmr:hello-world-rust-wasm-component-lib;

interface cbh {
  resource p-string {
    call: func(a: string);
  }
  resource r-string {
    call: func() -> string;
  }
}
interface cb {
  use cbh.{p-string as h-p-string, r-string as h-r-string};
  resource p-string {
    constructor(cb: h-p-string);
    call: func(a: string);
  }
  resource r-string {
    constructor(cb: h-r-string);
    call: func() -> string;
  }
}

interface hello-world {
  use cb.{p-string, r-string};
  set-cb: func(cb: p-string);
  run-cb-with-result-of: func(cb: borrow<r-string>);
}

world hello-world-rust-wasm-component-lib {
  import cbh;
  export cb;

  export hello-world;
}
import { PString, RString } from '../../../tests/cbh.js';

// ...

const PString$1 = class PString{
  constructor(arg0) {
    console.error(`[module="jcbhmr:hello-world-rust-wasm-component-lib/cb", function="[constructor]p-string"] call cb=${arguments[0]}`);
    if (!(arg0 instanceof PString)) {
      throw new Error('Resource error: Not a valid "HPString" resource.');
    }
    var handle0 = handleCnt0++;
    handleTable0.set(handle0, { rep: arg0, own: true });
    const ret = exports1['jcbhmr:hello-world-rust-wasm-component-lib/cb#[constructor]p-string'](handle0);
    console.error(`[module="jcbhmr:hello-world-rust-wasm-component-lib/cb", function="[constructor]p-string"] return result=${toResultString(ret)}`);
    var handle2 = ret;
    var rsc1 = new.target === PString$1 ? this : Object.create(PString$1.prototype);
    var rep3 = handleTable2.get(handle2).rep;
    Object.defineProperty(rsc1, resourceHandleSymbol, { writable: true, value: rep3});
    finalizationRegistry2.register(rsc1, handle2, rsc1);
    Object.defineProperty(rsc1, symbolDispose, { writable: true, value: function () {} });

    handleTable2.delete(handle2);
    return rsc1;
  }
}

When inside the class PString { } for the exported guest PString resource type it checks that the arg0 is instanceof PString. That's SUPPOSED to check that arg0 is an instance of the host's PString class. Instead due to shadowing, it's referring to itself (the class PString {} declaration that it's inside of). This is incorrect behaviour.

Instead, the code should look like something like this:

import { PString, RString } from '../../../tests/cbh.js';

// ...

const PString$1 = class {
  static {
    Object.defineProperty(this, 'name', { value: 'PString', configurable: true })
  }
  constructor(arg0) {
    console.error(`[module="jcbhmr:hello-world-rust-wasm-component-lib/cb", function="[constructor]p-string"] call cb=${arguments[0]}`);
    if (!(arg0 instanceof PString)) {
      throw new Error('Resource error: Not a valid "HPString" resource.');
    }
    var handle0 = handleCnt0++;
    handleTable0.set(handle0, { rep: arg0, own: true });
    const ret = exports1['jcbhmr:hello-world-rust-wasm-component-lib/cb#[constructor]p-string'](handle0);
    console.error(`[module="jcbhmr:hello-world-rust-wasm-component-lib/cb", function="[constructor]p-string"] return result=${toResultString(ret)}`);
    var handle2 = ret;
    var rsc1 = new.target === PString$1 ? this : Object.create(PString$1.prototype);
    var rep3 = handleTable2.get(handle2).rep;
    Object.defineProperty(rsc1, resourceHandleSymbol, { writable: true, value: rep3});
    finalizationRegistry2.register(rsc1, handle2, rsc1);
    Object.defineProperty(rsc1, symbolDispose, { writable: true, value: function () {} });

    handleTable2.delete(handle2);
    return rsc1;
  }
}

OR like this

import { PString as $host_PString, RString as $host_RString } from '../../../tests/cbh.js';

// ...

const PString$1 = class PString{
  constructor(arg0) {
    console.error(`[module="jcbhmr:hello-world-rust-wasm-component-lib/cb", function="[constructor]p-string"] call cb=${arguments[0]}`);
    if (!(arg0 instanceof $host_PString)) {
      throw new Error('Resource error: Not a valid "HPString" resource.');
    }
    var handle0 = handleCnt0++;
    handleTable0.set(handle0, { rep: arg0, own: true });
    const ret = exports1['jcbhmr:hello-world-rust-wasm-component-lib/cb#[constructor]p-string'](handle0);
    console.error(`[module="jcbhmr:hello-world-rust-wasm-component-lib/cb", function="[constructor]p-string"] return result=${toResultString(ret)}`);
    var handle2 = ret;
    var rsc1 = new.target === PString$1 ? this : Object.create(PString$1.prototype);
    var rep3 = handleTable2.get(handle2).rep;
    Object.defineProperty(rsc1, resourceHandleSymbol, { writable: true, value: rep3});
    finalizationRegistry2.register(rsc1, handle2, rsc1);
    Object.defineProperty(rsc1, symbolDispose, { writable: true, value: function () {} });

    handleTable2.delete(handle2);
    return rsc1;
  }
}

Notice how now the instanceoof PString refers to the OUTER host-defined PString class.

Suggestion:
Rename the class using Object.defineProperty() either in static {} block or after defining it.
OR mangle the names of imports like { PString as $host_PString } to avoid having them be shadowed by local export declarations.

@jcbhmr jcbhmr changed the title Local exported MyResource shadows imported host MyResource which causes instanceof MyResource to always fail Local function/class name shadows imported host item of same name Jan 15, 2024
@guybedford
Copy link
Collaborator

Thanks for the detailed report on this. I've gone with the class PString$1 { ... } approach here instead of const PString$1 = class PString{, with the fix in #340.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants