646599 - Constant folding should happen before deciding whether to turn obj["A"] into obj.A

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

jsfunfuzz's round-trip checker caught this:

js> (function() { return t["a" + "b"]; })
(function () {return t["ab"];})
js> (function () {return t["ab"];})
(function () {return t.ab;})

Comment 1

[Jesse Ruderman]

Another instance:

js> (function() { return t[true ? 'a' : 'b']; })
(function () {return t["a"];})
js> (function () {return t["a"];})
(function () {return t.a;})

Comment 2

[Tom S [:evilpie]]

Attachment: v1

Just do the folding manually before we decide whether to use obj.prop or obj[prop].

Really short patch, I hope this is okay for you Jeff.

Comment 3

[Tom S [:evilpie]]

Attachment: v1 with tests

Sorry forgot to hg add the test.

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 642208 [details] [diff] [review]
v1 with tests

Review of attachment 642208 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/Parser.cpp
@@ +5833,5 @@
>                  }
>              } else if (propExpr->isKind(PNK_NUMBER)) {
> +                Value number = NumberValue(propExpr->pn_dval);
> +                uint32_t dummy;
> +                if (!IsDefinitelyIndex(number, &dummy)) {

Please use (propExpr->pn_dval == ToUint32(propExpr->pn_dval)) to check for index-ness -- much better than relying on a method that sometimes gives the wrong answer and must itself be double-checked.

::: js/src/jit-test/tests/basic/testFoldPropertyAccess.js
@@ +17,5 @@
> +    }
> +]
> +
> +for (var i = 0; i < cases.length; i++) {
> +    dis(cases[i]);

Remove this.

Comment 5

[Tom S [:evilpie]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/28711b9f49cd

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/28711b9f49cd