Skip to content

Commit

Permalink
Add support to disable parametric nullity by default
Browse files Browse the repository at this point in the history
  • Loading branch information
@mcimadamore
mcimadamore committed Aug 23, 2024
1 parent 5319a5a commit 4df065d
Show file tree
Hide file tree
Showing 15 changed files with 121 additions and 72 deletions.
44 changes: 6 additions & 38 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,13 +543,7 @@ public String toString() {
if (moreInfo && hasTag(TYPEVAR)) {
sb.append(hashCode());
}
if (isNullable()) {
sb.append("?");
} else if (isNonNullable()) {
sb.append("!");
} else if (isParametric()) {
sb.append("*");
}
sb.append(getNullMarker().typeSuffix());
return sb.toString();
}

Expand Down Expand Up @@ -764,22 +758,6 @@ public Type asNullMarked(NullMarker nullMarker) {
base.addMetadata(new TypeMetadata.NullMarker(nullMarker));
}

public boolean isNullable() {
return getNullMarker() == NullMarker.NULLABLE;
}

public boolean isNonNullable() {
return getNullMarker() == NullMarker.NOT_NULL;
}

public boolean isParametric() {
return getNullMarker() == NullMarker.PARAMETRIC;
}

public boolean isNullUnspecified() {
return getNullMarker() == NullMarker.UNSPECIFIED;
}

public NullMarker getNullMarker() {
TypeMetadata.NullMarker nm = getMetadata(TypeMetadata.NullMarker.class);
return nm != null ?
Expand Down Expand Up @@ -1149,13 +1127,7 @@ public String toString() {
}
}

if (isNullable()) {
buf.append("?");
} else if (isNonNullable()) {
buf.append("!");
} else if (isParametric()) {
buf.append("*");
}
buf.append(getNullMarker().typeSuffix());

if (getTypeArguments().nonEmpty()) {
buf.append('<');
Expand Down Expand Up @@ -1472,13 +1444,7 @@ public String toString() {
t = this;
do {
t.appendAnnotationsString(sb, true);
if (t.isNullable()) {
sb.append("?");
} else if (t.isNonNullable()) {
sb.append("!");
} else if (t.isParametric()) {
sb.append("*");
}
sb.append(t.getNullMarker().typeSuffix());
sb.append("[]");
t = ((ArrayType) t).getComponentType();
} while (t.getKind() == TypeKind.ARRAY);
Expand Down Expand Up @@ -2236,7 +2202,9 @@ private void addBound(InferenceBound ib, Type bound, Types types, boolean update
}
} else {
Type bound2 = bound.map(toTypeVarMap).baseType();
bound2 = bound2.asNullMarked(bound.getNullMarker());
if (types.isParametric(qtype)) {
bound2 = bound2.asNullMarked(bound.getNullMarker());
}
List<Type> prevBounds = bounds.get(ib);
if (bound == qtype) return;
for (Type b : prevBounds) {
Expand Down
45 changes: 39 additions & 6 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import java.util.Optional;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.function.BiFunction;
import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.function.Predicate;
Expand Down Expand Up @@ -102,6 +101,7 @@ public class Types {

/* are nullable and null-restricted types allowed? */
private boolean allowNullRestrictedTypes;
private boolean tvarUnspecifiedNullity;

// <editor-fold defaultstate="collapsed" desc="Instantiating">
public static Types instance(Context context) {
Expand Down Expand Up @@ -131,6 +131,8 @@ public String toString() {
Preview preview = Preview.instance(context);
allowNullRestrictedTypes = (!preview.isPreview(Source.Feature.NULL_RESTRICTED_TYPES) || preview.isEnabled()) &&
Source.Feature.NULL_RESTRICTED_TYPES.allowedInSource(source);
Options options = Options.instance(context);
tvarUnspecifiedNullity = options.isSet("tvarUnspecifiedNullity");
}
// </editor-fold>

Expand Down Expand Up @@ -1143,7 +1145,7 @@ public Boolean visitType(Type t, Type s) {
case TYPEVAR:
return isSubtypeNoCapture(t.getUpperBound(), s);
case BOT: {
if (s.isNonNullable()) {
if (isNonNullable(s)) {
return false;
}
return
Expand Down Expand Up @@ -1770,7 +1772,7 @@ public Boolean visitWildcardType(WildcardType t, Type s) {

@Override
public Boolean visitClassType(ClassType t, Type s) {
if (s.hasTag(ERROR) || s.hasTag(BOT) && (!t.hasImplicitConstructor() || !t.isNonNullable()))
if (s.hasTag(ERROR) || s.hasTag(BOT) && (!t.hasImplicitConstructor() || !isNonNullable(t)))
return true;

if (s.hasTag(TYPEVAR)) {
Expand Down Expand Up @@ -2375,8 +2377,20 @@ public Type visitClassType(ClassType t, Symbol sym) {
if (baseParams.isEmpty()) {
// then base is a raw type
return erasure(sym.type);
} else if (baseParams.length() != ownerParams.length()) {
// rare type, recovery
return subst(sym.type, ownerParams,
baseParams.map(ta -> ta.asNullMarked(NullMarker.UNSPECIFIED)));
} else {
return subst(sym.type, ownerParams, baseParams);
ListBuffer<Type> newBaseParams = new ListBuffer<>();
for (Type tvar : ownerParams) {
Type baseParam = isParametric(tvar) ?
baseParams.head :
baseParams.head.asNullMarked(NullMarker.UNSPECIFIED);
newBaseParams.add(baseParam);
baseParams = baseParams.tail;
}
return subst(sym.type, ownerParams, newBaseParams.toList());
}
}
}
Expand Down Expand Up @@ -5397,11 +5411,30 @@ public Type constantType(LoadableConstant c) {
// </editor-fold>

// <editor-fold defaultstate="collapsed" desc="nullability methods">

public boolean isNullable(Type type) {
return type.getNullMarker() == NullMarker.NULLABLE;
}

public boolean isNonNullable(Type type) {
return type.getNullMarker() == NullMarker.NOT_NULL;
}

public boolean isParametric(Type type) {
return type.getNullMarker() == NullMarker.PARAMETRIC ||
(type.hasTag(TYPEVAR) && type.getNullMarker() == NullMarker.UNSPECIFIED && !tvarUnspecifiedNullity);
}

public boolean isNullUnspecified(Type type) {
return type.getNullMarker() == NullMarker.UNSPECIFIED &&
(!type.hasTag(TYPEVAR) || tvarUnspecifiedNullity);
}

/**
* Do t and s have the same nullability?
*/
public boolean hasSameNullability(Type t, Type s) {
if (s == null || t == null || t.isNullUnspecified() || s.isNullUnspecified()) {
if (s == null || t == null || isNullUnspecified(t) || isNullUnspecified(s)) {
return true;
}
return t.getNullMarker() == s.getNullMarker();
Expand All @@ -5411,7 +5444,7 @@ public boolean hasSameNullability(Type t, Type s) {
* Does t have narrower nullability than s?
*/
public boolean hasNarrowerNullability(Type t, Type s) {
if (s == null || t == null || t.isNullUnspecified() || s.isNullUnspecified()) {
if (s == null || t == null || isNullUnspecified(t) || isNullUnspecified(s)) {
return false;
}
return t.getNullMarker().ordinal() < s.getNullMarker().ordinal();
Expand Down
6 changes: 3 additions & 3 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,7 @@ public void visitVarDef(JCVariableDecl tree) {
elemOrType = types.elemtype(elemOrType);
}
if (allowNullRestrictedTypesForValueClassesOnly &&
((result.isNonNullable() || elemOrType.isNonNullable()) && (!elemOrType.isValueClass() || !elemOrType.hasImplicitConstructor()))) {
((types.isNonNullable(result) || types.isNonNullable(elemOrType)) && (!elemOrType.isValueClass() || !elemOrType.hasImplicitConstructor()))) {
log.error(tree.pos(),
types.elemtype(result) == null?
Errors.TypeCantBeNullRestricted(result) :
Expand Down Expand Up @@ -4465,11 +4465,11 @@ public void visitSelect(JCFieldAccess tree) {
site = capture(site); // Capture field access

// check nullness of site
if (site.isNullable()) {
if (types.isNullable(site)) {
chk.warnNullableTypes(tree.selected, Warnings.AccessingMemberOfNullable);
}

if (site.isParametric()) {
if (types.isParametric(site)) {
chk.warnNullableTypes(tree.selected, Warnings.AccessingMemberOfParametric);
}

Expand Down
4 changes: 2 additions & 2 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2542,7 +2542,7 @@ private void checkNonCyclicMembership(ClassSymbol c, DiagnosticPosition pos) {
}
// where
private boolean cyclePossible(VarSymbol symbol) {
return (symbol.flags() & STATIC) == 0 && symbol.type.isValueClass() && symbol.type.hasImplicitConstructor() && symbol.type.isNonNullable();
return (symbol.flags() & STATIC) == 0 && symbol.type.isValueClass() && symbol.type.hasImplicitConstructor() && types.isNonNullable(symbol.type);
}

void checkNonCyclicDecl(JCClassDecl tree) {
Expand Down Expand Up @@ -4638,7 +4638,7 @@ public void warn(LintCategory lint) {
} else {
boolean warned = this.warned;
if (warned) return;
if (expected.isParametric()) {
if (types.isParametric(expected)) {
// not sure this is the right warning
Check.this.warnNullableTypes(pos(), Warnings.NarrowingNullnessConversion);
}
Expand Down
4 changes: 2 additions & 2 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2216,7 +2216,7 @@ boolean isUninitializedNonNullableOrParametricField(VarSymbol sym) {
return sym.owner.kind == TYP &&
((sym.flags() & (FINAL | HASINIT | PARAMETER)) == 0 &&
classDef.sym.isEnclosedBy((ClassSymbol)sym.owner) &&
(sym.type.isNonNullable() || sym.type.isParametric()));
(types.isNonNullable(sym.type) || types.isParametric(sym.type)));
}

/** Initialize new trackable variable by setting its address field
Expand Down Expand Up @@ -2309,7 +2309,7 @@ void checkInit(DiagnosticPosition pos, VarSymbol sym, Error errkey) {
(sym.flags_field & CLASH) == 0) {
if (isUninitializedNonNullableOrParametricField(sym)) {
if (lint.isEnabled(Lint.LintCategory.NULL)) {
if (sym.type.isNonNullable()) {
if (types.isNonNullable(sym.type)) {
log.warning(pos, Warnings.NonNullableShouldBeInitialized);
} else {
log.warning(pos, Warnings.ParametricShouldBeInitialized);
Expand Down
2 changes: 1 addition & 1 deletion src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4402,7 +4402,7 @@ public void visitNewArray(JCNewArray tree) {
noOfDims++;
}
tree.elems = translate(tree.elems, types.elemtype(tree.type));
if (!allowNullRestrictedTypes || tree.elemtype == null || !originalElemType.type.isNonNullable()) {
if (!allowNullRestrictedTypes || tree.elemtype == null || !types.isNonNullable(originalElemType.type)) {
result = tree;
} else {
Symbol elemClass = syms.getClassField(tree.elemtype.type, types);
Expand Down
4 changes: 2 additions & 2 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1340,7 +1340,7 @@ protected void read(Symbol sym, int attrLen) {
if (sym.type.isPrimitive() || sym.type.hasTag(TypeTag.ARRAY)) {
throw badClassFile("attribute.not.applicable.to.field.type", names.NullRestricted, sym.type);
}
if (sym.type.isNonNullable()) {
if (types.isNonNullable(sym.type)) {
throw badClassFile("attribute.must.be.unique", names.NullRestricted);
}
sym.type = sym.type.asNullMarked(JCTree.JCNullableTypeExpression.NullMarker.NOT_NULL);
Expand Down Expand Up @@ -3523,6 +3523,6 @@ private void checkNonCyclicMembershipHelper(ClassSymbol c) {
}
// where
private boolean cyclePossible(VarSymbol symbol) {
return (symbol.flags() & STATIC) == 0 && symbol.type.isValueClass() && symbol.type.hasImplicitConstructor() && symbol.type.isNonNullable();
return (symbol.flags() & STATIC) == 0 && symbol.type.isValueClass() && symbol.type.hasImplicitConstructor() && types.isNonNullable(symbol.type);
}
}
2 changes: 1 addition & 1 deletion src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,7 @@ int writeImplicitCreationIfNeeded(ClassSymbol csym) {
/** Write "NullRestricted" attribute.
*/
int writeNullRestrictedIfNeeded(Symbol sym) {
if (allowNullRestrictedTypes && sym.kind == VAR && sym.type.isNonNullable() && !sym.type.hasTag(ARRAY)) {
if (allowNullRestrictedTypes && sym.kind == VAR && types.isNonNullable(sym.type) && !sym.type.hasTag(ARRAY)) {
int alenIdx = writeAttr(names.NullRestricted);
endAttr(alenIdx);
if (preview.isPreview(Source.Feature.VALUE_CLASSES)) {
Expand Down
8 changes: 4 additions & 4 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ public boolean isConstantDynamic(Symbol sym) {
public void genArgs(List<JCExpression> trees, List<Type> pts) {
for (List<JCExpression> l = trees; l.nonEmpty(); l = l.tail) {
genExpr(l.head, pts.head).load();
if (pts.head.isNonNullable() && !l.head.type.isNonNullable()) {
if (types.isNonNullable(pts.head) && !types.isNullable(l.head.type)) {
code.emitop0(dup);
genNullCheck(l.head);
}
Expand Down Expand Up @@ -1113,7 +1113,7 @@ public void visitVarDef(JCVariableDecl tree) {
Assert.check(code.isStatementStart());
code.newLocal(v);
genExpr(tree.init, v.erasure(types)).load();
if (tree.type.isNonNullable() && !tree.init.type.isNonNullable()) {
if (types.isNonNullable(tree.type) && !types.isNonNullable(tree.init.type)) {
code.emitop0(dup);
genNullCheck(tree.init);
}
Expand Down Expand Up @@ -2118,7 +2118,7 @@ public void visitParens(JCParens tree) {
public void visitAssign(JCAssign tree) {
Item l = genExpr(tree.lhs, tree.lhs.type);
genExpr(tree.rhs, tree.lhs.type).load();
if (tree.lhs.type.isNonNullable() && !tree.rhs.type.isNonNullable()) {
if (types.isNonNullable(tree.lhs.type) && !types.isNonNullable(tree.rhs.type)) {
code.emitop0(dup);
genNullCheck(tree.rhs);
}
Expand Down Expand Up @@ -2327,7 +2327,7 @@ Item completeBinop(JCTree lhs, JCTree rhs, OperatorSymbol operator) {

public void visitTypeCast(JCTypeCast tree) {
result = genExpr(tree.expr, tree.clazz.type).load();
if (tree.clazz.type.isNonNullable() && !tree.expr.type.isNonNullable()) {
if (types.isNonNullable(tree.clazz.type) && !types.isNonNullable(tree.expr.type)) {
code.emitop0(dup);
genNullCheck(tree.expr);
}
Expand Down
1 change: 1 addition & 0 deletions test/langtools/tools/javac/nullability/NullabilityInferenceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* @summary Smoke test for nullability inference
* @enablePreview
* @compile/fail/ref=NullabilityInferenceTest.out -XDrawDiagnostics NullabilityInferenceTest.java
* @compile/fail/ref=NullabilityInferenceTest_parametric.out -XDrawDiagnostics -XDtvarUnspecifiedNullity NullabilityInferenceTest.java
*/

import java.util.List;
Expand Down
14 changes: 7 additions & 7 deletions test/langtools/tools/javac/nullability/NullabilityInferenceTest.out
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
NullabilityInferenceTest.java:22:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:23:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:24:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:25:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
NullabilityInferenceTest.java:33:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:25:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:26:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
NullabilityInferenceTest.java:34:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:35:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:36:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
NullabilityInferenceTest.java:44:39: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
NullabilityInferenceTest.java:36:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:37:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
NullabilityInferenceTest.java:45:39: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
NullabilityInferenceTest.java:46:39: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:47:39: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
NullabilityInferenceTest.java:46:39: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
NullabilityInferenceTest.java:47:39: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String?>>, java.lang.Integer)
NullabilityInferenceTest.java:48:39: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<Test.Box<java.lang.String!>>, java.lang.Integer)
- compiler.note.unchecked.filename: NullabilityInferenceTest.java
- compiler.note.unchecked.recompile
- compiler.note.preview.filename: NullabilityInferenceTest.java, DEFAULT
Expand Down
Loading

0 comments on commit 4df065d

Please sign in to comment.