Discussion:
RFR 8200215: 17th loop of "let foo = ''"; throws ReferenceError
Sundararajan Athijegannathan
2018-03-26 12:00:30 UTC
Permalink
Please review http://cr.openjdk.java.net/~sundar/8200215/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8200215

Thanks,
-Sundar
Jim Laskey
2018-03-26 12:10:07 UTC
Permalink
+1
Post by Sundararajan Athijegannathan
Please review http://cr.openjdk.java.net/~sundar/8200215/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8200215
Thanks,
-Sundar
Attila Szegedi
2018-03-26 12:18:30 UTC
Permalink
+1, looks good. Presumably this happened on reversal to megamorphic so setObject was linked and it didn’t properly handle this case… the crux of the matter, if I understand it correctly is that once the property is set, the NEEDS_DECLARATION flag needs to be removed, right?

I presume the fast-linked (non-megamorphic) paths handle this correctly already, otherwise we would’ve seen the bug on first iteration, not on 17th :-)

Attila.
Post by Sundararajan Athijegannathan
Please review http://cr.openjdk.java.net/~sundar/8200215/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8200215
Thanks,
-Sundar
Sundararajan Athijegannathan
2018-03-26 12:27:37 UTC
Permalink
Hi Attila,

Yes, new Map has to be created with a property with modified flags. And
yes, 17 was the clue :)

Thanks,
-Sundar
Post by Attila Szegedi
+1, looks good. Presumably this happened on reversal to megamorphic so setObject was linked and it didn’t properly handle this case… the crux of the matter, if I understand it correctly is that once the property is set, the NEEDS_DECLARATION flag needs to be removed, right?
I presume the fast-linked (non-megamorphic) paths handle this correctly already, otherwise we would’ve seen the bug on first iteration, not on 17th :-)
Attila.
Post by Sundararajan Athijegannathan
Please review http://cr.openjdk.java.net/~sundar/8200215/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8200215
Thanks,
-Sundar
Hannes Wallnöfer
2018-03-26 12:33:22 UTC
Permalink
The new ScriptObject.declareAndSet method is almost identical, so the existing one could be rewritten to make use of the new one.

Otherwise looks good.

Hannes
Post by Sundararajan Athijegannathan
Please review http://cr.openjdk.java.net/~sundar/8200215/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8200215
Thanks,
-Sundar
Sundararajan Athijegannathan
2018-03-26 13:27:16 UTC
Permalink
Updated: http://cr.openjdk.java.net/~sundar/8200215/webrev.01/

Please review.

-Sundar
Post by Hannes Wallnöfer
The new ScriptObject.declareAndSet method is almost identical, so the existing one could be rewritten to make use of the new one.
Otherwise looks good.
Hannes
Post by Sundararajan Athijegannathan
Please review http://cr.openjdk.java.net/~sundar/8200215/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8200215
Thanks,
-Sundar
Hannes Wallnöfer
2018-03-26 13:30:21 UTC
Permalink
+1

Hannes
Post by Sundararajan Athijegannathan
Updated: http://cr.openjdk.java.net/~sundar/8200215/webrev.01/
Please review.
-Sundar
Post by Hannes Wallnöfer
The new ScriptObject.declareAndSet method is almost identical, so the existing one could be rewritten to make use of the new one.
Otherwise looks good.
Hannes
Post by Sundararajan Athijegannathan
Please review http://cr.openjdk.java.net/~sundar/8200215/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8200215
Thanks,
-Sundar
Attila Szegedi
2018-03-26 14:35:50 UTC
Permalink
+1
Post by Sundararajan Athijegannathan
Updated: http://cr.openjdk.java.net/~sundar/8200215/webrev.01/
Please review.
-Sundar
Post by Hannes Wallnöfer
The new ScriptObject.declareAndSet method is almost identical, so the existing one could be rewritten to make use of the new one.
Otherwise looks good.
Hannes
Post by Sundararajan Athijegannathan
Please review http://cr.openjdk.java.net/~sundar/8200215/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8200215
Thanks,
-Sundar
Loading...