Skip to content

Commit

Permalink
Fix compile classpath snapshotting ignoring constants
Browse files Browse the repository at this point in the history
This commit fixes the ABI extractor, which ignored the value of constants. This was bad because if a constant value
changed, we wouldn't trigger recompilation of consumers (independently of incremental compilation). Since the compiler
can inline those values, incremental compilation wouldn't catch the problem either.

Fixes: #1476
  • Loading branch information
melix committed Feb 28, 2017
1 parent ec9f6f9 commit d157fb7
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 4 deletions.
Expand Up @@ -83,7 +83,7 @@ public void visitEnd() {
}
for (FieldMember field : fields) {
FieldVisitor fieldVisitor = apiMemberAdapter.visitField(
field.getAccess(), field.getName(), field.getTypeDesc(), field.getSignature(), null);
field.getAccess(), field.getName(), field.getTypeDesc(), field.getSignature(), field.getValue());
visitAnnotationMembers(fieldVisitor, field.getAnnotations());
fieldVisitor.visitEnd();
}
Expand Down Expand Up @@ -188,7 +188,8 @@ public AnnotationVisitor visitParameterAnnotation(int parameter, String desc, bo
@Override
public FieldVisitor visitField(int access, String name, String desc, String signature, Object value) {
if (isCandidateApiMember(access, apiIncludesPackagePrivateMembers)) {
final FieldMember fieldMember = new FieldMember(access, name, signature, desc);
Object keepValue = (access & ACC_STATIC) == ACC_STATIC && ((access & ACC_FINAL) == ACC_FINAL) ? value : null;
final FieldMember fieldMember = new FieldMember(access, name, signature, desc, keepValue);
fields.add(fieldMember);
return new FieldVisitor(ASM5) {
@Override
Expand Down
Expand Up @@ -16,24 +16,32 @@

package org.gradle.api.internal.tasks.compile;

import com.google.common.collect.Ordering;
import org.objectweb.asm.Type;

import java.lang.reflect.Modifier;

public class FieldMember extends TypedMember implements Comparable<FieldMember> {

public FieldMember(int access, String name, String signature, String typeDesc) {
private final Object value;

public FieldMember(int access, String name, String signature, String typeDesc, Object value) {
super(access, name, signature, typeDesc);
this.value = value;
}

@Override
public int compareTo(FieldMember o) {
return super.compare(o).result();
return super.compare(o).compare(value, o.value, Ordering.arbitrary()).result();
}

@Override
public String toString() {
return String.format(
"%s %s %s", Modifier.toString(getAccess()), Type.getType(getTypeDesc()).getClassName(), getName());
}

public Object getValue() {
return value;
}
}
Expand Up @@ -20,7 +20,9 @@ package org.gradle.java.compile.incremental
import groovy.transform.NotYetImplemented
import org.gradle.integtests.fixtures.AbstractIntegrationSpec
import org.gradle.integtests.fixtures.CompilationOutputsFixture
import org.gradle.util.TestPrecondition
import spock.lang.Issue
import spock.lang.Requires
import spock.lang.Unroll

abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extends AbstractIntegrationSpec {
Expand Down Expand Up @@ -650,6 +652,7 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
}
@Issue("gradle/gradle#1474")
@Requires({TestPrecondition.JDK8_OR_LATER}) // todo: also fix for JDK < 8
def "recompiles dependent class in case a constant is computed from another constant"() {
java api: ["class A { public static final int FOO = 10; }"], impl: ['class B { public static final int BAR = 2 + A.FOO; } ']
impl.snapshot { run 'compileJava' }
Expand Down
Expand Up @@ -385,6 +385,42 @@ public class ToolImpl {
executedAndNotSkipped ':b:compileJava'
}

def "recompiles when constant value of API changes"() {
given:
buildFile << """
project(':b') {
dependencies {
compile project(':a')
}
}
"""
def sourceFile = file("a/src/main/java/ToolImpl.java")
sourceFile << """
public class ToolImpl { public static final int CONST = 1; }
"""
file("b/src/main/java/Main.java") << """
public class Main { public static final int CONST2 = 1 + ToolImpl.CONST; }
"""

when:
succeeds ':b:compileJava'

then:
executedAndNotSkipped ':a:compileJava'
executedAndNotSkipped ':b:compileJava'

when:
// change to constant value
sourceFile.text = """
public class ToolImpl { public static final int CONST = 10; }
"""

then:
succeeds ':b:compileJava'
executedAndNotSkipped ':a:compileJava'
executedAndNotSkipped ':b:compileJava'
}

def "recompiles when generic type signatures of implementation class changes"() {
given:
buildFile << """
Expand Down

0 comments on commit d157fb7

Please sign in to comment.