Implement Display for Objects #171

IovoslavIovchev · 2019-10-19T15:08:28Z

NB: Due to the fact that the main binary uses the dbg! macro, pretty printing, while present, does not the make output, well.. pretty (e.g. new lines are rendered as \n).

IovoslavIovchev · 2019-10-19T16:03:49Z

#170

jasonwilliams · 2019-10-19T16:30:55Z

This may be out of scope for your PR, but be weary of circular references.
Printing an object A which has a property B which has a property back to A I think causes an infinite loop.

Or at least it did when I was playing with it.
JS naturally has a lot of circular references, so you may want to think how you show properties

IovoslavIovchev · 2019-10-19T16:57:11Z

This implementation indeed does not take circular dependencies into consideration.

I believe I have an idea how that can be achieved, so I will take some more time to look into it and deliver everything in one PR.

IovoslavIovchev · 2019-10-21T15:27:48Z

@jasonwilliams could you take a look?

IovoslavIovchev · 2019-10-21T15:53:29Z

I decided to not filter out the prototype slot, as I still have not found a scenario which causes a stack overflow.
I propose we leave it as is, and if needed, remove it later.

jasonwilliams

Looks good

jasonwilliams · 2019-10-22T13:07:16Z

Do we want to somehow share this logic with console.log https://github.com/jasonwilliams/boa/blob/master/src/lib/js/console.rs ?

IovoslavIovchev · 2019-10-22T15:07:09Z

We definitely could do that, only we need to find a way to escape code duplication for printing properties.

jasonwilliams · 2019-10-22T17:20:23Z

@IovoslavIovchev Not sure if you're aware but the newlines aren't being parsed
This is the output i get

[src/bin/bin.rs:57] exec(&buffer) = "{\n\n    __proto__: {\n        valueOf: function() { [native code] },\n        constructor: {\n            prototype: \'[Cycle]\'\n            call: function() { [native code] },\n            construct: function() { [native code] }\n        },\n        toString: function() { [native code] }\n        BooleanData: false,\n        extensible: true,\n        __proto__: undefined\n    },\n    BooleanData: false\n}"

IovoslavIovchev · 2019-10-22T17:33:11Z

@jasonwilliams As I have mentioned in the PR description, this is because of the dbg! macro. It does not render new lines or tabs, etc. but rather \n, \t, etc.

If you used println! rather than dgb! on ln 57 in src/bin/bin.rs, you would see the output beautified. Hope that helps.

jasonwilliams · 2019-10-22T17:51:54Z

@IovoslavIovchev oh my bad.
Should we change bin.rs to print!() instead of dbg!() then?

jasonwilliams · 2019-10-22T17:55:32Z

{

    BooleanData: false,
    __proto__: {
        constructor: {
            prototype: '[Cycle]'
            call: function() { [native code] },    
            construct: function() { [native code] }
        },
        valueOf: function() { [native code] },     
        toString: function() { [native code] }     
        BooleanData: false,
        extensible: true,
        __proto__: undefined
    }
}

Looks great 😁

Do we want to make it clear the difference between properties and slots?
Console.log should definitely not be printing slot data, so i suppose the use cases are different there.

IovoslavIovchev · 2019-10-22T17:57:49Z

Well, that really depends. In my opinion we should not, as bin.rs is used for debugging purposes anyway, and it is better to see that the output is what you actually expect.

The downside obviously is that when it wraps (the way it does in your case) it gets really hard to read.

IovoslavIovchev · 2019-10-22T17:59:49Z

Additionally, one slight mistake I made was I joined the properties and slots before printing and that resulted in

        valueOf: function() { [native code] },     
        toString: function() { [native code] }     
        BooleanData: false,

as you can see there is no comma after toString.

However, I have fixed it now. You can review the latest change.

jasonwilliams · 2019-10-22T18:01:19Z

Sure, you also have a conflict

jasonwilliams · 2019-10-29T10:41:27Z

No worries, was just curious

…inting

…boa into feature/object-printing

jasonwilliams · 2019-11-12T22:27:14Z

You need any help with this?

IovoslavIovchev · 2019-11-13T06:03:59Z

I hit a wall with prototypes. When the execution reaches length (for instance when printing Array.prototype) I get an Err and the program panics.

IovoslavIovchev · 2019-11-16T16:36:55Z

src/lib/builtins/array.rs

+        // let mut engine = Executor::new(realm);
+        // let init = r#"
+        // var empty = new Array();
+        // var one = new Array(1);


Currently, new Array(1) produces [1] (an array with one element, which is 1), when, in fact, it should produce an array with length 1 and an empty slot.

We will pick this up in jasonwilliams#209

IovoslavIovchev · 2019-11-16T17:04:43Z

@jasonwilliams could you take a look again?

jasonwilliams · 2019-11-17T16:16:20Z

src/lib/builtins/array.rs

    let callback = &args[0];
    let this_arg = if args.len() > 1 {
        args[1].clone()
    } else {


Looks like Array.prototype.length shouldn't have a getter according to the spec

jasonwilliams · 2019-11-17T16:20:29Z

src/lib/builtins/string.rs

@@ -741,7 +741,9 @@ pub fn create_constructor(global: &Value) -> Value {

    // Create prototype
    let proto = ValueData::new_obj(Some(global));
-    let prop = Property::default().get(to_value(get_string_length as NativeFunctionData));
+    let prop = Property::default()
+        .get(to_value(get_string_length as NativeFunctionData))


String.prototype.length does not have a getter according to the spec https://tc39.es/ecma262/#sec-properties-of-the-string-prototype-object

IovoslavIovchev added 6 commits October 19, 2019 16:39

Added the initial re-implementation of Display for Objects

0076f08

Added internal object slots printing

8ce28e6

Used INSTANCE_PROTOTYPE instead of "__proto__"

72dca2f

Updated TODO comment

f20fb6c

rustfmt

85024eb

Fixed clippy and unit tests

5788f6e

IovoslavIovchev mentioned this pull request Oct 19, 2019

There are no quotes around a String value #172

Closed

IovoslavIovchev changed the title ~~WIP: Started implementing Display for Objects~~ Implement Display for Objects Oct 19, 2019

IovoslavIovchev added 6 commits October 20, 2019 14:39

WIP: Added a working prototype for detecting cycles

995ab5c

Refactored the object printing logic

5ecadf1

Reverted test.js

fa73b84

Fixed print identation

70f7d0a

rustfmt

394cc64

Added printing for the internal_slots

05a2748

jasonwilliams approved these changes Oct 22, 2019

View reviewed changes

Fixed missing comma between internal slots & props when printing

b9698e4

IovoslavIovchev changed the title ~~Implement Display for Objects~~ [WIP] Implement Display for Objects Oct 29, 2019

IovoslavIovchev mentioned this pull request Oct 31, 2019

Create Print() function #142

Closed

IovoslavIovchev added 8 commits November 1, 2019 19:39

Merge branch 'master' into feature/object-printing

0a076c1

Merge remote-tracking branch 'upstream/master' into feature/object-pr…

4c2814f

…inting

Implemented a macro for printing objects & refactored

b26433c

Merge remote-tracking branch 'upstream/master' into feature/object-pr…

390e265

…inting

Used display_obj in console.log

1bf4ccd

rustfmt

80af637

Merge remote-tracking branch 'upstream/master' into feature/object-pr…

16a9610

…inting

Merge branch 'feature/object-printing' of github.com:IovoslavIovchev/…

4bb4624

…boa into feature/object-printing

IovoslavIovchev added 5 commits November 16, 2019 10:09

Fixed String & Array prototypes length not being set

d13e1ad

Extracted common logic in a function

fda92c7

Commented out problematic lines in concat

877522e

rustfmt

bca21e1

Refactored console.rs & value.rs

9f7be25

IovoslavIovchev commented Nov 16, 2019

View reviewed changes

IovoslavIovchev added 2 commits November 16, 2019 18:58

Fixed log_string_from looking for wrong string slot

0a2196b

Reverted commented test

f999365

IovoslavIovchev changed the title ~~[WIP] Implement Display for Objects~~ Implement Display for Objects Nov 17, 2019

jasonwilliams reviewed Nov 17, 2019

View reviewed changes

IovoslavIovchev added 3 commits November 17, 2019 18:35

Removed Array & String prototype lengths' getters

c77057c

Removed unused functions

6ce305c

rustfmt

870e727

IovoslavIovchev mentioned this pull request Nov 21, 2019

Display for objects #211

Merged

IovoslavIovchev closed this Nov 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Display for Objects #171

Implement Display for Objects #171

IovoslavIovchev commented Oct 19, 2019 •

edited

Loading

IovoslavIovchev commented Oct 19, 2019

jasonwilliams commented Oct 19, 2019 •

edited

Loading

IovoslavIovchev commented Oct 19, 2019

IovoslavIovchev commented Oct 21, 2019

IovoslavIovchev commented Oct 21, 2019

jasonwilliams left a comment

jasonwilliams commented Oct 22, 2019

IovoslavIovchev commented Oct 22, 2019

jasonwilliams commented Oct 22, 2019 •

edited

Loading

IovoslavIovchev commented Oct 22, 2019

jasonwilliams commented Oct 22, 2019 •

edited

Loading

jasonwilliams commented Oct 22, 2019 •

edited

Loading

IovoslavIovchev commented Oct 22, 2019

IovoslavIovchev commented Oct 22, 2019

jasonwilliams commented Oct 22, 2019

jasonwilliams commented Oct 29, 2019

jasonwilliams commented Nov 12, 2019

IovoslavIovchev commented Nov 13, 2019 •

edited

Loading

IovoslavIovchev Nov 16, 2019 •

edited

Loading

jasonwilliams Nov 17, 2019

IovoslavIovchev commented Nov 16, 2019

jasonwilliams Nov 17, 2019

jasonwilliams Nov 17, 2019

Implement Display for Objects #171

Implement Display for Objects #171

Conversation

IovoslavIovchev commented Oct 19, 2019 • edited Loading

IovoslavIovchev commented Oct 19, 2019

jasonwilliams commented Oct 19, 2019 • edited Loading

IovoslavIovchev commented Oct 19, 2019

IovoslavIovchev commented Oct 21, 2019

IovoslavIovchev commented Oct 21, 2019

jasonwilliams left a comment

Choose a reason for hiding this comment

jasonwilliams commented Oct 22, 2019

IovoslavIovchev commented Oct 22, 2019

jasonwilliams commented Oct 22, 2019 • edited Loading

IovoslavIovchev commented Oct 22, 2019

jasonwilliams commented Oct 22, 2019 • edited Loading

jasonwilliams commented Oct 22, 2019 • edited Loading

IovoslavIovchev commented Oct 22, 2019

IovoslavIovchev commented Oct 22, 2019

jasonwilliams commented Oct 22, 2019

jasonwilliams commented Oct 29, 2019

jasonwilliams commented Nov 12, 2019

IovoslavIovchev commented Nov 13, 2019 • edited Loading

IovoslavIovchev Nov 16, 2019 • edited Loading

Choose a reason for hiding this comment

jasonwilliams Nov 17, 2019

Choose a reason for hiding this comment

IovoslavIovchev commented Nov 16, 2019

jasonwilliams Nov 17, 2019

Choose a reason for hiding this comment

jasonwilliams Nov 17, 2019

Choose a reason for hiding this comment

IovoslavIovchev commented Oct 19, 2019 •

edited

Loading

jasonwilliams commented Oct 19, 2019 •

edited

Loading

jasonwilliams commented Oct 22, 2019 •

edited

Loading

jasonwilliams commented Oct 22, 2019 •

edited

Loading

jasonwilliams commented Oct 22, 2019 •

edited

Loading

IovoslavIovchev commented Nov 13, 2019 •

edited

Loading

IovoslavIovchev Nov 16, 2019 •

edited

Loading