It would be useful to implement the well-known symbols, in order to do things like properly implement iterators (#576). ECMAScript specification.
This code should now work and give the expected result:
Symbol.iterator;
The expected output is Symbol(Symbol.iterator).
The simplest way of doing this would be to have the symbols initialised in the init function of Symbol. However, symbols use a hash (currently just a counter) in order to differentiate them, and generating a hash is currently a method on Interpreter, which is not passed to the init function. It was suggested that it might be possible to pass the interpreter to the init functions, but this might not be possible, as the init functions are called in the setup of Realm, in the create_intrinsics function, and a Realm is used to create an Interpreter so the Interpreter won't exist when the Symbol object is initialised.
One way to fix this would be to move the hash counter to Realm and pass the Realm to the init functions. This could potentially be quite disruptive, as currently the global object, which is a part of Realm, is passed to the init functions, and this would have to be replaced with the Realm in order to avoid borrowing the same thing twice, meaning every init function would have to be changed in order to accommodate this. However, I think this would be the cleanest solution in the long term.
Another less disruptive, but dirtier way of doing this would be to hardcode the hash values of the well-known symbols and set the hash counter in Interpreter to start at the next number. As there are a fixed number (for now) of well known symbols it should be possible to do this without the possibility of collisions. However, this is likely to lead to bugs if more well-known symbols are added or the hash is used for something else as well, and on the whole is not a clean way of solving this.
Hmmm. Here is another solution:
We move create_intrinsics to the Interpreter, and in Interpreter::new() we call it, like we do in Realm::create(), we could add a getter for the global object, and we can pass the interpreter instead of global object in the init functions
The interpreter is needed not only for symbol creation but we will need it for other things too in the future.
What do you think? @joshwd36
Side note, create_intrinsics was called by realm::create to match up with the spec https://tc39.es/ecma262/#sec-createintrinsics
Side note,
create_intrinsicswas called by realm::create to match up with the spec tc39.es/ecma262/#sec-createintrinsics
It shouldn't matter where we call it, it is only an implementation detail.
Hmmm. Here is another solution:
We move
create_intrinsicsto theInterpreter, and inInterpreter::new()we call it, like we do inRealm::create(), we could add a getter for theglobalobject, and we can pass the interpreter instead of global object in theinitfunctionsThe interpreter is needed not only for symbol creation but we will need it for other things too in the future.
What do you think? @joshwd36
That would work too. I guess something to consider with that is if possibly a user wanted to use the same realm in multiple interpreters, if that's even possible, but other than that I can't see a problem.
I guess something to consider with that is if possibly a user wanted to use the same realm in multiple interpreters, if that's even possible,
Currently the Realm is consumed by the Interpreter so I don`t think this is a concern, at least for now.
Currently the
Realmis consumed by theInterpreterso I don`t think this is a concern, at least for now.
That's fair. For the sake of following the spec it might be best to keep it in Realm::create, but I don't really have any preference. What situations are there where it would be useful to use the interpreter in future?
For the sake of following the spec it might be best to keep it in Realm::create
This is an implementation detail, it's does not matter where we initialize the builtin stuff, as long as they are initialized, It does not change expected behaiviour.
What situations are there where it would be useful to use the interpreter in future?
An example is .set_field() currently it uses the ToString trait to_string() which calls Display, this is not spec compliant, it should call Interpreters .to_string() #373
For the sake of following the spec it might be best to keep it in Realm::create
This is an implementation detail, it's does not matter where we initialize the builtin stuff, as long as they are initialized, It does not change expected behaiviour.
What situations are there where it would be useful to use the interpreter in future?
An example is
.set_field()currently it uses theToStringtraitto_string()which callsDisplay, this is not spec compliant, it should callInterpreters.to_string()#373
That's interesting, as it definitely seems interpreter is needed there. This might be the best way then.
I had a go at this to see whether anything came up and and it ended up working very well. Should I make a PR or wait for more discussion here?
I had a go at this to see whether anything came up and and it ended up working very well.
Awesome!
Should I make a PR or wait for more discussion here?
You can make a draft PR and we will be able to see if we need to change anything, or if we need more discussion, it's good to have a example of one of the solutions :)