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

Why was the Display implementation for Object removed? #170

Closed
IovoslavIovchev opened this issue Oct 19, 2019 · 9 comments
Closed

Why was the Display implementation for Object removed? #170

IovoslavIovchev opened this issue Oct 19, 2019 · 9 comments
Assignees

Comments

@IovoslavIovchev
Copy link
Contributor

Hello,

While playing around with this project I came accross:

#[test]
fn concat() {
    //TODO: array display formatter

and, after some digging, I found that at some point in the past Display was implemented for ValueData::Object (as far as I know, arrays in boa are objects).

So why exactly was it removed? Was it because the previous implementation was too verbose?

@jasonwilliams
Copy link
Member

Hey @IovoslavIovchev I can answer this one.
Basically we were experiencing stack overflows when printing objects.

After investigation I saw that the printer prints every property and slot for an object.
This creates an infinite loop because the prototype hidden slot of String will point to Object so all of Objects properties get printed and so on.

I think for now I changed object to just print {} (I tried reducing it to just properties but I can’t remember why that didn’t work)

Anyway, there is some work to be done need to match whatever the other implementations do

@jasonwilliams
Copy link
Member

The output was getting too big and verbose. If there is a nice way to pretty print objects I’m for it.

@jasonwilliams
Copy link
Member

If you wish to experiment you can try adding it back, then try printing a string object

@IovoslavIovchev
Copy link
Contributor Author

I tried various forms of Strings, Objects and Arrays but none of them caused a stack overflow (I'm using 2427b3b, by the way).

Did a stack overflow occur every time, or just in particular cases?

@IovoslavIovchev
Copy link
Contributor Author

One way to avoid verbosity (and a stack overflow) would be to only print own properties and not print the prototype, if that is something that might be deemed acceptable.

Additionally, pretty printing can be achieved by simply printing each new property on a new line.

@jasonwilliams
Copy link
Member

jasonwilliams commented Oct 19, 2019

Sure, it’s a good start

I think all objects should attempt to call their own .toString() counterpart before being displayed anyway.

In terms of the overflow, I don’t remember how to reproduce the issue, but it only happened when slots were exposed.

@IovoslavIovchev
Copy link
Contributor Author

I will take a look at re-implementing Display then.

@jasonwilliams
Copy link
Member

Thanks @IovoslavIovchev

@IovoslavIovchev
Copy link
Contributor Author

Done in #211

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