Incoherent invocation type inference?
B. Blaser
bsrbnd at gmail.com
Sun Jan 22 16:26:44 UTC 2017
Hi,
2017-01-19 18:22 GMT+01:00 Maurizio Cimadamore <maurizio.cimadamore at oracle.com>:
> Not sure about your proposed patch.
>
> To me the warning should be a property of the method declaration, not of the
> specific inference.
>
> If a method returns a naked type-variable which is not mentioned anywhere in
> the method parameter types -> Lint warning (not an unchecked warning - just
> an optional Lint one - category TBD).
>
> It's true that, by looking at the callsite, you can also warn for clients
> passing 'null' arguments, but the extra benefit is not worth the extra
> complexity IMHO. And, I think this is a problem of bad API, not one of bad
> clients. A well-designed API should not have any methods that match the
> criteria stated above - as their behavior would ultimately be at the
> client's mercy.
>
> Maurizio
Here under is a suggestion for the implementation of the rule you
stated above (Attr.visitMethodDef(), upon rev. b6960e2da008).
I kept a check on the callsite for further evaluation (in
Attr.checkMethod() to be more general). I think both are
complementary.
"<T> T[] List.toArray(T[])" is a good example of a well designed API.
But an invocation of the form "a = l.toArray(null)" is dangerous due
to the obvious API's lack of control over T and might be warned.
For the Lint category, I suggest to name it "generic" and to use it
for warnings about type variables. Other checks like the "final" one
could fall into the same category.
Does this look better?
Thanks,
Bernard
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
@@ -268,6 +268,11 @@
UNCHECKED("unchecked"),
/**
+ * Warn about issues relating to type variables.
+ */
+ GENERIC("generic"),
+
+ /**
* Warn about potentially unsafe vararg methods
*/
VARARGS("varargs");
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
@@ -947,6 +947,7 @@
deferredLintHandler.flush(tree.pos());
chk.checkDeprecatedAnnotation(tree.pos(), m);
+ chk.checkGenMethDeclaration(tree.pos(), m);
// Create a new environment with local scope
// for attributing the method.
@@ -3918,6 +3919,8 @@
final List<JCExpression> argtrees,
List<Type> argtypes,
List<Type> typeargtypes) {
+ chk.checkGenMethInvocation(env.tree.pos(), sym, resultInfo, argtypes);
+
// Test (5): if symbol is an instance method of a raw type, issue
// an unchecked warning if its argument types change under erasure.
if ((sym.flags() & STATIC) == 0 &&
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
@@ -3876,4 +3876,76 @@
}
}
+ // Check the declaration's precision for type variables.
+ void checkGenMethDeclaration(DiagnosticPosition pos, Symbol sym) {
+ if (sym.type.hasTag(FORALL)) {
+ ForAll gmt = (ForAll)sym.type;
+ MethodType mt = (MethodType)gmt.qtype;
+ List<Type> tvars = gmt.tvars;
+
+ Type elemResType = mt.restype;
+ while (elemResType.hasTag(ARRAY)) {
+ elemResType = ((ArrayType)elemResType).elemtype;
+ }
+ if (elemResType.hasTag(TYPEVAR) && tvars.contains(elemResType)) {
+ boolean ok = false;
+ for (Type t: mt.argtypes) {
+ if (t.contains(elemResType))
+ ok = true;
+ }
+ if (!ok)
+ warnGeneric(
+ pos,
+ "generic.imprecise.method.declaration",
+ Kinds.kindName(sym),
+ sym.name,
+ Kinds.kindName(sym.location()),
+ sym.location(),
+ elemResType);
+ }
+ }
+ }
+
+ // Check the actual arguments' precision for type variables.
+ void checkGenMethInvocation(
+ DiagnosticPosition pos,
+ Symbol sym,
+ Attr.ResultInfo resultInfo,
+ List<Type> argtypes) {
+
+ if (sym.type.hasTag(FORALL)) {
+ ForAll gmt = (ForAll)sym.type;
+ MethodType mt = (MethodType)gmt.qtype;
+
+ Type elemResType = mt.restype;
+ while (elemResType.hasTag(ARRAY)) {
+ elemResType = ((ArrayType)elemResType).elemtype;
+ }
+ if (resultInfo != null && resultInfo.pt != Type.noType &&
elemResType.hasTag(TYPEVAR)) {
+ boolean ok = false;
+ if (mt.argtypes.length() <= argtypes.length()) {
+ for (int i=0; i<mt.argtypes.length(); i++) {
+ Type pType = mt.argtypes.get(i);
+ Type aType = argtypes.get(i);
+ if (pType.contains(elemResType) && aType !=
syms.botType)
+ ok = true;
+ }
+ }
+ if (!ok)
+ warnGeneric(
+ pos,
+ "generic.imprecise.method.invocation",
+ Kinds.kindName(sym),
+ sym.name,
+ Kinds.kindName(sym.location()),
+ sym.location(),
+ elemResType);
+ }
+ }
+ }
+
+ void warnGeneric(DiagnosticPosition pos, String key, Object... args) {
+ if (lint.isEnabled(LintCategory.GENERIC))
+ log.warning(LintCategory.GENERIC, pos, key, args);
+ }
}
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
@@ -1756,6 +1756,14 @@
compiler.warn.unchecked.varargs.non.reifiable.type=\
Possible heap pollution from parameterized vararg type {0}
+# 0: symbol kind, 1: name, 2: symbol kind, 3: symbol, 4: type
+compiler.warn.generic.imprecise.method.declaration=\
+ Imprecise method declaration: parameters of {0} {1} in {2} {3}
should refer to {4}
+
+# 0: symbol kind, 1: name, 2: symbol kind, 3: symbol, 4: type
+compiler.warn.generic.imprecise.method.invocation=\
+ Imprecise method invocation: arguments of {0} {1} in {2} {3}
should refer to {4}
+
# 0: symbol
compiler.warn.varargs.unsafe.use.varargs.param=\
Varargs method could cause heap pollution from non-reifiable
varargs parameter {0}
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties
b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties
@@ -238,6 +238,9 @@
javac.opt.Xlint.desc.unchecked=\
Warn about unchecked operations.
+javac.opt.Xlint.desc.generic=\
+ Warn about issues relating to type variables.
+
javac.opt.Xlint.desc.varargs=\
Warn about potentially unsafe vararg methods
More information about the compiler-dev
mailing list