Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent operand stack - ternary operator which LHS and/or RHS has auto boxing & cast #116

Closed
HeartSaVioR opened this issue Mar 10, 2020 · 4 comments

Comments

@HeartSaVioR
Copy link
Contributor

This is one of test failures I've seen in Spark with Janino 3.1.1.

Please add below tests into ExpressionEvaluatorTest and run to see the failures:

    @Test public void
    testTernaryWithAutoBoxingAndCastingInLHSAndRHS() throws Exception {
        new ScriptEvaluator().cook(
            ""
                + "class A {\n"
                + "    private Integer val;\n"
                + "    public A(Integer v) {\n"
                + "         val = v;\n"
                + "    }\n"
                + "    public boolean isNull() {\n"
                + "        return val == null;\n"
                + "    }\n"
                + "    public int getInt() {\n"
                + "        return val;\n"
                + "    }\n"
                + "}\n"
                + "A a = new A(3);\n"
                + "Object[] b = new Object[]{\n"
                // auto boxing & casting in LHS
                + "a.isNull() ? (Object) a.getInt(): null,\n"
                // auto boxing & casting in RHS
                + "!a.isNull() ? null: (Object) a.getInt(),\n"
                // simple casting
                + "(Object) \"hello\"};\n"
        );
    }

I checked with two different UTs ad it failed on both LHS and RHS - I've flatten these tests into one.

It seems that the expected operand is Object but (Object) a.getInt() adds Integer.valueOf() (which is correct and Javac also does that) which pushes Integer as the operand.

Below code works with javac -source 1.6 -target 1.6:

class A {
    private Integer val;
    public A(Integer v) {
         val = v;
    }
    public boolean isNull() {
        return val == null;
    }
    public int getInt() {
        return val;
    }
}

class M {
	public static void main(String[] args) {
		A a = new A(3);
                A b = new A(null);
		Object[] c = new Object[]{ a.isNull() ? null : (Object) a.getInt(), !b.isNull() ? (Object) b.getInt() : null, "hello" };
                System.out.println(c[0]);
                System.out.println(c[1]);
                System.out.println(c[2]);
	}
}
@HeartSaVioR HeartSaVioR changed the title Inconsistent operand stack - ternary operator which LHS or RHS has auto boxing & cast Inconsistent operand stack - ternary operator which LHS and/or RHS has auto boxing & cast Mar 10, 2020
HeartSaVioR added a commit to HeartSaVioR/janino that referenced this issue Mar 13, 2020
…xing conversion & widening reference coversion happens together
HeartSaVioR added a commit to HeartSaVioR/janino that referenced this issue Mar 13, 2020
…xing conversion & widening reference conversion happens together
@HeartSaVioR
Copy link
Contributor Author

Raised a PR #117 to fix this.

HeartSaVioR added a commit to HeartSaVioR/janino that referenced this issue Mar 13, 2020
…xing conversion & widening reference conversion happens together
@aunkrig
Copy link
Member

aunkrig commented Mar 16, 2020

Merged the UT as JlsTest.test_15_25__Conditional_operator__2(), because this test is relevant for both compiler factories (janino and jdk).

@aunkrig
Copy link
Member

aunkrig commented Mar 16, 2020

InternalCompilerException (CodeContext.java:975) reproduced.
Will now try and see if PR #117 fixes the problem.

@aunkrig
Copy link
Member

aunkrig commented Mar 16, 2020

Merged PR #117 which successfully fixes this issue.

@aunkrig aunkrig closed this as completed Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants