From 5842f4f6f4a2eef1ff888dd058e699fdcf022c1e Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Wed, 29 Oct 2014 17:10:43 +0100 Subject: [PATCH] recompile classes with potentially ambiguous method calls when vararg method is added --- .../incremental/membersChange/addMethod.log | 2 + .../membersChange/addVarargMethod.log | 14 +++++ .../membersChange/addVarargMethod/src/A.java | 9 +++ .../addVarargMethod/src/A.java.new | 9 +++ .../membersChange/addVarargMethod/src/C.java | 5 ++ .../membersChange/addVarargMethod/src/D.java | 5 ++ .../dependencyView/ClassfileAnalyzer.java | 6 +- .../java/dependencyView/Mappings.java | 60 ++++++++++++------- .../java/dependencyView/MethodRepr.java | 26 ++++---- .../java/dependencyView/UsageRepr.java | 41 +------------ .../incremental/storage/BuildDataManager.java | 2 +- .../org/jetbrains/ether/MemberChangeTest.java | 4 ++ 12 files changed, 105 insertions(+), 78 deletions(-) create mode 100644 java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod.log create mode 100644 java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/A.java create mode 100644 java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/A.java.new create mode 100644 java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/C.java create mode 100644 java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/D.java diff --git a/java/java-tests/testData/compileServer/incremental/membersChange/addMethod.log b/java/java-tests/testData/compileServer/incremental/membersChange/addMethod.log index 316e4be06caa..9e0611c49b3a 100644 --- a/java/java-tests/testData/compileServer/incremental/membersChange/addMethod.log +++ b/java/java-tests/testData/compileServer/incremental/membersChange/addMethod.log @@ -5,8 +5,10 @@ Compiling files: src/A.java End of files Cleaning output files: +out/production/AddMethod/C.class out/production/AddMethod/D.class End of files Compiling files: +src/C.java src/D.java End of files diff --git a/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod.log b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod.log new file mode 100644 index 000000000000..424b39bc3bd2 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod.log @@ -0,0 +1,14 @@ +Cleaning output files: +out/production/AddVarargMethod/A.class +End of files +Compiling files: +src/A.java +End of files +Cleaning output files: +out/production/AddVarargMethod/C.class +out/production/AddVarargMethod/D.class +End of files +Compiling files: +src/C.java +src/D.java +End of files diff --git a/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/A.java b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/A.java new file mode 100644 index 000000000000..eee8a41d0deb --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/A.java @@ -0,0 +1,9 @@ +public class A { + void f (String x, Integer y) { + + } + + void f (String x, Integer y, Long z, String... v){ + + } +} diff --git a/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/A.java.new b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/A.java.new new file mode 100644 index 000000000000..1b7d9e4bdbd8 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/A.java.new @@ -0,0 +1,9 @@ +public class A { + void f (String x, Integer y, String... v) { + + } + + void f (String x, Integer y, Long z, String... v){ + + } +} diff --git a/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/C.java b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/C.java new file mode 100644 index 000000000000..4aa7df6d14a0 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/C.java @@ -0,0 +1,5 @@ +public class C { + void f (A a){ + a.f("1", 2); + } +} diff --git a/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/D.java b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/D.java new file mode 100644 index 000000000000..8c9dba5c485c --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/membersChange/addVarargMethod/src/D.java @@ -0,0 +1,5 @@ +public class D { + void f(A a){ + a.f("1", 2, null, "comment"); + } +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ClassfileAnalyzer.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ClassfileAnalyzer.java index 01013e05186a..137f4527b5a8 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ClassfileAnalyzer.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/ClassfileAnalyzer.java @@ -173,7 +173,7 @@ class ClassfileAnalyzer { } myUsages.add(UsageRepr.createMethodUsage(myContext, methodName, myType.className, methodDescr)); - myUsages.add(UsageRepr.createMetaMethodUsage(myContext, methodName, myType.className, methodDescr)); + myUsages.add(UsageRepr.createMetaMethodUsage(myContext, methodName, myType.className)); myUsedArguments.add(methodName); } @@ -183,7 +183,7 @@ class ClassfileAnalyzer { final String methodDescr = "()" + desc; myUsages.add(UsageRepr.createMethodUsage(myContext, methodName, myType.className, methodDescr)); - myUsages.add(UsageRepr.createMetaMethodUsage(myContext, methodName, myType.className, methodDescr)); + myUsages.add(UsageRepr.createMetaMethodUsage(myContext, methodName, myType.className)); myUsedArguments.add(methodName); } @@ -526,7 +526,7 @@ class ClassfileAnalyzer { final int methodOwner = myContext.get(owner); myUsages.add(UsageRepr.createMethodUsage(myContext, methodName, methodOwner, desc)); - myUsages.add(UsageRepr.createMetaMethodUsage(myContext, methodName, methodOwner, desc)); + myUsages.add(UsageRepr.createMetaMethodUsage(myContext, methodName, methodOwner)); addClassUsage(TypeRepr.getType(myContext, Type.getReturnType(desc))); super.visitMethodInsn(opcode, owner, name, desc, itf); diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java index 3274a7ef896c..612adb06e27d 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java @@ -400,16 +400,31 @@ public class Mappings { private boolean hasOverriddenMethods(final ClassRepr fromClass, final MethodRepr.Predicate predicate) { for (int superName : fromClass.getSupers()) { - final ClassRepr superClass = reprByName(superName); - if (superClass == null) { - return true; // assumption + if (superName == myObjectClassName) { + continue; } - for (MethodRepr mm : superClass.findMethods(predicate)) { - if (isVisibleIn(superClass, mm, fromClass)) { + final ClassRepr superClass = reprByName(superName); + if (superClass != null) { + for (MethodRepr mm : superClass.findMethods(predicate)) { + if (isVisibleIn(superClass, mm, fromClass)) { + return true; + } + } + if (hasOverriddenMethods(superClass, predicate)) { return true; } } - if (hasOverriddenMethods(superClass, predicate)) { + } + return false; + } + + private boolean extendsLibraryClass(final ClassRepr fromClass) { + for (int superName : fromClass.getSupers()) { + if (superName != myObjectClassName) { + continue; + } + final ClassRepr superClass = reprByName(superName); + if (superClass == null || extendsLibraryClass(superClass)) { return true; } } @@ -440,6 +455,9 @@ public class Mappings { void addOverriddenFields(final FieldRepr f, final ClassRepr fromClass, final Collection> container) { for (int supername : fromClass.getSupers()) { + if (supername == myObjectClassName) { + continue; + } final ClassRepr superClass = reprByName(supername); if (superClass != null) { final FieldRepr ff = superClass.findField(f.name); @@ -455,6 +473,9 @@ public class Mappings { boolean hasOverriddenFields(final FieldRepr f, final ClassRepr fromClass) { for (int supername : fromClass.getSupers()) { + if (supername == myObjectClassName) { + continue; + } final ClassRepr superClass = reprByName(supername); if (superClass != null) { final FieldRepr ff = superClass.findField(f.name); @@ -534,15 +555,8 @@ public class Mappings { return Boolean.FALSE; } - boolean isMethodVisible(final int className, final MethodRepr m) { - final ClassRepr r = reprByName(className); - if (r != null) { - if (r.findMethods(MethodRepr.equalByJavaRules(m)).size() > 0) { - return true; - } - return hasOverriddenMethods(r, MethodRepr.equalByJavaRules(m)); - } - return false; + boolean isMethodVisible(final ClassRepr classRepr, final MethodRepr m) { + return classRepr.findMethods(MethodRepr.equalByJavaRules(m)).size() > 0 || hasOverriddenMethods(classRepr, MethodRepr.equalByJavaRules(m)); } boolean isFieldVisible(final int className, final FieldRepr field) { @@ -1082,10 +1096,7 @@ public class Mappings { } final ClassRepr oldIt = oldItRef.get(); - if (oldIt != null && myPresent.hasOverriddenMethods(oldIt, MethodRepr.equalByJavaRules(m))) { - - } - else { + if (oldIt == null || !myPresent.hasOverriddenMethods(oldIt, MethodRepr.equalByJavaRules(m))) { if (m.myArgumentTypes.length > 0) { propagated = myFuture.propagateMethodAccess(m, it.name); debug("Conservative case on overriding methods, affecting method usages"); @@ -1170,10 +1181,13 @@ public class Mappings { final Collection sourceFileNames = myClassToSourceFile.get(subClass); if (sourceFileNames != null && !myCompiledFiles.containsAll(sourceFileNames)) { final int outerClass = r.getOuterClassName(); - if (!isEmpty(outerClass) && myFuture.isMethodVisible(outerClass, m)) { - myAffectedFiles.addAll(sourceFileNames); - for (File sourceFileName : sourceFileNames) { - debug("Affecting file due to local overriding: ", sourceFileName); + if (!isEmpty(outerClass)) { + final ClassRepr outerClassRepr = myFuture.reprByName(outerClass); + if (outerClassRepr != null && (myFuture.isMethodVisible(outerClassRepr, m) || myFuture.extendsLibraryClass(outerClassRepr))) { + myAffectedFiles.addAll(sourceFileNames); + for (File sourceFileName : sourceFileNames) { + debug("Affecting file due to local overriding: ", sourceFileName); + } } } } diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/MethodRepr.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/MethodRepr.java index fea3d8a3d8d8..1338bca67e08 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/MethodRepr.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/MethodRepr.java @@ -112,8 +112,8 @@ class MethodRepr extends ProtoMember { public void updateClassUsages(final DependencyContext context, final int owner, final Set s) { myType.updateClassUsages(context, owner, s); - for (int i = 0; i < myArgumentTypes.length; i++) { - myArgumentTypes[i].updateClassUsages(context, owner, s); + for (final TypeRepr.AbstractType argType : myArgumentTypes) { + argType.updateClassUsages(context, owner, s); } if (myExceptions != null) { @@ -124,17 +124,17 @@ class MethodRepr extends ProtoMember { } public MethodRepr(final DependencyContext context, - final int a, - final int n, - final int s, - final String d, - final String[] e, - final Object value) { - super(a, s, n, TypeRepr.getType(context, Type.getReturnType(d)), value); + final int accessFlags, + final int name, + final int signature, + final String descriptor, + final String[] exceptions, + final Object defaultValue) { + super(accessFlags, signature, name, TypeRepr.getType(context, Type.getReturnType(descriptor)), defaultValue); Set typeCollection = - e != null ? new THashSet(e.length) : Collections.emptySet(); - myExceptions = (Set)TypeRepr.createClassType(context, e, typeCollection); - myArgumentTypes = TypeRepr.getType(context, Type.getArgumentTypes(d)); + exceptions != null ? new THashSet(exceptions.length) : Collections.emptySet(); + myExceptions = (Set)TypeRepr.createClassType(context, exceptions, typeCollection); + myArgumentTypes = TypeRepr.getType(context, Type.getArgumentTypes(descriptor)); } public MethodRepr(final DependencyContext context, final DataInput in) { @@ -216,7 +216,7 @@ class MethodRepr extends ProtoMember { } public UsageRepr.Usage createMetaUsage(final DependencyContext context, final int owner) { - return UsageRepr.createMetaMethodUsage(context, name, owner, getDescr(context)); + return UsageRepr.createMetaMethodUsage(context, name, owner); } @Override diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/UsageRepr.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/UsageRepr.java index e7b376980fae..d6d3ff799a7b 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/UsageRepr.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/UsageRepr.java @@ -278,52 +278,18 @@ class UsageRepr { } public static class MetaMethodUsage extends FMUsage { - private int myArity; - public MetaMethodUsage(final DependencyContext context, final int n, final int o, final String descr) { + public MetaMethodUsage(final int n, final int o) { super(n, o); - myArity = TypeRepr.getType(context, Type.getArgumentTypes(descr)).length; } public MetaMethodUsage(final DataInput in) { super(in); - try { - myArity = DataInputOutputUtil.readINT(in); - } - catch (IOException e) { - throw new BuildDataCorruptedException(e); - } } @Override public void save(final DataOutput out) { save(METAMETHOD_USAGE, out); - try { - DataInputOutputUtil.writeINT(out, myArity); - } - catch (IOException e) { - throw new BuildDataCorruptedException(e); - } - } - - @Override - public boolean equals(final Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - if (!super.equals(o)) return false; - - MetaMethodUsage that = (MetaMethodUsage)o; - - if (myArity != that.myArity) return false; - - return super.equals(o); - } - - @Override - public int hashCode() { - int result = super.hashCode(); - result = 31 * result + myArity; - return result; } @Override @@ -334,7 +300,6 @@ class UsageRepr { @Override public void toStream(DependencyContext context, PrintStream stream) { super.toStream(context, stream); - stream.println(" Arity: " + Integer.toString(myArity)); } } @@ -679,8 +644,8 @@ class UsageRepr { return context.getUsage(new MethodUsage(context, name, owner, descr)); } - public static Usage createMetaMethodUsage(final DependencyContext context, final int name, final int owner, final String descr) { - return context.getUsage(new MetaMethodUsage(context, name, owner, descr)); + public static Usage createMetaMethodUsage(final DependencyContext context, final int name, final int owner) { + return context.getUsage(new MetaMethodUsage(name, owner)); } public static Usage createClassUsage(final DependencyContext context, final int name) { diff --git a/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/BuildDataManager.java b/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/BuildDataManager.java index a6ed636e8d11..9a40498ec619 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/BuildDataManager.java +++ b/jps/jps-builders/src/org/jetbrains/jps/incremental/storage/BuildDataManager.java @@ -42,7 +42,7 @@ import java.util.concurrent.ConcurrentMap; * Date: 10/7/11 */ public class BuildDataManager implements StorageOwner { - private static final int VERSION = 25; + private static final int VERSION = 26; private static final Logger LOG = Logger.getInstance("#org.jetbrains.jps.incremental.storage.BuildDataManager"); private static final String SRC_TO_FORM_STORAGE = "src-form"; private static final String OUT_TARGET_STORAGE = "out-target"; diff --git a/jps/jps-builders/testSrc/org/jetbrains/ether/MemberChangeTest.java b/jps/jps-builders/testSrc/org/jetbrains/ether/MemberChangeTest.java index 90ecb820bbc6..3866756a0929 100644 --- a/jps/jps-builders/testSrc/org/jetbrains/ether/MemberChangeTest.java +++ b/jps/jps-builders/testSrc/org/jetbrains/ether/MemberChangeTest.java @@ -203,4 +203,8 @@ public class MemberChangeTest extends IncrementalTestCase { public void testAddMethod() { doTest(); } + + public void testAddVarargMethod() { + doTest(); + } }