Skip to content

Commit

Permalink
Enhanced switch v2.0 - Part I (#3310)
Browse files Browse the repository at this point in the history
* Reimplement resolution, analysis and bookkeeping involved with `CaseStatement` and resolution of `SwitchStatement` for a streamlined and cleaner implementation resulting in readily obvious control flow and minimization of state.
* Report non-constant label expressions during resolution rather than waiting for flow analysis phase.
* Change how duplicate label expressions are reported. Don't blame the whole case, just the duplicate expression; Don't blame the original, just the duplicate.
* Represent qualified enumerators with qualified names internally so to avoid name clashes
* Eliminate the hack to inject synthetic `BreakStatement` in label rule switch blocks. Enhance the code generator to handle these explicitly.
* Simplify the implementation of SwitchStatement.getFallThroughState correcting various issues.
* Eliminate the unnecessary vector `SwitchStatement.constMapping` that is passed to `org.eclipse.jdt.internal.compiler.codegen.CodeStream.tableswitch` - constanMapping[i] == i always!
* Enable various tests in `PrimitiveInPatternsTest.java` that were disabled, fixing broken tests; Remove their disabled cousins from `PrimitiveInPatternsTestSH.java`
* Add a disabled regression test for #3319 (not yet understood/fixed)
* Numerous defect fixes as listed below

* Fixes #3336
* Fixes #3318
* Fixes #3320
* Fixes #3321
* Fixes #3334
* Fixes #3335
* Fixes #3265
* Fixes #3169
* Fixes #3339
* Fixes #3337
* Fixes #3344
  • Loading branch information
srikanth-sankaran authored Nov 26, 2024
1 parent 2fe0af3 commit a589ad2
Show file tree
Hide file tree
Showing 31 changed files with 1,236 additions and 1,391 deletions.
20 changes: 0 additions & 20 deletions org.eclipse.jdt.core.compiler.batch/.settings/.api_filters
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,4 @@
</message_arguments>
</filter>
</resource>
<resource path="src/org/eclipse/jdt/core/compiler/IProblem.java" type="org.eclipse.jdt.core.compiler.IProblem">
<filter comment="renamed constant for JEP 482" id="405864542">
<message_arguments>
<message_argument value="org.eclipse.jdt.core.compiler.IProblem"/>
<message_argument value="DisallowedStatementInPrologue"/>
</message_arguments>
</filter>
<filter comment="this preview constant (marked @noreference) is unnecesary" id="405864542">
<message_arguments>
<message_argument value="org.eclipse.jdt.core.compiler.IProblem"/>
<message_argument value="DuplicateTotalPattern"/>
</message_arguments>
</filter>
<filter comment="renamed constant for JEP 482" id="405864542">
<message_arguments>
<message_argument value="org.eclipse.jdt.core.compiler.IProblem"/>
<message_argument value="ExpressionInPreConstructorContext"/>
</message_arguments>
</filter>
</resource>
</component>
2 changes: 1 addition & 1 deletion org.eclipse.jdt.core.compiler.batch/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Main-Class: org.eclipse.jdt.internal.compiler.batch.Main
Bundle-ManifestVersion: 2
Bundle-Name: Eclipse Compiler for Java(TM)
Bundle-SymbolicName: org.eclipse.jdt.core.compiler.batch
Bundle-Version: 3.40.0.qualifier
Bundle-Version: 3.40.100.qualifier
Bundle-ClassPath: .
Bundle-Vendor: Eclipse.org
Automatic-Module-Name: org.eclipse.jdt.core.compiler.batch
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.jdt.core.compiler.batch/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<version>4.35.0-SNAPSHOT</version>
</parent>
<artifactId>org.eclipse.jdt.core.compiler.batch</artifactId>
<version>3.40.0-SNAPSHOT</version>
<version>3.40.100-SNAPSHOT</version>
<packaging>eclipse-plugin</packaging>

<properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.internal.compiler.ast.*;
import org.eclipse.jdt.internal.compiler.ast.CaseStatement.ResolvedCase;
import org.eclipse.jdt.internal.compiler.ast.CaseStatement.LabelExpression;
import org.eclipse.jdt.internal.compiler.ast.SwitchStatement.SingletonBootstrap;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.codegen.*;
Expand Down Expand Up @@ -3613,8 +3613,8 @@ private int generateBootstrapMethods(List<Object> bootStrapMethodsList) {
}
} else if (o instanceof String) {
localContentsOffset = addBootStrapStringConcatEntry(localContentsOffset, (String) o, fPtr);
} else if (o instanceof ResolvedCase) {
localContentsOffset = addBootStrapTypeCaseConstantEntry(localContentsOffset, (ResolvedCase) o, fPtr);
} else if (o instanceof LabelExpression) {
localContentsOffset = addBootStrapTypeCaseConstantEntry(localContentsOffset, (LabelExpression) o, fPtr);
} else if (o instanceof TypeBinding) {
localContentsOffset = addClassDescBootstrap(localContentsOffset, (TypeBinding) o, fPtr);
} else if (o instanceof SingletonBootstrap sb) {
Expand Down Expand Up @@ -3807,7 +3807,7 @@ private int addBootStrapRecordEntry(int localContentsOffset, TypeDeclaration typ
}
return localContentsOffset;
}
private int addBootStrapTypeCaseConstantEntry(int localContentsOffset, ResolvedCase caseConstant, Map<String, Integer> fPtr) {
private int addBootStrapTypeCaseConstantEntry(int localContentsOffset, LabelExpression caseConstant, Map<String, Integer> fPtr) {
final int contentsEntries = 10;
if (contentsEntries + localContentsOffset >= this.contents.length) {
resizeContents(contentsEntries);
Expand Down Expand Up @@ -3852,7 +3852,8 @@ private int addBootStrapTypeCaseConstantEntry(int localContentsOffset, ResolvedC
this.contents[localContentsOffset++] = (byte) (idx >> 8);
this.contents[localContentsOffset++] = (byte) idx;

idx = this.constantPool.literalIndex(caseConstant.c.stringValue());
String enumerator = caseConstant.expression instanceof QualifiedNameReference qnr ? new String(qnr.tokens[qnr.tokens.length - 1]) : caseConstant.expression.toString();
idx = this.constantPool.literalIndex(enumerator);
this.contents[localContentsOffset++] = (byte) (idx >> 8);
this.contents[localContentsOffset++] = (byte) idx;

Expand Down Expand Up @@ -3930,7 +3931,7 @@ private int addSingletonBootstrap(int localContentsOffset, SingletonBootstrap sb
}

private int addBootStrapTypeSwitchEntry(int localContentsOffset, SwitchStatement switchStatement, Map<String, Integer> fPtr) {
CaseStatement.ResolvedCase[] constants = switchStatement.otherConstants;
CaseStatement.LabelExpression[] constants = switchStatement.labelExpressions;
int numArgs = constants.length;
final int contentsEntries = 10 + (numArgs * 2);
int indexFortypeSwitch = fPtr.get(ClassFile.TYPESWITCH_STRING);
Expand All @@ -3952,16 +3953,16 @@ private int addBootStrapTypeSwitchEntry(int localContentsOffset, SwitchStatement
this.contents[numArgsLocation++] = (byte) (numArgs >> 8);
this.contents[numArgsLocation] = (byte) numArgs;
localContentsOffset += 2;
for (CaseStatement.ResolvedCase c : constants) {
for (CaseStatement.LabelExpression c : constants) {
if (c.isPattern()) {
int typeOrDynIndex;
if (c.e.resolvedType.isPrimitiveType()) {
if (c.expression.resolvedType.isPrimitiveType()) {
// Dynamic for Class.getPrimitiveClass(Z) or such
typeOrDynIndex = this.constantPool.literalIndexForDynamic(c.primitivesBootstrapIdx,
c.t.signature(),
c.type.signature(),
ConstantPool.JavaLangClassSignature);
} else {
char[] typeName = c.t.constantPoolName();
char[] typeName = c.type.constantPoolName();
typeOrDynIndex = this.constantPool.literalIndexForType(typeName);
}
this.contents[localContentsOffset++] = (byte) (typeOrDynIndex >> 8);
Expand All @@ -3972,26 +3973,26 @@ private int addBootStrapTypeSwitchEntry(int localContentsOffset, SwitchStatement
ConstantPool.JAVA_LANG_ENUM_ENUMDESC);
this.contents[localContentsOffset++] = (byte) (typeIndex >> 8);
this.contents[localContentsOffset++] = (byte) typeIndex;
} else if ((c.e instanceof StringLiteral)||(c.c instanceof StringConstant)) {
} else if ((c.expression instanceof StringLiteral)||(c.constant instanceof StringConstant)) {
int intValIdx =
this.constantPool.literalIndex(c.c.stringValue());
this.constantPool.literalIndex(c.constant.stringValue());
this.contents[localContentsOffset++] = (byte) (intValIdx >> 8);
this.contents[localContentsOffset++] = (byte) intValIdx;
} else {
if (c.e instanceof NullLiteral) continue;
int valIdx = switch (c.t.id) {
if (c.expression instanceof NullLiteral) continue;
int valIdx = switch (c.type.id) {
case TypeIds.T_boolean -> // Dynamic for Boolean.getStaticFinal(TRUE|FALSE) :
this.constantPool.literalIndexForDynamic(c.primitivesBootstrapIdx,
c.c.booleanValue() ? BooleanConstant.TRUE_STRING : BooleanConstant.FALSE_STRING,
c.constant.booleanValue() ? BooleanConstant.TRUE_STRING : BooleanConstant.FALSE_STRING,
ConstantPool.JavaLangBooleanSignature);
case TypeIds.T_byte, TypeIds.T_char, TypeIds.T_short, TypeIds.T_int ->
this.constantPool.literalIndex(c.intValue());
case TypeIds.T_long ->
this.constantPool.literalIndex(c.c.longValue());
this.constantPool.literalIndex(c.constant.longValue());
case TypeIds.T_float ->
this.constantPool.literalIndex(c.c.floatValue());
this.constantPool.literalIndex(c.constant.floatValue());
case TypeIds.T_double ->
this.constantPool.literalIndex(c.c.doubleValue());
this.constantPool.literalIndex(c.constant.doubleValue());
default ->
throw new IllegalArgumentException("Switch has unexpected type: "+switchStatement); //$NON-NLS-1$
};
Expand Down Expand Up @@ -4019,25 +4020,23 @@ private int addBootStrapEnumSwitchEntry(int localContentsOffset, SwitchStatement

// u2 num_bootstrap_arguments
int numArgsLocation = localContentsOffset;
CaseStatement.ResolvedCase[] constants = switchStatement.otherConstants;
CaseStatement.LabelExpression[] constants = switchStatement.labelExpressions;
int numArgs = constants.length;
if (switchStatement.containsNull) --numArgs;
this.contents[numArgsLocation++] = (byte) (numArgs >> 8);
this.contents[numArgsLocation] = (byte) numArgs;
localContentsOffset += 2;

for (CaseStatement.ResolvedCase c : constants) {
for (CaseStatement.LabelExpression c : constants) {
if (c.isPattern()) {
char[] typeName = switchStatement.expression.resolvedType.constantPoolName();
int typeIndex = this.constantPool.literalIndexForType(typeName);
this.contents[localContentsOffset++] = (byte) (typeIndex >> 8);
this.contents[localContentsOffset++] = (byte) typeIndex;
} else {
if (c.e instanceof NullLiteral) continue;
String s = c.e instanceof QualifiedNameReference qnr ? // handle superfluously qualified enumerator.
new String(qnr.tokens[qnr.tokens.length-1]) : c.e.toString();
int intValIdx =
this.constantPool.literalIndex(s);
if (c.expression instanceof NullLiteral) continue;
String enumerator = c.expression instanceof QualifiedNameReference qnr ? new String(qnr.tokens[qnr.tokens.length - 1]) : c.expression.toString();
int intValIdx = this.constantPool.literalIndex(enumerator);
this.contents[localContentsOffset++] = (byte) (intValIdx >> 8);
this.contents[localContentsOffset++] = (byte) intValIdx;
}
Expand Down Expand Up @@ -6370,7 +6369,7 @@ public int recordBootstrapMethod(SwitchStatement switchStatement) {
this.bootstrapMethods.add(switchStatement);
return this.bootstrapMethods.size() - 1;
}
public int recordBootstrapMethod(ResolvedCase resolvedCase) {
public int recordBootstrapMethod(LabelExpression resolvedCase) {
if (this.bootstrapMethods == null) {
this.bootstrapMethods = new ArrayList<>();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

public class BreakStatement extends BranchStatement {

public boolean isSynthetic;
public BreakStatement(char[] label, int sourceStart, int e) {
super(label, sourceStart, e);
}
Expand Down Expand Up @@ -115,10 +114,4 @@ public boolean doesNotCompleteNormally() {
public boolean canCompleteNormally() {
return false;
}


@Override
protected boolean doNotReportUnreachable() {
return this.isSynthetic;
}
}
Loading

0 comments on commit a589ad2

Please sign in to comment.