474310 - Incorrect optimization of unary increment/decrement operators applied to number function parameteres

Status: RESOLVED FIXED

(Rhino Graveyard :: Compiler, defect)

Description

[Sergei Babovich]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.5) Gecko/2008121622 Fedora/3.0.5-1.fc9 Firefox/3.0.5
Build Identifier: rhino1_7R1

Consider the following java script function:

function f1(i : Number) : String
{
 return ++i;
}

function f2(i : Number) : String
{
  f1(i);
}
 
The result of 
f2("5") will return 1 - NOT 6 !!! 

Here is java code for f1() generated by compiler:

    private static Object _c1(c1 c1_1, Context context, Scriptable scriptable, Scriptable scriptable1, Object i, double d, Object aobj[])
    {
        scriptable = c1_1.getParentScope();
    //    0    0:aload_0         
    //    1    1:invokeinterface #201 <Method Scriptable Scriptable.getParentScope()>
    //    2    6:astore_2        
        return OptRuntime.wrapDouble(++d);
    //    3    7:dload           5
    //    4    9:dconst_1        
    //    5   10:dadd            
    //    6   11:dup2            
    //    7   12:dstore          5
    //    8   14:invokestatic    #205 <Method Double OptRuntime.wrapDouble(double)>
    //    9   17:goto            20
    //   10   20:areturn         
    }


As you can see optimized code ignores Object input parameter. org.mozilla.javascript.optimizer.Codegen.visitIncDec() needs to be fixed.

If replace ++i with i+1 correct java code is generated: 
private static Object _c1(c1 c1_1, Context context, Scriptable scriptable, Scriptable scriptable1, Object i, double d, Object aobj[])
    {
        if(i == Void.TYPE)
    //*   0    0:aload           4
    //*   1    2:getstatic       #201 <Field Class Void.TYPE>
    //*   2    5:if_acmpne       15
            i = OptRuntime.wrapDouble(d);
    //    3    8:dload           5
    //    4   10:invokestatic    #205 <Method Double OptRuntime.wrapDouble(double)>
    //    5   13:astore          4
        scriptable = c1_1.getParentScope();
    //    6   15:aload_0         
    //    7   16:invokeinterface #211 <Method Scriptable Scriptable.getParentScope()>
    //    8   21:astore_2        
        return OptRuntime.add(i, 1.0D);
    //    9   22:aload           4
    //   10   24:dconst_1        
    //   11   25:invokestatic    #215 <Method Object OptRuntime.add(Object, double)>
    //   12   28:goto            31
    //   13   31:areturn         
    }

 
  


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Norris Boyd]

Base Rhino doesn't implement type annotations. Also, f2 doesn't have a return statement. Changing these I get correct behavior:

function f1(i )
{
 return ++i;
}

function f2(i )
{
 return f1(i);
}
print(f2("5"));

prints "6".

Comment 2

[Sergei Babovich]

Looks like bug appears with optimization levels > 0. Check below (test.js contains your test):

[.. classes]$ java org.mozilla.javascript.tools.jsc.Main -opt 0 test.js
[.. classes]$ java test
6
[.. classes]$ java org.mozilla.javascript.tools.jsc.Main -opt 1 test.js
[.. classes]$ java test
1

Comment 3

[Norris Boyd]

Confirmed

Comment 4

[Norris Boyd]

Attachment: Proposed patch

Comment 5

[Norris Boyd]

Committed change to CVS HEAD and 1.7R2 branch.