gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.51k stars 372 forks source link

Add compiler implementation, tests for switch expressions for Java 14 #9878

Closed niloc132 closed 1 week ago

niloc132 commented 8 months ago

See https://openjdk.org/jeps/361.

At first glance, this seems like a fairly easy construct to consider, but at a minimum it requires executing statements inside of expressions - this seems to either require decomposing the enclosing expression(s) into statements and executing a switch/case or if/else, or extracting statements to synthetic lambdas or methods (closing over appropriate other locals, etc). As a pathological example, consider this, a switch expression with only one case that executes unconditionally, with embedded statements, passed as an argument to a method call with other expressions that may have side effects:

someMethod(obj.affectState(), switch(0) {
  default -> {
    List<String> list = new ArrayList<>();
    list.add("a");
    list.addAll(obj.affectState());
    yield list;
  }
});

Rewriting away just the switch expression while staying in Java could look like

// All sibling expressions (but not children/parents as far as I can tell?) must
// be extracted to ensure they are executed in the correct order. These can/should
// be final.
final Collection<String> arg1 = obj.affectState();
// Provide another local for the switch expression's value, to be assigned in
// each case/default
final Collection<String> arg2;
switch(0) {// switch expression becomes switch statement, with child expressions
  default: {
    List<String> list = new ArrayList<>();
    list.add("a");
    list.addAll(obj.affectState());
    // yield <value> is rewritten to assignment and break
    arg2 = list;
    break;
  }
}
// Actual parent express resumes
someMethod(arg1, arg2);

Or, again as a synthetic method:

// notice that we close over `obj`, but not .affectState(), as we might not invoke it
someMethod(obj.affectState(), syntheticSwitchMethod(obj));

...

private static Collection<String> syntheticSwitchMethods(OtherObject obj) {
  switch(0) {// switch expression becomes switch statement, with child expressions
    default: {
      List<String> list = new ArrayList<>();
      list.add("a");
      list.addAll(obj.affectState());
      // yield <value> is rewritten to return <value>
      return list;
    }
  }
}

Supporting multiple expressions values in each case, and the -> label both appear to purely be syntactic sugar, and the JDT should automatically cover this for us.

niloc132 commented 8 months ago

The method approach likely does not make any sense as it would prevent case blocks from assigning values to local variables.

niloc132 commented 5 months ago

Moving the switch expression to an earlier statement (and creating temp vars) doesn't work if for expressions within a statement that must be executed conditionally. Examples:

String str = booleanExpr() ? "hello" : switch(0) {...};

In this case, the switch must not be executed if the boolean expression is false - taking the approach of rewriting the switch to an earlier statement would also require rewriting the ternary into an if/else, roughly:

var tmp;
if (booleanExpr()) {
  tmp = "hello"
} else {
  tmp = switch(0) {...};// and then decompose switch here
}
String str = tmp;

Another option could be that the ternary could be itself rewritten to switch expression... except boolean types aren't allowed to be be used in a switch.

For loops have the same issue:

for (int i = 0; switch(i) {...}; i++) {...}

(also applies if the switch expression is in the "increment" expression) Here we need to execute the switch expression every time through the loop, so making it an earlier statement wouldn't be valid. Instead, we could break down the for into a while:

int i = 0;//"initialization"
boolean tmp$termination = switch(0) {...};//termination expr #1
while (tmp$termination) {
  // loop statements
  // increment:
  i++;
  //termination expr #2:
  tmp$termination = switch(0) {...};
}

While and do-while also have this issue, but can be resolved the same way:

while(expression) {
  // statements
}

could be rewritten to

boolean tmp$expression = expression;// termination expr #1
while (tmp$expression) {
  //statements
  tmp$expression = expression;// termination expr #2
}

A do-while should be simpler, since we only need to evaluate the expression at the end of the loop, instead of at the beginning:

do {
  // statements
} while (expression);

rewritten to

boolean tmp$expression;
do {
  // statements
  tmp$expression = expression;
} while (tmp$expression);

I can't presently think of other cases of containing expressions/statements that would need to be rewritten like this... thoughts?

rluble commented 5 months ago

FYI, I prototyped the implementation of switch expressions in J2CL and the easier general solution is to create a "lambda" (in J2CL we have FunctionExpression that represents a lambda but with JS capture semantics, I don't recall in GWT but we should have something similar) . That "lambda" needs to have JS semantics, i.e. should be able to modify variables in the enclosing scope.

In kotlin, statements are expressions and I also prototyped a transformation along the lines in your comment. Completely doable and in fact we ended using a kotlin frontend pass that does exactly that. But it is very infections, you have to handle every construct that takes an expression and bubble it up making explicit the order of computation.

I think the first approach using lambdas is straightforward.

niloc132 commented 5 months ago

Short-circuiting operations like && and || will also need to be handled. These will need to behave like ternary expressions, and all be broken down into temp local variable statements with if/else statements to do the branching

foo(value(), bar() && switch(val) {...});

would need to become something like

var tmp1 = value();
var tmp2;
if (bar()) {
  var tmp3 = switch(val) {...};
  if (tmp3) {
    tmp2 = true;
  } else {
    tmp2 = false;
  }
} else {
  tmp2 = false;
}
foo(tmp1, tmp2);

and so on.

niloc132 commented 5 months ago

Hi @rluble, thanks for your thoughts - how did you handle the ability to assign to locals if you use a lambda? I considered that briefly, but it seems incomplete out of the gate:

int i = 0;
someMethod(i++, switch(0) {
  default -> {
    yield i++;
  }
}, i++);
rluble commented 5 months ago

Well they are not really lambdas in J2CL, or I should say that in the internal representation lambdas are allowed to change variables that are in scope, i.e. they have JavaScript semantics not Java semantics.

niloc132 commented 5 months ago

Got it - I am not sure that will fly with GWT (since GWT has an AST between the JDT and JS), but it might be worth a try:

This is why I picked the above, more difficult approach - let us avoid adding new JNode types, change visitors. In theory we could have a JSwitchExpression type to keep GwtAstBuilder from getting too much bigger, but then immediately normalize those switch/case expressions away into statements (and rewrite other sibling expressions, and wrap in conditionals as above) - but at least it keeps the rewriting local to "how do you translate this". Do you think I missed something here, a way that would allow something like the semantics you described, with minimal rewriting?