Skip to content

Commit

Permalink
Cumulative bug fixes for 4.35 (#3342)
Browse files Browse the repository at this point in the history
* Verify error with primitive patterns involving boxed types

* Fixes #3129
* Fixes #3181

* Pattern binding variable not recognized in poly conditional operator
expression

 * Fixes #3222

* [Records] Unhelpful warning "Empty block should be documented" for
records without components

* Fixes #3316

* [Tests][Sealed Types] Many sealed type tests are run at the same
compliance over and over

* Fixes #3346
  • Loading branch information
srikanth-sankaran authored Nov 26, 2024
1 parent a589ad2 commit 1cf69bb
Show file tree
Hide file tree
Showing 12 changed files with 334 additions and 661 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,9 @@ public TypeBinding resolveType(BlockScope scope) {
return null;
} else {
if (this.originalValueIfTrueType.kind() == Binding.POLY_TYPE)
this.originalValueIfTrueType = this.valueIfTrue.resolveType(scope);
this.originalValueIfTrueType = this.valueIfTrue.resolveTypeWithBindings(this.condition.bindingsWhenTrue(), scope);
if (this.originalValueIfFalseType.kind() == Binding.POLY_TYPE)
this.originalValueIfFalseType = this.valueIfFalse.resolveType(scope);
this.originalValueIfFalseType = this.valueIfFalse.resolveTypeWithBindings(this.condition.bindingsWhenFalse(), scope);

if (this.originalValueIfTrueType == null || !this.originalValueIfTrueType.isValidBinding())
return this.resolvedType = null;
Expand Down Expand Up @@ -814,8 +814,27 @@ public boolean isPolyExpression() throws UnsupportedOperationException {

@Override
public boolean isCompatibleWith(TypeBinding left, Scope scope) {
return isPolyExpression() ? this.valueIfTrue.isCompatibleWith(left, scope) && this.valueIfFalse.isCompatibleWith(left, scope) :
if (!isPolyExpression())
super.isCompatibleWith(left, scope);

scope.include(this.condition.bindingsWhenTrue());
try {
if (!this.valueIfTrue.isCompatibleWith(left, scope))
return false;
} finally {
scope.exclude(this.condition.bindingsWhenTrue());
}

scope.include(this.condition.bindingsWhenFalse());
try {
if (!this.valueIfFalse.isCompatibleWith(left, scope))
return false;
} finally {
scope.exclude(this.condition.bindingsWhenFalse());
}

return true;

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package org.eclipse.jdt.internal.compiler.ast;

import org.eclipse.jdt.internal.compiler.ast.Pattern.PrimitiveConversionRoute;
import org.eclipse.jdt.internal.compiler.codegen.BranchLabel;
import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
import org.eclipse.jdt.internal.compiler.lookup.BaseTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
Expand All @@ -25,7 +24,7 @@
*/
interface IGenerateTypeCheck {

default void generateTypeCheck(TypeBinding providedType, TypeReference expectedTypeRef, BlockScope scope, CodeStream codeStream, BranchLabel falseLabel, PrimitiveConversionRoute route) {
default void generateTypeCheck(TypeBinding providedType, TypeReference expectedTypeRef, BlockScope scope, CodeStream codeStream, PrimitiveConversionRoute route) {
switch (route) {
case IDENTITY_CONVERSION -> {
consumeProvidedValue(providedType, codeStream);
Expand All @@ -46,8 +45,7 @@ default void generateTypeCheck(TypeBinding providedType, TypeReference expectedT
}
case WIDENING_REFERENCE_AND_UNBOXING_COVERSION,
WIDENING_REFERENCE_AND_UNBOXING_COVERSION_AND_WIDENING_PRIMITIVE_CONVERSION -> {
codeStream.ifnull(falseLabel);
codeStream.iconst_1();
codeStream.instance_of(scope.getJavaLangObject());
setPatternIsTotalType();
}
case NARROWING_AND_UNBOXING_CONVERSION -> {
Expand All @@ -56,8 +54,7 @@ default void generateTypeCheck(TypeBinding providedType, TypeReference expectedT
}
case UNBOXING_CONVERSION,
UNBOXING_AND_WIDENING_PRIMITIVE_CONVERSION -> {
codeStream.ifnull(falseLabel);
codeStream.iconst_1();
codeStream.instance_of(scope.getJavaLangObject());
setPatternIsTotalType();
}
case NO_CONVERSION_ROUTE -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public void generateOptimizedBoolean(BlockScope currentScope, CodeStream codeStr
route = this.testContextRecord.route();
providedType = this.testContextRecord.right();
}
generateTypeCheck(providedType, this.type, currentScope, codeStream, internalFalseLabel, route);
generateTypeCheck(providedType, this.type, currentScope, codeStream, route);

if (this.pattern != null) {
codeStream.ifeq(internalFalseLabel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ original record instance as receiver - leaving the stack drained.
if (!p.isUnnamed())
codeStream.dup(componentType); // lastComponent ? named ? ([C, C] : [R, C, C]) : ([C] : [R, C])
if (p instanceof TypePattern) {
((TypePattern) p).generateTypeCheck(currentScope, codeStream, matchFailLabel);
((TypePattern) p).generateTypeCheck(currentScope, codeStream);
} else {
codeStream.instance_of(p.resolvedType); // lastComponent ? named ? ([C, boolean] : [R, C, boolean]) : ([boolean] : [R, boolean])
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1372,9 +1372,9 @@ public void resolve() {
}
}

if ((this.bits & ASTNode.UndocumentedEmptyBlock) != 0 && this.nRecordComponents == 0) {
this.scope.problemReporter().undocumentedEmptyBlock(this.bodyStart-1, this.bodyEnd);
}
if (!sourceType.isRecord() && (this.bits & ASTNode.UndocumentedEmptyBlock) != 0)
this.scope.problemReporter().undocumentedEmptyBlock(this.bodyStart - 1, this.bodyEnd);

boolean needSerialVersion =
this.scope.compilerOptions().getSeverity(CompilerOptions.MissingSerialVersion) != ProblemSeverities.Ignore
&& sourceType.isClass()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream, BranchL
}
}

public void generateTypeCheck(BlockScope scope, CodeStream codeStream, BranchLabel internalFalseLabel) {
generateTypeCheck(this.outerExpressionType, getType(), scope, codeStream, internalFalseLabel,
public void generateTypeCheck(BlockScope scope, CodeStream codeStream) {
generateTypeCheck(this.outerExpressionType, getType(), scope, codeStream,
Pattern.findPrimitiveConversionRoute(this.resolvedType, this.accessorMethod.returnType, scope));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1460,23 +1460,4 @@ public void reportClashingDeclarations(LocalVariableBinding [] left, LocalVariab
public boolean resolvingGuardExpression() {
return this.resolvingGuardExpression;
}

public void include(LocalVariableBinding[] bindings) {
// `this` is assumed to be populated with bindings.
if (bindings != null) {
for (LocalVariableBinding binding : bindings) {
binding.modifiers &= ~ExtraCompilerModifiers.AccOutOfFlowScope;
}
}
}

public void exclude(LocalVariableBinding[] bindings) {
// `this` is assumed to be populated with bindings.
if (bindings != null) {
for (LocalVariableBinding binding : bindings) {
binding.modifiers |= ExtraCompilerModifiers.AccOutOfFlowScope;
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5797,4 +5797,21 @@ public List<ClassScope> collectClassesBeingInitialized() {
return list;
}

public void include(LocalVariableBinding[] bindings) {
// `this` is assumed to be populated with bindings.
if (bindings != null) {
for (LocalVariableBinding binding : bindings) {
binding.modifiers &= ~ExtraCompilerModifiers.AccOutOfFlowScope;
}
}
}

public void exclude(LocalVariableBinding[] bindings) {
// `this` is assumed to be populated with bindings.
if (bindings != null) {
for (LocalVariableBinding binding : bindings) {
binding.modifiers |= ExtraCompilerModifiers.AccOutOfFlowScope;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import java.util.Map;
import junit.framework.Test;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;

public class InstanceofPrimaryPatternTest extends AbstractRegressionTest {

Expand All @@ -35,26 +34,7 @@ public static Test suite() {
public InstanceofPrimaryPatternTest(String testName){
super(testName);
}
// Enables the tests to run individually
protected Map<String, String> getCompilerOptions(boolean preview) {
Map<String, String> defaultOptions = super.getCompilerOptions();
if (this.complianceLevel >= ClassFileConstants.getLatestJDKLevel()
&& preview) {
defaultOptions.put(CompilerOptions.OPTION_EnablePreviews, CompilerOptions.ENABLED);
}
return defaultOptions;
}

protected Map<String, String> getCompilerOptions() {
return getCompilerOptions(true);
}

@Override
protected void runConformTest(String[] testFiles, String expectedOutput, Map<String, String> customOptions) {
if(!isJRE17Plus)
return;
runConformTest(testFiles, expectedOutput, customOptions, new String[] {"--enable-preview"}, JAVAC_OPTIONS);
}
protected void runNegativeTest(
String[] testFiles,
String expectedCompilerLog,
Expand All @@ -71,7 +51,6 @@ protected void runNegativeTest(
runner.runNegativeTest();
}
public void test001() {
Map<String, String> options = getCompilerOptions(true);
runConformTest(
new String[] {
"X.java",
Expand All @@ -86,8 +65,7 @@ public void test001() {
" }\n" +
"}\n",
},
"Hello World!",
options);
"Hello World!");
}
public void test002() {
String expectedDiagnostics = this.complianceLevel < ClassFileConstants.JDK20 ?
Expand Down Expand Up @@ -670,4 +648,89 @@ private void foo(String x) {
----------
""");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3222
// [Patterns][Ternary] Pattern binding variable not recognized in poly conditional operator expression
public void testIssue3222() {
runConformTest(
new String[] {
"X.java",
"""
public class X {
interface I {
int foo();
}
static void foo(I i) {
System.out.println(i.foo());
}
public static void main(String[] args) {
foo(args instanceof String [] argv ? () -> argv.length + 13 : () -> 42);
}
}
"""
},
"13");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3222
// [Patterns][Ternary] Pattern binding variable not recognized in poly conditional operator expression
public void testIssue3222_2() {
runConformTest(
new String[] {
"X.java",
"""
public class X {
interface I {
int foo();
}
static void foo(I i) {
System.out.println(i.foo());
}
public static void main(String[] args) {
foo(!(args instanceof String [] argv) ? () -> 42 : () -> argv.length + 13);
}
}
"""
},
"13");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3222
// [Patterns][Ternary] Pattern binding variable not recognized in poly conditional operator expression
public void testIssue3222_3() {
runConformTest(
new String[] {
"X.java",
"""
import java.util.function.Supplier;
public class X {
interface I {
int foo(Supplier<Integer> arg);
default int foo(Object arg) {
return 13;
}
}
public X() {
super();
}
public static void main(String[] argv) {
int i = ((I) (x) -> {
return x.get();
}).foo((argv instanceof String [] args ? () -> args.length + 42 : (Supplier<Integer>) null));
System.out.println(i);
}
}
"""
},
"42");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7281,6 +7281,56 @@ public static void main(String[] args) {
},
"int:30");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3181
// VerifyError during conversion between T extends Long and double
public void testGH3181() {
runConformTest(new String[] {
"X.java",
"""
record Record<T extends Long>(T t) {
}
public class X {
public static <T extends Long> double foo(Record<T> s) {
return switch (s) {
case Record(double s1) -> s1 + 1.0;
default -> 0;
};
}
public static void main(String[] args) {
System.out.println(foo(new Record<>(42L)));
}
}
"""
},
"43.0");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3129
// VerifyError with instanceof record patterns with conversion from double to Long
public void testGH3129() {
runConformTest(new String[] {
"X.java",
"""
record Record(Long i) {
}
public class X {
public static double convert(Record r) {
return r instanceof Record(double d) ? d + 1.0 : 0;
}
public static void main(String[] args) {
System.out.println(convert(new Record(42L)));
}
}
"""
},
"43.0");
}

public void _testSpec00X() {
runNegativeTest(new String[] {
"X.java",
Expand Down
Loading

0 comments on commit 1cf69bb

Please sign in to comment.