[Patterns][Ternary] Pattern binding variable not recognized in poly conditional operator expression

[srikanth-sankaran]

Found by code inspection and white box testing: This program compiles as it should and prints 13 when run with javac. Eclipse refuses to compile. Eclipse behavior is incorrect.

public class X {

    interface I  {
        int foo();
    }

    static void foo(I i) {
        System.out.println(i.foo());
    }

    public static void main(String[] args) {
        int i = args instanceof String [] argv ? argv.length : 0;
        foo(args instanceof String [] argv ? () -> argv.length + 13  : () -> 42);

    }

}

Problem arises from these lines in org.eclipse.jdt.internal.compiler.ast.ConditionalExpression.resolveType(BlockScope)

                        if (this.originalValueIfTrueType.kind() == Binding.POLY_TYPE)
                this.originalValueIfTrueType = this.valueIfTrue.resolveType(scope);
            if (this.originalValueIfFalseType.kind() == Binding.POLY_TYPE)
                this.originalValueIfFalseType = this.valueIfFalse.resolveType(scope);

Those calls to resolveType should actually be calls to resolveTypeWithBindings

[stephan-herrmann]

Those calls to resolveType should actually be calls to resolveTypeWithBindings

Actually I'm not really familiar with this option. Can you state a rule when exactly the latter should be used?

[srikanth-sankaran]

Those calls to resolveType should actually be calls to resolveTypeWithBindings

Actually I'm not really familiar with this option. Can you state a rule when exactly the latter should be used?

Compare the defective lines above with these lines correctly implementing resolution of the ternary's condition and the valueIfTrue and valueIfFalse arms. This is extracted from the if (this.constant != Constant.NotAConstant) { path of org.eclipse.jdt.internal.compiler.ast.ConditionalExpression.resolveType(BlockScope)

            TypeBinding conditionType = this.condition.resolveTypeExpecting(scope, TypeBinding.BOOLEAN);
            this.originalValueIfTrueType = this.valueIfTrue.resolveTypeWithBindings(this.condition.bindingsWhenTrue(), scope);
            this.originalValueIfFalseType = this.valueIfFalse.resolveTypeWithBindings(this.condition.bindingsWhenFalse(), scope);

Basically condition in the ternary may inject some pattern bindings variables into scope in one or the other arms. Those arms must be resolved with those pattern binding variables in scope.

org.eclipse.jdt.internal.compiler.ast.Expression.resolveTypeWithBindings(LocalVariableBinding[], BlockScope) takes a array of LVBs and resolves its receiver with those LVBs in scope. Right after those LVBs go out of scope.

If any pattern variable introduced by the condition is not thus included in scope, then we will have resolution errors.

(This is the result of refactored implementation during Jan-Mar of 2024)

[stephan-herrmann]

Basically condition in the ternary may inject some pattern bindings variables into scope in one or the other arms. Those arms must be resolved with those pattern binding variables in scope.

Is this only about ternaries? Or where exactly should this rule be applied?

[srikanth-sankaran]

(notice that one arm gets resolved with this.condition.bindingsWhenTrue() and the other with `this.condition.bindingsWhenFalse())

[srikanth-sankaran]

Basically condition in the ternary may inject some pattern bindings variables into scope in one or the other arms. Those arms must be resolved with those pattern binding variables in scope.

Is this only about ternaries? Or where exactly should this rule be applied?

No, any construct that could introduce a pattern variable - if (), while(), do { } while, for(), ... should be prepared accordingly.

So IfStatement should resolve its condition in its own scope, but should resolve its then {} in a potentially altered scope that carries additional bindings injected by the condition when true.

Look at all the callers of org.eclipse.jdt.internal.compiler.ast.Expression.resolveTypeExpectingWithBindings(LocalVariableBinding[], BlockScope, TypeBinding) org.eclipse.jdt.internal.compiler.ast.Expression.resolveTypeWithBindings(LocalVariableBinding[], BlockScope) org.eclipse.jdt.internal.compiler.ast.Statement.resolveWithBindings(LocalVariableBinding[], BlockScope) org.eclipse.jdt.internal.compiler.ast.LambdaExpression.resolveTypeWithBindings(LocalVariableBinding[], BlockScope, boolean)

and

also at:

org.eclipse.jdt.internal.compiler.ast.Statement.bindingsWhenTrue() org.eclipse.jdt.internal.compiler.ast.Statement.bindingsWhenFalse() org.eclipse.jdt.internal.compiler.ast.Statement.bindingsWhenComplete()

I gave a talk to the IBM JDT folks in March/April of 2024 when the Patterns 2.0 was merged into master. This is not captured in any wiki unfortunately.

[srikanth-sankaran]

if (x instanceof String s) { // bindings when true { s } when false {} // s is in scope } else { // s not in scope }

if (!(x instanceof String s)) { // bindings when true {} when false { s } // s not in scope } else { // s in scope }

that is what we are trying to achieve with those resolve*WithBindings() calls

[stephan-herrmann]

I knew about patterns in if, but then at first I searched only for callers of resolveTypeWithBindings(LocalVariableBinding[], BlockScope) which looked incomplete, so that's why I kept asking.

Inspecting the call hierarchy of BlockScope.include(LocalVariableBinding[]) looks much more complete. Thanks.

[stephan-herrmann]

This is not captured in any wiki unfortunately.

be my guest :)

[srikanth-sankaran]

This is not captured in any wiki unfortunately.

be my guest :)

2025! Grammar work and design documentation :) Cross my heart and hope to die :)

[stephan-herrmann]

No, any construct that could introduce a pattern variable - if (), while(), do { } while, for(), ... should be prepared accordingly.

Hihi, I was thinking so, only to find that DoStatement didn't join the party. Perhaps, because all following statements in scope is the empty set. But what about do { ... } while (!(foo instanceof Bar bar)); print(bar); ?? ;-P

[srikanth-sankaran]

do { ... } while (!(foo instanceof Bar bar)); print(bar);

This one compiles fine:

public class X {

    public static void main(String[] args) {
        X foo = new X(); 
        do { System.out.println(); 
        } while (!(foo instanceof X x)); 
        System.out.println(x);
    }

}

Can you a share a snippet with the problem if you think there is a defect ?

[srikanth-sankaran]

Take a look at org.eclipse.jdt.internal.compiler.ast.DoStatement.bindingsWhenComplete() and the call sites of org.eclipse.jdt.internal.compiler.ast.Statement.bindingsWhenComplete()

I don't think there is a defect here

[stephan-herrmann]

Can you a share a snippet with the problem if you think there is a defect ?

I search for understanding more urgently than for defects :)

In that vein I challenged your statement, which sounded like DoStatement should use resolveTypeWithBindings(), which it doesn't. With your last answer I could try to summarize:

• structures with an embedded condition (where an instanceof pattern could occur) should
  • use resolveType*WithBindings() for resolving nested AST following the condition, if any
  • implement bindingsWhenComplete() to inform subsequent statements

Are we on the same page now?

[srikanth-sankaran]

Can you a share a snippet with the problem if you think there is a defect ?

I search for understanding more urgently than for defects :)

I do the opposite, so between us both objectives are covered :)

In that vein I challenged your statement, which sounded like DoStatement should use resolveTypeWithBindings(), which it doesn't. With your last answer I could try to summarize:

• structures with an embedded condition (where an instanceof pattern could occur) should

  • use resolveType*WithBindings() for resolving nested AST following the condition, if any

I would generalize it as:

• structures which could inject pattern bindings conditionally into the scope of different/some regions of code, should arrange to have those regions of code resolved with those bindings included.

i.e not just instanceof pattern, a record pattern too: e.g., switch (...) { case Point(int x, int y) p -> { / x, y in scope / }

(but, but, that is nitpicking, instance of is implicit in switch+ case record pattern one can say)

• implement bindingsWhenComplete() to inform subsequent statements

Yes, (including if when one or the other arm doesn't complete normally`)

Are we on the same page now?

Yes. I would add one more point.

• implement bindingsWhenTrue() and bindingsWhenFalse() as needed.

Drilling down into which ASTNodes need to implement these three bindingsWhen* methods and who calls them is a great way to get a quick understanding of the refactored implementation.