Fix optimization of function calls to not leak this.

The optimizer was propagating an isFn bit improperly when folding expressions
which caused (0, a.b)() to simplify to (a.b()) which leaks (a.b) as the "this"
value for the call.

There was another problem in folding which caused an infinite loop on the test
cases added to JsOptimizerTest.  That was fixed in Operation.fold.

Submitted at r5713

[MarkM, 2015-02-26 18:07:54]

codereview.appspot is not showing me any differences.

[Mike Samuel, 2015-02-26 18:08:08]

The optimizer was propagating an isFn bit improperly when folding expressions
which caused (0, a.b)() to simplify to (a.b()) which leaks (a.b) as the "this"
value for the call.

There was another problem in folding which caused an infinite loop on the test
cases added to JsOptimizerTest.  That was fixed in Operation.fold.

[Mike Samuel, 2015-02-26 18:10:27]

The optimizer was propagating an isFn bit improperly when folding expressions
which caused (0, a.b)() to simplify to (a.b()) which leaks (a.b) as the "this"
value for the call.

There was another problem in folding which caused an infinite loop on the test
cases added to JsOptimizerTest.  That was fixed in Operation.fold.

[Mike Samuel, 2015-02-26 18:12:16]

The optimizer was propagating an isFn bit improperly when folding expressions
which caused (0, a.b)() to simplify to (a.b()) which leaks (a.b) as the "this"
value for the call.

There was another problem in folding which caused an infinite loop on the test
cases added to JsOptimizerTest.  That was fixed in Operation.fold.

[Mike Samuel, 2015-02-26 18:32:36]

Yeah.  I had diff-cmd=colordiff in my .subversion/config and our myvn
script doesn't know to ignore it.

On Thu, Feb 26, 2015 at 1:07 PM,  <erights@gmail.com> wrote:
> codereview.appspot is not showing me any differences.
>
>
> https://codereview.appspot.com/202360043/

[kpreid_google, 2015-02-26 18:32:38]

LGTM

https://codereview.appspot.com/202360043/diff/2/tests/com/google/caja/ancilla...
File tests/com/google/caja/ancillary/opt/JsOptimizerTest.java (right):

https://codereview.appspot.com/202360043/diff/2/tests/com/google/caja/ancilla...
tests/com/google/caja/ancillary/opt/JsOptimizerTest.java:87: public final void
testThisNotUnintentionallyPassed1() throws Exception {
This test isn't quite testing what its name suggests, but I agree the name
parallelism is relevant. How about a comment like "/** Test that (0,...) is not
inserted when it is not necessary */"?

[Mike Samuel, 2015-02-26 20:28:47]

The optimizer was propagating an isFn bit improperly when folding expressions
which caused (0, a.b)() to simplify to (a.b()) which leaks (a.b) as the "this"
value for the call.

There was another problem in folding which caused an infinite loop on the test
cases added to JsOptimizerTest.  That was fixed in Operation.fold.

[Mike Samuel, 2015-02-26 20:29:28]

https://codereview.appspot.com/202360043/diff/2/tests/com/google/caja/ancilla...
File tests/com/google/caja/ancillary/opt/JsOptimizerTest.java (right):

https://codereview.appspot.com/202360043/diff/2/tests/com/google/caja/ancilla...
tests/com/google/caja/ancillary/opt/JsOptimizerTest.java:87: public final void
testThisNotUnintentionallyPassed1() throws Exception {
On 2015/02/26 18:32:38, kpreid_google wrote:
> This test isn't quite testing what its name suggests, but I agree the name
> parallelism is relevant. How about a comment like "/** Test that (0,...) is
not
> inserted when it is not necessary */"?

Done.

[kpreid_google, 2015-02-26 20:36:38]

LGTM