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.

Sign up for GitHub

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

8340125: [lw5] null checks are not being generated for method invocations and array writes #1251

Closed
wants to merge 9 commits into from
+312 −52
Closed

8340125: [lw5] null checks are not being generated for method invocations and array writes #1251

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
75e2a37
8340125: [lw5] null checks are not being generated for method invocat…
vicente-romero-oracle Sep 13, 2024
36d8b6f
8340125: [lw5] null checks are not being generated for method invocat…
vicente-romero-oracle Sep 13, 2024
98f741e
Merge with remote branch
vicente-romero-oracle Sep 25, 2024
aaf95a0
addressing review comments
vicente-romero-oracle Sep 25, 2024
abb05f8
experimental code review
vicente-romero-oracle Sep 27, 2024
b9173d8
Merge branch 'lw5' into JDK-8340125
vicente-romero-oracle Sep 30, 2024
6390b49
refactoring
vicente-romero-oracle Oct 3, 2024
a23c363
additional refactoring
vicente-romero-oracle Oct 3, 2024
334ec9b
refactoring
vicente-romero-oracle Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 46 additions & 24 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java
View file
Original file line number Diff line number Diff line change
@@ -173,6 +173,13 @@ JCExpression retype(JCExpression tree, Type erasedType, Type target) {
return tree;
}

JCExpression generateNullCheckIfNeeded(JCExpression tree) {
if (expectedNullness == JCNullableTypeExpression.NullMarker.NOT_NULL && !types.isNonNullable(tree.type)) {
return attr.makeNullCheck(tree);
}
return tree;
}

/** Translate method argument list, casting each argument
* to its corresponding type in a list of target types.
* @param _args The method argument list.
@@ -203,12 +210,11 @@ <T extends JCTree> List<T> translateArgs(List<T> _args,
if (erasedParameterTypes.isEmpty()) return _args;
List<T> args = _args;
while (erasedParameterTypes.tail.nonEmpty()) {
if (!unerasedParamTypes.isEmpty() && unerasedParamTypes.head != null) {
if (types.isNonNullable(unerasedParamTypes.head) && !types.isNonNullable(args.head.type)) {
args.head = (T)attr.makeNullCheck((JCExpression)args.head);
}
}
args.head = translate(args.head, erasedParameterTypes.head);
args.head = translate(args.head, erasedParameterTypes.head,
(!unerasedParamTypes.isEmpty() && unerasedParamTypes.head != null) ?
unerasedParamTypes.head.getNullMarker() :
JCNullableTypeExpression.NullMarker.UNSPECIFIED
);
args = args.tail;
erasedParameterTypes = erasedParameterTypes.tail;
if (!unerasedParamTypes.isEmpty()) {
@@ -219,19 +225,15 @@ <T extends JCTree> List<T> translateArgs(List<T> _args,
Assert.check(varargsElement != null || args.length() == 1);
if (varargsElement != null) {
while (args.nonEmpty()) {
if (types.isNonNullable(originalVarargsElement) && !types.isNonNullable(args.head.type)) {
args.head = (T)attr.makeNullCheck((JCExpression)args.head);
}
args.head = translate(args.head, varargsElement);
args.head = translate(args.head, varargsElement, originalVarargsElement.getNullMarker());
args = args.tail;
}
} else {
if (!unerasedParamTypes.isEmpty() && unerasedParamTypes.head != null) {
if (types.isNonNullable(unerasedParamTypes.head) && !types.isNonNullable(args.head.type)) {
args.head = (T)attr.makeNullCheck((JCExpression)args.head);
}
}
args.head = translate(args.head, parameter);
args.head = translate(args.head, parameter,
(!unerasedParamTypes.isEmpty() && unerasedParamTypes.head != null) ?
unerasedParamTypes.head.getNullMarker() :
JCNullableTypeExpression.NullMarker.UNSPECIFIED
);
}
return _args;
}
@@ -467,28 +469,44 @@ void addBridges(DiagnosticPosition pos, ClassSymbol origin, ListBuffer<JCTree> b
*/
private Type pt;

private JCNullableTypeExpression.NullMarker expectedNullness = JCNullableTypeExpression.NullMarker.UNSPECIFIED;

/** Visitor method: perform a type translation on tree.
*/
public <T extends JCTree> T translate(T tree, Type pt) {
return translate(tree, pt, JCNullableTypeExpression.NullMarker.UNSPECIFIED);
}

public <T extends JCTree> T translate(T tree, Type pt, JCNullableTypeExpression.NullMarker expectedNullness) {
Type prevPt = this.pt;
JCNullableTypeExpression.NullMarker prevExpectedNullness = this.expectedNullness;
try {
this.pt = pt;
this.expectedNullness = expectedNullness;
return translate(tree);
} finally {
this.pt = prevPt;
this.expectedNullness = prevExpectedNullness;
}
}

/** Visitor method: perform a type translation on list of trees.
*/
public <T extends JCTree> List<T> translate(List<T> trees, Type pt) {
return translate(trees, pt, JCNullableTypeExpression.NullMarker.UNSPECIFIED);
}

public <T extends JCTree> List<T> translate(List<T> trees, Type pt, JCNullableTypeExpression.NullMarker expectedNullness) {
Type prevPt = this.pt;
JCNullableTypeExpression.NullMarker prevExpectedNullness = this.expectedNullness;
List<T> res;
try {
this.pt = pt;
this.expectedNullness = expectedNullness;
res = translate(trees);
} finally {
this.pt = prevPt;
this.expectedNullness = prevExpectedNullness;
}
return res;
}
@@ -518,12 +536,7 @@ public void visitMethodDef(JCMethodDecl tree) {

public void visitVarDef(JCVariableDecl tree) {
tree.vartype = translate(tree.vartype, null);
if (tree.init != null) {
if (types.isNonNullable(tree.type) && !types.isNonNullable(tree.init.type)) {
tree.init = attr.makeNullCheck(tree.init);
}
}
tree.init = translate(tree.init, tree.sym.erasure(types));
tree.init = translate(tree.init, tree.sym.erasure(types), tree.type.getNullMarker());
tree.type = erasure(tree.type);
result = tree;
}
@@ -625,6 +638,7 @@ public void visitSwitchExpression(JCSwitchExpression tree) {
tree.cases = translate(tree.cases, tree.type);
tree.type = erasure(tree.type);
result = retype(tree, tree.type, pt);
result = generateNullCheckIfNeeded((JCExpression)result);
}

public void visitRecordPattern(JCRecordPattern tree) {
@@ -655,6 +669,7 @@ public void visitConditional(JCConditional tree) {
tree.falsepart = translate(tree.falsepart, erasure(tree.type));
tree.type = erasure(tree.type);
result = retype(tree, tree.type, pt);
result = generateNullCheckIfNeeded((JCExpression)result);
}

public void visitIf(JCIf tree) {
@@ -685,6 +700,7 @@ public void visitYield(JCYield tree) {
tree.value = translate(tree.value, erasure(tree.value.type));
tree.value.type = erasure(tree.value.type);
tree.value = retype(tree.value, tree.value.type, pt);
result = generateNullCheckIfNeeded((JCExpression)result);
result = tree;
}

@@ -728,6 +744,7 @@ public void visitApply(JCMethodInvocation tree) {
tree.type = types.erasure(tree.type);
// Insert casts of method invocation results as needed.
result = retype(tree, mt.getReturnType(), pt);
result = generateNullCheckIfNeeded((JCExpression)result);
}

public void visitNewClass(JCNewClass tree) {
@@ -775,7 +792,7 @@ public void visitNewArray(JCNewArray tree) {
}

public void visitParens(JCParens tree) {
tree.expr = translate(tree.expr, pt);
tree.expr = translate(tree.expr, pt, expectedNullness);
tree.type = erasure(tree.expr.type);
result = tree;
}
@@ -788,6 +805,7 @@ public void visitAssign(JCAssign tree) {
tree.rhs = translate(tree.rhs, erasure(tree.lhs.type));
tree.type = erasure(tree.lhs.type);
result = retype(tree, tree.type, pt);
result = generateNullCheckIfNeeded((JCExpression)result);
}

public void visitAssignop(JCAssignOp tree) {
@@ -845,6 +863,7 @@ public void visitTypeCast(JCTypeCast tree) {
}
}
result = retype(tree, tree.type, pt);
result = generateNullCheckIfNeeded((JCExpression)result);
}

public void visitTypeTest(JCInstanceOf tree) {
@@ -859,6 +878,7 @@ public void visitIndexed(JCArrayAccess tree) {

// Insert casts of indexed expressions as needed.
result = retype(tree, types.elemtype(tree.indexed.type), pt);
result = generateNullCheckIfNeeded((JCExpression)result);
}

// There ought to be nothing to rewrite here;
@@ -881,6 +901,7 @@ public void visitIdent(JCIdent tree) {
// Insert casts of variable uses as needed.
else if (tree.sym.kind == VAR) {
result = retype(tree, et, pt);
result = generateNullCheckIfNeeded((JCExpression)result);
}
else {
tree.type = erasure(tree.type);
@@ -911,7 +932,8 @@ public void visitSelect(JCFieldAccess tree) {
}
// Insert casts of variable uses as needed.
else if (tree.sym.kind == VAR) {
result = retype(tree, tree.sym.erasure(types), pt);
result = generateNullCheckIfNeeded(tree);
result = retype((JCExpression) result, tree.sym.erasure(types), pt);
}
else {
tree.type = erasure(tree.type);