# HG changeset patch
# User mcimadamore
# Date 1318852473 -3600
# Node ID 9444a714fdaf41b69775aba5c4e2b7013560867a
# Parent ca322e50ddb7a3a28d45fea188dc28cb8b4930f5
7097436: Project Coin: duplicate varargs warnings on method annotated with @SafeVarargs
Summary: Duplicate aliasing check during subtyping leads to spurious varargs diagnostic
Reviewed-by: jjg
diff -r ca322e50ddb7 -r 9444a714fdaf langtools/src/share/classes/com/sun/tools/javac/code/Types.java
--- a/langtools/src/share/classes/com/sun/tools/javac/code/Types.java Thu Oct 06 18:39:31 2011 +0100
+++ b/langtools/src/share/classes/com/sun/tools/javac/code/Types.java Mon Oct 17 12:54:33 2011 +0100
@@ -278,7 +278,6 @@
boolean tPrimitive = t.isPrimitive();
boolean sPrimitive = s.isPrimitive();
if (tPrimitive == sPrimitive) {
- checkUnsafeVarargsConversion(t, s, warn);
return isSubtypeUnchecked(t, s, warn);
}
if (!allowBoxing) return false;
@@ -286,27 +285,6 @@
? isSubtype(boxedClass(t).type, s)
: isSubtype(unboxedType(t), s);
}
- //where
- private void checkUnsafeVarargsConversion(Type t, Type s, Warner warn) {
- if (t.tag != ARRAY || isReifiable(t)) return;
- ArrayType from = (ArrayType)t;
- boolean shouldWarn = false;
- switch (s.tag) {
- case ARRAY:
- ArrayType to = (ArrayType)s;
- shouldWarn = from.isVarargs() &&
- !to.isVarargs() &&
- !isReifiable(from);
- break;
- case CLASS:
- shouldWarn = from.isVarargs() &&
- isSubtype(from, s);
- break;
- }
- if (shouldWarn) {
- warn.warn(LintCategory.VARARGS);
- }
- }
/**
* Is t a subtype of or convertiable via boxing/unboxing
@@ -328,42 +306,63 @@
* Is t an unchecked subtype of s?
*/
public boolean isSubtypeUnchecked(Type t, Type s, Warner warn) {
- if (t.tag == ARRAY && s.tag == ARRAY) {
- if (((ArrayType)t).elemtype.tag <= lastBaseTag) {
- return isSameType(elemtype(t), elemtype(s));
- } else {
- ArrayType from = (ArrayType)t;
- ArrayType to = (ArrayType)s;
- if (from.isVarargs() &&
- !to.isVarargs() &&
- !isReifiable(from)) {
- warn.warn(LintCategory.VARARGS);
- }
- return isSubtypeUnchecked(elemtype(t), elemtype(s), warn);
- }
- } else if (isSubtype(t, s)) {
- return true;
+ boolean result = isSubtypeUncheckedInternal(t, s, warn);
+ if (result) {
+ checkUnsafeVarargsConversion(t, s, warn);
}
- else if (t.tag == TYPEVAR) {
- return isSubtypeUnchecked(t.getUpperBound(), s, warn);
- }
- else if (s.tag == UNDETVAR) {
- UndetVar uv = (UndetVar)s;
- if (uv.inst != null)
- return isSubtypeUnchecked(t, uv.inst, warn);
- }
- else if (!s.isRaw()) {
- Type t2 = asSuper(t, s.tsym);
- if (t2 != null && t2.isRaw()) {
- if (isReifiable(s))
- warn.silentWarn(LintCategory.UNCHECKED);
- else
- warn.warn(LintCategory.UNCHECKED);
+ return result;
+ }
+ //where
+ private boolean isSubtypeUncheckedInternal(Type t, Type s, Warner warn) {
+ if (t.tag == ARRAY && s.tag == ARRAY) {
+ if (((ArrayType)t).elemtype.tag <= lastBaseTag) {
+ return isSameType(elemtype(t), elemtype(s));
+ } else {
+ return isSubtypeUnchecked(elemtype(t), elemtype(s), warn);
+ }
+ } else if (isSubtype(t, s)) {
return true;
}
+ else if (t.tag == TYPEVAR) {
+ return isSubtypeUnchecked(t.getUpperBound(), s, warn);
+ }
+ else if (s.tag == UNDETVAR) {
+ UndetVar uv = (UndetVar)s;
+ if (uv.inst != null)
+ return isSubtypeUnchecked(t, uv.inst, warn);
+ }
+ else if (!s.isRaw()) {
+ Type t2 = asSuper(t, s.tsym);
+ if (t2 != null && t2.isRaw()) {
+ if (isReifiable(s))
+ warn.silentWarn(LintCategory.UNCHECKED);
+ else
+ warn.warn(LintCategory.UNCHECKED);
+ return true;
+ }
+ }
+ return false;
}
- return false;
- }
+
+ private void checkUnsafeVarargsConversion(Type t, Type s, Warner warn) {
+ if (t.tag != ARRAY || isReifiable(t)) return;
+ ArrayType from = (ArrayType)t;
+ boolean shouldWarn = false;
+ switch (s.tag) {
+ case ARRAY:
+ ArrayType to = (ArrayType)s;
+ shouldWarn = from.isVarargs() &&
+ !to.isVarargs() &&
+ !isReifiable(from);
+ break;
+ case CLASS:
+ shouldWarn = from.isVarargs();
+ break;
+ }
+ if (shouldWarn) {
+ warn.warn(LintCategory.VARARGS);
+ }
+ }
/**
* Is t a subtype of s?
diff -r ca322e50ddb7 -r 9444a714fdaf langtools/test/tools/javac/varargs/7097436/T7097436.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/varargs/7097436/T7097436.java Mon Oct 17 12:54:33 2011 +0100
@@ -0,0 +1,18 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 7097436
+ * @summary ClassCastException occurs in assignment expressions without any heap pollutions
+ * @compile/fail/ref=T7097436.out -Xlint:varargs -Werror -XDrawDiagnostics T7097436.java
+ */
+
+import java.util.List;
+
+class T7097436 {
+ @SafeVarargs
+ static void m(List... ls) {
+ Object o = ls; //warning
+ Object[] oArr = ls; //warning
+ String s = ls; // no warning
+ Integer[] iArr = ls; // no warning
+ }
+}
diff -r ca322e50ddb7 -r 9444a714fdaf langtools/test/tools/javac/varargs/7097436/T7097436.out
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/varargs/7097436/T7097436.out Mon Oct 17 12:54:33 2011 +0100
@@ -0,0 +1,6 @@
+T7097436.java:13:20: compiler.warn.varargs.unsafe.use.varargs.param: ls
+T7097436.java:14:25: compiler.warn.varargs.unsafe.use.varargs.param: ls
+T7097436.java:15:20: compiler.err.prob.found.req: (compiler.misc.incompatible.types), java.util.List[], java.lang.String
+T7097436.java:16:26: compiler.err.prob.found.req: (compiler.misc.incompatible.types), java.util.List[], java.lang.Integer[]
+2 errors
+2 warnings
diff -r ca322e50ddb7 -r 9444a714fdaf langtools/test/tools/javac/varargs/warning/Warn5.java
--- a/langtools/test/tools/javac/varargs/warning/Warn5.java Thu Oct 06 18:39:31 2011 +0100
+++ b/langtools/test/tools/javac/varargs/warning/Warn5.java Mon Oct 17 12:54:33 2011 +0100
@@ -23,7 +23,7 @@
/**
* @test
- * @bug 6993978
+ * @bug 6993978 7097436
* @summary Project Coin: Annotation to reduce varargs warnings
* @author mcimadamore
* @run main Warn5
@@ -31,8 +31,8 @@
import com.sun.source.util.JavacTask;
import com.sun.tools.javac.api.JavacTool;
import java.net.URI;
-import java.util.ArrayList;
import java.util.Arrays;
+import java.util.EnumSet;
import javax.tools.Diagnostic;
import javax.tools.JavaCompiler;
import javax.tools.JavaFileObject;
@@ -95,7 +95,6 @@
METHOD("void m"),
CONSTRUCTOR("Test");
-
String name;
MethodKind(String name) {
@@ -155,7 +154,124 @@
}
}
- static class JavaSource extends SimpleJavaFileObject {
+ enum WarningKind {
+ UNSAFE_BODY,
+ UNSAFE_DECL,
+ MALFORMED_SAFEVARARGS,
+ REDUNDANT_SAFEVARARGS;
+ }
+
+ // Create a single file manager and reuse it for each compile to save time.
+ static StandardJavaFileManager fm = JavacTool.create().getStandardFileManager(null, null, null);
+
+ public static void main(String... args) throws Exception {
+ for (SourceLevel sourceLevel : SourceLevel.values()) {
+ for (XlintOption xlint : XlintOption.values()) {
+ for (TrustMe trustMe : TrustMe.values()) {
+ for (SuppressLevel suppressLevel : SuppressLevel.values()) {
+ for (ModifierKind modKind : ModifierKind.values()) {
+ for (MethodKind methKind : MethodKind.values()) {
+ for (SignatureKind sig : SignatureKind.values()) {
+ for (BodyKind body : BodyKind.values()) {
+ new Warn5(sourceLevel,
+ xlint,
+ trustMe,
+ suppressLevel,
+ modKind,
+ methKind,
+ sig,
+ body).test();
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ final SourceLevel sourceLevel;
+ final XlintOption xlint;
+ final TrustMe trustMe;
+ final SuppressLevel suppressLevel;
+ final ModifierKind modKind;
+ final MethodKind methKind;
+ final SignatureKind sig;
+ final BodyKind body;
+ final JavaSource source;
+ final DiagnosticChecker dc;
+
+ public Warn5(SourceLevel sourceLevel, XlintOption xlint, TrustMe trustMe, SuppressLevel suppressLevel, ModifierKind modKind, MethodKind methKind, SignatureKind sig, BodyKind body) {
+ this.sourceLevel = sourceLevel;
+ this.xlint = xlint;
+ this.trustMe = trustMe;
+ this.suppressLevel = suppressLevel;
+ this.modKind = modKind;
+ this.methKind = methKind;
+ this.sig = sig;
+ this.body = body;
+ this.source = new JavaSource();
+ this.dc = new DiagnosticChecker();
+ }
+
+ void test() throws Exception {
+ final JavaCompiler tool = ToolProvider.getSystemJavaCompiler();
+ JavacTask ct = (JavacTask)tool.getTask(null, fm, dc,
+ Arrays.asList(xlint.getXlintOption(), "-source", sourceLevel.sourceKey), null, Arrays.asList(source));
+ ct.analyze();
+ check();
+ }
+
+ void check() {
+
+ EnumSet expectedWarnings = EnumSet.noneOf(WarningKind.class);
+
+ if (sourceLevel == SourceLevel.JDK_7 &&
+ trustMe == TrustMe.TRUST &&
+ suppressLevel != SuppressLevel.VARARGS &&
+ xlint != XlintOption.NONE &&
+ sig.isVarargs && !sig.isReifiableArg && body.hasAliasing &&
+ (methKind == MethodKind.CONSTRUCTOR || (methKind == MethodKind.METHOD && modKind != ModifierKind.NONE))) {
+ expectedWarnings.add(WarningKind.UNSAFE_BODY);
+ }
+
+ if (sourceLevel == SourceLevel.JDK_7 &&
+ trustMe == TrustMe.DONT_TRUST &&
+ sig.isVarargs &&
+ !sig.isReifiableArg &&
+ xlint == XlintOption.ALL) {
+ expectedWarnings.add(WarningKind.UNSAFE_DECL);
+ }
+
+ if (sourceLevel == SourceLevel.JDK_7 &&
+ trustMe == TrustMe.TRUST &&
+ (!sig.isVarargs ||
+ (modKind == ModifierKind.NONE && methKind == MethodKind.METHOD))) {
+ expectedWarnings.add(WarningKind.MALFORMED_SAFEVARARGS);
+ }
+
+ if (sourceLevel == SourceLevel.JDK_7 &&
+ trustMe == TrustMe.TRUST &&
+ xlint != XlintOption.NONE &&
+ suppressLevel != SuppressLevel.VARARGS &&
+ (modKind != ModifierKind.NONE || methKind == MethodKind.CONSTRUCTOR) &&
+ sig.isVarargs &&
+ sig.isReifiableArg) {
+ expectedWarnings.add(WarningKind.REDUNDANT_SAFEVARARGS);
+ }
+
+ if (!expectedWarnings.containsAll(dc.warnings) ||
+ !dc.warnings.containsAll(expectedWarnings)) {
+ throw new Error("invalid diagnostics for source:\n" +
+ source.getCharContent(true) +
+ "\nOptions: " + xlint.getXlintOption() +
+ "\nExpected warnings: " + expectedWarnings +
+ "\nFound warnings: " + dc.warnings);
+ }
+ }
+
+ class JavaSource extends SimpleJavaFileObject {
String template = "import com.sun.tools.javac.api.*;\n" +
"import java.util.List;\n" +
@@ -167,12 +283,11 @@
String source;
- public JavaSource(TrustMe trustMe, SuppressLevel suppressLevel, ModifierKind modKind,
- MethodKind methKind, SignatureKind meth, BodyKind body) {
+ public JavaSource() {
super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
source = template.replace("#T", trustMe.anno).
replace("#S", suppressLevel.getSuppressAnno()).
- replace("#M", meth.getSignature(modKind, methKind)).
+ replace("#M", sig.getSignature(modKind, methKind)).
replace("#B", body.body);
}
@@ -182,117 +297,34 @@
}
}
- public static void main(String... args) throws Exception {
- for (SourceLevel sourceLevel : SourceLevel.values()) {
- for (XlintOption xlint : XlintOption.values()) {
- for (TrustMe trustMe : TrustMe.values()) {
- for (SuppressLevel suppressLevel : SuppressLevel.values()) {
- for (ModifierKind modKind : ModifierKind.values()) {
- for (MethodKind methKind : MethodKind.values()) {
- for (SignatureKind sig : SignatureKind.values()) {
- for (BodyKind body : BodyKind.values()) {
- test(sourceLevel,
- xlint,
- trustMe,
- suppressLevel,
- modKind,
- methKind,
- sig,
- body);
- }
- }
- }
- }
- }
- }
- }
- }
- }
-
- // Create a single file manager and reuse it for each compile to save time.
- static StandardJavaFileManager fm = JavacTool.create().getStandardFileManager(null, null, null);
-
- static void test(SourceLevel sourceLevel, XlintOption xlint, TrustMe trustMe, SuppressLevel suppressLevel,
- ModifierKind modKind, MethodKind methKind, SignatureKind sig, BodyKind body) throws Exception {
- final JavaCompiler tool = ToolProvider.getSystemJavaCompiler();
- JavaSource source = new JavaSource(trustMe, suppressLevel, modKind, methKind, sig, body);
- DiagnosticChecker dc = new DiagnosticChecker();
- JavacTask ct = (JavacTask)tool.getTask(null, fm, dc,
- Arrays.asList(xlint.getXlintOption(), "-source", sourceLevel.sourceKey), null, Arrays.asList(source));
- ct.analyze();
- check(sourceLevel, dc, source, xlint, trustMe,
- suppressLevel, modKind, methKind, sig, body);
- }
-
- static void check(SourceLevel sourceLevel, DiagnosticChecker dc, JavaSource source,
- XlintOption xlint, TrustMe trustMe, SuppressLevel suppressLevel, ModifierKind modKind,
- MethodKind methKind, SignatureKind meth, BodyKind body) {
+ class DiagnosticChecker implements javax.tools.DiagnosticListener {
- boolean hasPotentiallyUnsafeBody = sourceLevel == SourceLevel.JDK_7 &&
- trustMe == TrustMe.TRUST &&
- suppressLevel != SuppressLevel.VARARGS &&
- xlint != XlintOption.NONE &&
- meth.isVarargs && !meth.isReifiableArg && body.hasAliasing &&
- (methKind == MethodKind.CONSTRUCTOR || (methKind == MethodKind.METHOD && modKind != ModifierKind.NONE));
-
- boolean hasPotentiallyPollutingDecl = sourceLevel == SourceLevel.JDK_7 &&
- trustMe == TrustMe.DONT_TRUST &&
- meth.isVarargs &&
- !meth.isReifiableArg &&
- xlint == XlintOption.ALL;
-
- boolean hasMalformedAnnoInDecl = sourceLevel == SourceLevel.JDK_7 &&
- trustMe == TrustMe.TRUST &&
- (!meth.isVarargs ||
- (modKind == ModifierKind.NONE && methKind == MethodKind.METHOD));
-
- boolean hasRedundantAnnoInDecl = sourceLevel == SourceLevel.JDK_7 &&
- trustMe == TrustMe.TRUST &&
- xlint != XlintOption.NONE &&
- suppressLevel != SuppressLevel.VARARGS &&
- (modKind != ModifierKind.NONE || methKind == MethodKind.CONSTRUCTOR) &&
- meth.isVarargs &&
- meth.isReifiableArg;
-
- if (hasPotentiallyUnsafeBody != dc.hasPotentiallyUnsafeBody ||
- hasPotentiallyPollutingDecl != dc.hasPotentiallyPollutingDecl ||
- hasMalformedAnnoInDecl != dc.hasMalformedAnnoInDecl ||
- hasRedundantAnnoInDecl != dc.hasRedundantAnnoInDecl) {
- throw new Error("invalid diagnostics for source:\n" +
- source.getCharContent(true) +
- "\nOptions: " + xlint.getXlintOption() +
- "\nExpected potentially unsafe body warning: " + hasPotentiallyUnsafeBody +
- "\nExpected potentially polluting decl warning: " + hasPotentiallyPollutingDecl +
- "\nExpected malformed anno error: " + hasMalformedAnnoInDecl +
- "\nExpected redundant anno warning: " + hasRedundantAnnoInDecl +
- "\nFound potentially unsafe body warning: " + dc.hasPotentiallyUnsafeBody +
- "\nFound potentially polluting decl warning: " + dc.hasPotentiallyPollutingDecl +
- "\nFound malformed anno error: " + dc.hasMalformedAnnoInDecl +
- "\nFound redundant anno warning: " + dc.hasRedundantAnnoInDecl);
- }
- }
-
- static class DiagnosticChecker implements javax.tools.DiagnosticListener {
-
- boolean hasPotentiallyUnsafeBody = false;
- boolean hasPotentiallyPollutingDecl = false;
- boolean hasMalformedAnnoInDecl = false;
- boolean hasRedundantAnnoInDecl = false;
+ EnumSet warnings = EnumSet.noneOf(WarningKind.class);
public void report(Diagnostic diagnostic) {
if (diagnostic.getKind() == Diagnostic.Kind.WARNING) {
if (diagnostic.getCode().contains("unsafe.use.varargs.param")) {
- hasPotentiallyUnsafeBody = true;
+ setWarning(WarningKind.UNSAFE_BODY);
} else if (diagnostic.getCode().contains("redundant.trustme")) {
- hasRedundantAnnoInDecl = true;
+ setWarning(WarningKind.REDUNDANT_SAFEVARARGS);
}
} else if (diagnostic.getKind() == Diagnostic.Kind.MANDATORY_WARNING &&
diagnostic.getCode().contains("varargs.non.reifiable.type")) {
- hasPotentiallyPollutingDecl = true;
+ setWarning(WarningKind.UNSAFE_DECL);
} else if (diagnostic.getKind() == Diagnostic.Kind.ERROR &&
diagnostic.getCode().contains("invalid.trustme")) {
- hasMalformedAnnoInDecl = true;
+ setWarning(WarningKind.MALFORMED_SAFEVARARGS);
}
}
+
+ void setWarning(WarningKind wk) {
+ if (!warnings.add(wk)) {
+ throw new AssertionError("Duplicate warning of kind " + wk + " in source:\n" + source);
+ }
+ }
+
+ boolean hasWarning(WarningKind wk) {
+ return warnings.contains(wk);
+ }
}
}