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

Symbol.keyFor() as in @agoric/marshal gives undefined #258

Closed
dckc opened this issue Aug 31, 2019 · 3 comments
Closed

Symbol.keyFor() as in @agoric/marshal gives undefined #258

dckc opened this issue Aug 31, 2019 · 3 comments

Comments

@dckc
Copy link
Contributor

dckc commented Aug 31, 2019

When I run this with mcconfig -d -m -p lin, I get same? false actual type: undefined:

export default function main() {
  var canonical = Symbol.for('s');

  const actual = Symbol.keyFor(canonical);
  const same = actual === 's';
  trace(`same? ${same} actual type: ${typeof actual}\n`);
}

It's very odd, because it's based on the built-ins/Symbol/keyFor/arg-symbol-registry-hit.js test262 test, which passes:

~/projects/test262/test$ ~/projects/moddable/build/bin/lin/debug/xst built-ins/Symbol/keyFor/arg-symbol-registry-hit.js 
### built-ins/Symbol/keyFor/arg-symbol-registry-hit.js (sloppy): OK
### built-ins/Symbol/keyFor/arg-symbol-registry-hit.js (strict): OK
# 0:00:00
100% 2/2

context: I'm struggling with the Symbol.for('sym1') test case in test-marshal.js https://github.com/Agoric/SwingSet/blob/master/test/test-marshal.js#L33-L36

@phoddie
Copy link
Collaborator

phoddie commented Aug 31, 2019

Good find. This only happens for symbols created from strings stored internally as XS_STRING_X_KIND -- which is, more or less, strings that are known at build time.

The fix is straightforward. In xsSymbol.c change lines 153 & 154 as follows:

	if (key && ((key->kind == XS_KEY_KIND) || (key->kind == XS_KEY_X_KIND)) && ((key->flag & XS_DONT_ENUM_FLAG) == 0)) {
		mxResult->kind = (key->kind == XS_KEY_KIND) ? XS_STRING_KIND : XS_STRING_X_KIND;

FWIW, a quick review of the other functions in xsSymbol.c suggests that they handle this case correctly..

We'll push the fix to the repository in the coming days.

@phoddie
Copy link
Collaborator

phoddie commented Sep 2, 2019

The fix has been pushed.

@dckc
Copy link
Contributor Author

dckc commented Sep 2, 2019

confirmed: fixed in 002cbb7

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

No branches or pull requests

2 participants