Fix the bug about marking expressions as read early
@{obj4.text}
@{obj4.useHello ? obj4.text : `hello`}
This case was broken and would not re-read obj4.text if
only obj4.useHello is invalidated. It was partially fixed in
Change-Id: Id449c8819b8dc0301a7ab893002478914780d480 but
but it was bringing it down to exact equality which would
mean we could fail to mark sth as read.
The coverage logic we use in expressions when marking them
as read was giving false positives, which results in
marking expressions as read before they are fully read.
This CL fixes that bug. The safe fix introduces some false
negatives when a conditional is behind another conditional.
We can address this post v1.
There was also another bug about setting conditional flags
even though the ternary does not need to be calculated.
@{obja.boolMethod(a)}
@{a ? objb.boolMethod(b) : objc.boolMethod(c)}
When obja is invalidated, it would re calculate the second
binding expression too even though it is never used (because
that expression is not invalidated). The re-calculation would
happen because we would calculate the value of `a` and set
the conditional flags w/o checking invalidation.
This would result in unnecessary calculations. I've also fixed
it for first degree where the ternary is not under another
ternary. The proper fix would requires bigger effort, post V1.
bug: 22957203
Change-Id: Ib73f31eac358f2ad7652395a021baaa93b79adf7
diff --git a/compiler/src/main/java/android/databinding/tool/expr/Expr.java b/compiler/src/main/java/android/databinding/tool/expr/Expr.java
index a562f4e..fc7129c 100644
--- a/compiler/src/main/java/android/databinding/tool/expr/Expr.java
+++ b/compiler/src/main/java/android/databinding/tool/expr/Expr.java
@@ -210,7 +210,6 @@
}
for (Dependency dependency : getDependants()) {
- // first traverse non-conditionals because we'll avoid adding conditionals if we are get because of these anyways
if (dependency.getCondition() == null) {
bitSet.or(dependency.getDependant().getShouldReadFlagsWithConditionals());
} else {
@@ -474,6 +473,7 @@
clone.andNot(mReadSoFar);
mRead = clone.isEmpty();
+
if (!mRead && !mReadSoFar.isEmpty()) {
// check if remaining dependencies can be satisfied w/ existing values
// for predicate flags, this expr may already be calculated to get the predicate
@@ -481,25 +481,29 @@
// them. If any of them is completely covered w/ our non-conditional flags, no reason
// to add them to the list since we'll already be calculated due to our non-conditional
// flags
-
+ boolean allCovered = true;
for (int i = clone.nextSetBit(0); i != -1; i = clone.nextSetBit(i + 1)) {
final Expr expr = mModel.findFlagExpression(i);
- if (expr == null || !expr.isConditional()) {
+ if (expr == null) {
continue;
}
- final BitSet readForConditional = expr.findConditionalFlags();
+ if (!expr.isConditional()) {
+ allCovered = false;
+ break;
+ }
+ final BitSet readForConditional = (BitSet) expr.findConditionalFlags().clone();
+
+ // FIXME: this does not do full traversal so misses some cases
// to calculate that conditional, i should've read /readForConditional/ flags
- // if my read-so-far bits has any common w/ that; that means i would've already
+ // if my read-so-far bits cover that; that means i would've already
// read myself
- clone.andNot(readForConditional);
- final BitSet invalidFlags = (BitSet) getInvalidFlags().clone();
- invalidFlags.xor(readForConditional);
- mRead = invalidFlags.isEmpty() || clone.isEmpty();
- if (mRead) {
+ readForConditional.andNot(mReadSoFar);
+ if (!readForConditional.isEmpty()) {
+ allCovered = false;
break;
}
}
-
+ mRead = allCovered;
}
if (mRead) {
mShouldReadFlags = null; // if we've been marked as read, clear should read flags
@@ -543,10 +547,12 @@
private Node mCalculationPaths = null;
+ /**
+ * All flag paths that will result in calculation of this expression.
+ */
protected Node getAllCalculationPaths() {
if (mCalculationPaths == null) {
Node node = new Node();
- // TODO distant parent w/ conditionals are still not traversed :/
if (isConditional()) {
node.mBitSet.or(getPredicateInvalidFlags());
} else {
@@ -559,6 +565,7 @@
cond.setConditionFlag(
dependant.getRequirementFlagIndex(dependency.getExpectedOutput()));
cond.mParents.add(dependant.getAllCalculationPaths());
+ node.mParents.add(cond);
} else {
node.mParents.add(dependant.getAllCalculationPaths());
}
@@ -674,19 +681,24 @@
public boolean areAllPathsSatisfied(BitSet readSoFar) {
if (mConditionFlag != -1) {
- return readSoFar.get(mConditionFlag) || mParents.get(0)
- .areAllPathsSatisfied(readSoFar);
+ return readSoFar.get(mConditionFlag)
+ || mParents.get(0).areAllPathsSatisfied(readSoFar);
} else {
- final BitSet clone = (BitSet) readSoFar.clone();
- clone.and(mBitSet);
- if (!clone.isEmpty()) {
- return true;
- }
- if (mParents.isEmpty()) {
+ final BitSet myBitsClone = (BitSet) mBitSet.clone();
+ myBitsClone.andNot(readSoFar);
+ if (!myBitsClone.isEmpty()) {
+ // read so far does not cover all of my invalidation. The only way I could be
+ // covered is that I only have 1 conditional dependent which is covered by this.
+ if (mParents.size() == 1 && mParents.get(0).mConditionFlag != -1) {
+ return mParents.get(0).areAllPathsSatisfied(readSoFar);
+ }
return false;
}
+ if (mParents.isEmpty()) {
+ return true;
+ }
for (Node parent : mParents) {
- if (!parent.areAllPathsSatisfied(clone)) {
+ if (!parent.areAllPathsSatisfied(readSoFar)) {
return false;
}
}
diff --git a/compiler/src/main/java/android/databinding/tool/expr/ExprModel.java b/compiler/src/main/java/android/databinding/tool/expr/ExprModel.java
index a4eb1a8..57d19cf 100644
--- a/compiler/src/main/java/android/databinding/tool/expr/ExprModel.java
+++ b/compiler/src/main/java/android/databinding/tool/expr/ExprModel.java
@@ -53,6 +53,7 @@
* Any expression can be invalidated by invalidating this flag.
*/
private BitSet mInvalidateAnyFlags;
+ private int mInvalidateAnyFlagIndex;
/**
* Used by code generation. Keeps the list of expressions that are waiting to be evaluated.
@@ -356,7 +357,7 @@
for (Expr expr : mExprMap.values()) {
expr.getDependencies();
}
- final int invalidateAnyFlagIndex = counter ++;
+ mInvalidateAnyFlagIndex = counter ++;
flagMapping.add("INVALIDATE ANY");
mInvalidateableFieldLimit = counter;
mInvalidateableFlags = new BitSet();
@@ -393,7 +394,7 @@
mFlagBucketCount = 1 + (getTotalFlagCount() / FlagSet.sBucketSize);
mInvalidateAnyFlags = new BitSet();
- mInvalidateAnyFlags.set(invalidateAnyFlagIndex, true);
+ mInvalidateAnyFlags.set(mInvalidateAnyFlagIndex, true);
for (Expr expr : mExprMap.values()) {
expr.getShouldReadFlagsWithConditionals();
@@ -515,14 +516,15 @@
}
}
for (Expr partialRead : markedSomeFlagsAsRead) {
- boolean allPathsAreSatisfied = partialRead.getAllCalculationPaths()
- .areAllPathsSatisfied(partialRead.mReadSoFar);
- if (!allPathsAreSatisfied) {
- continue;
- }
+ // even if all paths are not satisfied, we can elevate certain conditional dependencies
+ // if all of their paths are satisfied.
for (Dependency dependency : partialRead.getDependants()) {
- if (dependency.getDependant().considerElevatingConditionals(partialRead)) {
- elevated = true;
+ Expr dependant = dependency.getDependant();
+ if (dependant.isConditional() && dependant.getAllCalculationPaths()
+ .areAllPathsSatisfied(partialRead.mReadSoFar)) {
+ if (dependant.considerElevatingConditionals(partialRead)) {
+ elevated = true;
+ }
}
}
}
@@ -593,6 +595,10 @@
return mInvalidateAnyFlags;
}
+ public int getInvalidateAnyFlagIndex() {
+ return mInvalidateAnyFlagIndex;
+ }
+
public Expr argListExpr(Iterable<Expr> expressions) {
return register(new ArgListExpr(mArgListIdCounter ++, expressions));
}
diff --git a/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt b/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt
index 47a44f5..55b858e 100644
--- a/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt
+++ b/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt
@@ -192,6 +192,10 @@
FlagSet(expr.getShouldReadFlags(), expr.getModel().getFlagBucketCount())
}
+val Expr.shouldReadWithConditionalsFlagSet by Delegates.versionedLazy { expr : Expr ->
+ FlagSet(expr.getShouldReadFlagsWithConditionals(), expr.getModel().getFlagBucketCount())
+}
+
val Expr.conditionalFlags by Delegates.lazy { expr : Expr ->
arrayListOf(FlagSet(expr.getRequirementFlagIndex(false)),
FlagSet(expr.getRequirementFlagIndex(true)))
@@ -868,31 +872,66 @@
L.d("flag set:%s . inherited flags: %s. need another if: %s", flagSet, inheritedFlags, needsIfWrapper);
// if I am the condition for an expression, set its flag
- val conditionals = expr.getDependants().filter {
+ expr.getDependants().filter {
!it.isConditional() && it.getDependant() is TernaryExpr &&
(it.getDependant() as TernaryExpr).getPred() == expr
- }.map { it.getDependant() }
- if (conditionals.isNotEmpty()) {
- tab("// setting conditional flags")
- tab("if (${expr.executePendingLocalName}) {") {
- conditionals.forEach {
- val set = it.getRequirementFlagSet(true)
- mDirtyFlags.mapOr(set) { suffix, index ->
- tab("${tmpDirtyFlags.localValue(index)} |= ${set.localValue(index)};")
+ }.map { it.getDependant() }.groupBy {
+ // group by when those ternaries will be evaluated (e.g. don't set conditional flags for no reason)
+ val ternaryBitSet = it.getShouldReadFlagsWithConditionals()
+ val isBehindTernary = ternaryBitSet.nextSetBit(model.getInvalidateAnyFlagIndex()) == -1
+ if (!isBehindTernary) {
+ val ternaryFlags = it.shouldReadWithConditionalsFlagSet
+ "if(${tmpDirtyFlags.mapOr(ternaryFlags){ suffix, index ->
+ "(${tmpDirtyFlags.localValue(index)} & ${ternaryFlags.localValue(index)}) != 0"
+ }.joinToString(" || ")}) {"
+ } else {
+ // TODO if it is behind a ternary, we should set it when its predicate is elevated
+ // Normally, this would mean that there is another code path to re-read our current expression.
+ // Unfortunately, this may not be true due to the coverage detection in `expr#markAsReadIfDone`, this may never happen.
+ // for v1.0, we'll go with always setting it and suffering an unnecessary calculation for this edge case.
+ // we can solve this by listening to elevation events from the model.
+ ""
+ }
+ }.forEach {
+ val hasAnotherIf = it.key != ""
+ if (hasAnotherIf) {
+ tab(it.key) {
+ tab("if (${expr.executePendingLocalName}) {") {
+ it.value.forEach {
+ val set = it.getRequirementFlagSet(true)
+ mDirtyFlags.mapOr(set) { suffix, index ->
+ tab("${tmpDirtyFlags.localValue(index)} |= ${set.localValue(index)};")
+ }
+ }
+ }
+ tab("} else {") {
+ it.value.forEach {
+ val set = it.getRequirementFlagSet(false)
+ mDirtyFlags.mapOr(set) { suffix, index ->
+ tab("${tmpDirtyFlags.localValue(index)} |= ${set.localValue(index)};")
+ }
+ }
+ }.tab("}")
+ }.app("}")
+ } else {
+ tab("if (${expr.executePendingLocalName}) {") {
+ it.value.forEach {
+ val set = it.getRequirementFlagSet(true)
+ mDirtyFlags.mapOr(set) { suffix, index ->
+ tab("${tmpDirtyFlags.localValue(index)} |= ${set.localValue(index)};")
+ }
}
}
- }
- tab("} else {") {
- conditionals.forEach {
- val set = it.getRequirementFlagSet(false)
- mDirtyFlags.mapOr(set) { suffix, index ->
- tab("${tmpDirtyFlags.localValue(index)} |= ${set.localValue(index)};")
+ tab("} else {") {
+ it.value.forEach {
+ val set = it.getRequirementFlagSet(false)
+ mDirtyFlags.mapOr(set) { suffix, index ->
+ tab("${tmpDirtyFlags.localValue(index)} |= ${set.localValue(index)};")
+ }
}
- }
+ } app("}")
}
- tab("}")
}
-
val chosen = expr.getDependants().filter {
val dependant = it.getDependant()
batch.contains(dependant) &&
diff --git a/compiler/src/test/java/android/databinding/tool/LayoutBinderTest.java b/compiler/src/test/java/android/databinding/tool/LayoutBinderTest.java
index 042b563..2b4d2a3 100644
--- a/compiler/src/test/java/android/databinding/tool/LayoutBinderTest.java
+++ b/compiler/src/test/java/android/databinding/tool/LayoutBinderTest.java
@@ -34,7 +34,7 @@
import static org.junit.Assert.assertTrue;
public class LayoutBinderTest {
- LayoutBinder mLayoutBinder;
+ MockLayoutBinder mLayoutBinder;
ExprModel mExprModel;
@Before
public void setUp() throws Exception {
diff --git a/compiler/src/test/java/android/databinding/tool/MockLayoutBinder.java b/compiler/src/test/java/android/databinding/tool/MockLayoutBinder.java
index 3380ffa..0807e7c 100644
--- a/compiler/src/test/java/android/databinding/tool/MockLayoutBinder.java
+++ b/compiler/src/test/java/android/databinding/tool/MockLayoutBinder.java
@@ -13,6 +13,8 @@
package android.databinding.tool;
+import android.databinding.tool.expr.IdentifierExpr;
+import android.databinding.tool.store.Location;
import android.databinding.tool.store.ResourceBundle;
import java.io.File;
@@ -24,4 +26,8 @@
"com.test.submodule",
false));
}
+
+ public IdentifierExpr addVariable(String name, String type, Location location) {
+ return super.addVariable(name, type, location, true);
+ }
}
diff --git a/compiler/src/test/java/android/databinding/tool/expr/ExprModelTest.java b/compiler/src/test/java/android/databinding/tool/expr/ExprModelTest.java
index ef45b75..d0ee0d6 100644
--- a/compiler/src/test/java/android/databinding/tool/expr/ExprModelTest.java
+++ b/compiler/src/test/java/android/databinding/tool/expr/ExprModelTest.java
@@ -138,7 +138,7 @@
@Test
public void testShouldRead() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", "java.lang.String", null);
IdentifierExpr b = lb.addVariable("b", "java.lang.String", null);
@@ -160,7 +160,7 @@
@Test
public void testTernaryWithPlus() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr user = lb
.addVariable("user", "android.databinding.tool.expr.ExprModelTest.User",
@@ -215,7 +215,7 @@
@Test
public void testTernaryInsideTernary() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr cond1 = lb.addVariable("cond1", "boolean", null);
IdentifierExpr cond2 = lb.addVariable("cond2", "boolean", null);
@@ -260,7 +260,7 @@
@Test
public void testRequirementFlags() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", "java.lang.String", null);
IdentifierExpr b = lb.addVariable("b", "java.lang.String", null);
@@ -332,7 +332,7 @@
@Test
public void testPostConditionalDependencies() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr u1 = lb.addVariable("u1", User.class.getCanonicalName(), null);
@@ -396,24 +396,26 @@
assertTrue(mExprModel.markBitsRead());
shouldRead = getShouldRead();
- // actually, there is no real case to read u1 anymore because if b>c was not true,
+ // FIXME: there is no real case to read u1 anymore because if b>c was not true,
// u1.getCond(d) will never be set. Right now, we don't have mechanism to figure this out
// and also it does not affect correctness (just an unnecessary if stmt)
assertExactMatch(shouldRead, u1, u2, u1LastName, u2LastName, bcTernary.getIfTrue(), bcTernary);
firstRead = getReadFirst(shouldRead);
assertExactMatch(firstRead, u1, u2);
-
+ assertFlags(u1, bcTernary.getIfTrue().getRequirementFlagIndex(true));
+ assertFlags(u2, bcTernary.getIfTrue().getRequirementFlagIndex(false));
assertFlags(u1LastName, bcTernary.getIfTrue().getRequirementFlagIndex(true));
assertFlags(u2LastName, bcTernary.getIfTrue().getRequirementFlagIndex(false));
- assertFlags(u2, bcTernary.getIfTrue().getRequirementFlagIndex(false));
assertFlags(bcTernary.getIfTrue(), bcTernary.getRequirementFlagIndex(true));
assertFlags(bcTernary, b, c, u1, u2, d, u1LastName, u2LastName, e);
+
+ assertFalse(mExprModel.markBitsRead());
}
@Test
public void testCircularDependency() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", int.class.getCanonicalName(),
null);
@@ -431,7 +433,7 @@
@Test
public void testNestedCircularDependency() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", int.class.getCanonicalName(),
null);
@@ -454,23 +456,24 @@
@Test
public void testInterExprDependency() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr u = lb.addVariable("u", User.class.getCanonicalName(),
null);
final Expr uComment = parse(lb, "u.comment", FieldAccessExpr.class);
final TernaryExpr uTernary = parse(lb, "u.useComment ? u.comment : `xx`", TernaryExpr.class);
mExprModel.seal();
+ assertTrue(uTernary.getPred().canBeInvalidated());
List<Expr> shouldRead = getShouldRead();
assertExactMatch(shouldRead, u, uComment, uTernary.getPred());
assertTrue(mExprModel.markBitsRead());
shouldRead = getShouldRead();
- assertExactMatch(shouldRead, u, uComment, uTernary);
+ assertExactMatch(shouldRead, uComment, uTernary);
}
@Test
public void testInterExprCircularDependency() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", int.class.getCanonicalName(),
null);
@@ -488,7 +491,7 @@
@Test
public void testInterExprCircularDependency2() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", boolean.class.getCanonicalName(),
null);
@@ -499,20 +502,15 @@
mExprModel.seal();
List<Expr> shouldRead = getShouldRead();
assertExactMatch(shouldRead, a, b);
+ assertFlags(a, a, b);
+ assertFlags(b, a, b);
List<Expr> readFirst = getReadFirst(shouldRead);
assertExactMatch(readFirst, a, b);
assertTrue(mExprModel.markBitsRead());
shouldRead = getShouldRead();
- // read a and b again, this time for their dependencies and also the rest since everything
- // is ready to be read
- assertExactMatch(shouldRead, a, b, abTernary, baTernary);
- List<Expr> justRead = new ArrayList<Expr>();
+ assertExactMatch(shouldRead, abTernary, baTernary);
readFirst = getReadFirst(shouldRead);
- assertExactMatch(readFirst, a, b);
- Collections.addAll(justRead, a, b);
- readFirst = filterOut(getReadFirst(shouldRead, justRead), justRead);
assertExactMatch(readFirst, abTernary, baTernary);
-
assertFalse(mExprModel.markBitsRead());
shouldRead = getShouldRead();
assertEquals(0, shouldRead.size());
@@ -520,7 +518,7 @@
@Test
public void testInterExprCircularDependency3() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", boolean.class.getCanonicalName(),
null);
@@ -537,7 +535,7 @@
shouldRead = getShouldRead();
// read a and b again, this time for their dependencies and also the rest since everything
// is ready to be read
- assertExactMatch(shouldRead, a, b, c, abTernary, abTernary2);
+ assertExactMatch(shouldRead, c, abTernary, abTernary2);
mExprModel.markBitsRead();
shouldRead = getShouldRead();
assertEquals(0, shouldRead.size());
@@ -545,7 +543,7 @@
@Test
public void testInterExprCircularDependency4() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", boolean.class.getCanonicalName(),
null);
@@ -560,6 +558,8 @@
final TernaryExpr baTernary = parse(lb, "b ? a : false", TernaryExpr.class);
mExprModel.seal();
List<Expr> shouldRead = getShouldRead();
+ // check if a,b or c should be read. these are easily calculated from binding expressions'
+ // invalidation
assertExactMatch(shouldRead, c, a, b);
List<Expr> justRead = new ArrayList<Expr>();
@@ -569,16 +569,36 @@
assertEquals(0, filterOut(getReadFirst(shouldRead, justRead), justRead).size());
assertTrue(mExprModel.markBitsRead());
shouldRead = getShouldRead();
- assertExactMatch(shouldRead, a, b, d, cTernary.getIfTrue(), cTernary, abTernary, baTernary);
+ // if a and b are not invalid, a won't be read in the first step. But if c's expression
+ // is invalid and c == true, a must be read. Depending on a, d might be read as well.
+ // don't need to read b anymore because `a ? b : true` and `b ? a : false` has the same
+ // invalidation flags.
+ assertExactMatch(shouldRead, a, abTernary, baTernary);
justRead.clear();
readFirst = getReadFirst(shouldRead);
- assertExactMatch(readFirst, a, b, d);
- Collections.addAll(justRead, a, b, d);
+ // first must read `a`.
+ assertExactMatch(readFirst, a);
+ Collections.addAll(justRead, a);
readFirst = filterOut(getReadFirst(shouldRead, justRead), justRead);
- assertExactMatch(readFirst, cTernary.getIfTrue(), abTernary, baTernary);
- Collections.addAll(justRead, cTernary.getIfTrue(), abTernary, baTernary);
+ assertExactMatch(readFirst, abTernary, baTernary);
+ Collections.addAll(justRead, abTernary, baTernary);
+
+ readFirst = filterOut(getReadFirst(shouldRead, justRead), justRead);
+ assertEquals(0, filterOut(getReadFirst(shouldRead, justRead), justRead).size());
+ assertTrue(mExprModel.markBitsRead());
+
+ shouldRead = getShouldRead();
+ // now we can read adf ternary and c ternary
+ justRead.clear();
+ assertExactMatch(shouldRead, d, cTernary.getIfTrue(), cTernary);
+ readFirst = getReadFirst(shouldRead);
+ assertExactMatch(readFirst, d);
+ Collections.addAll(justRead, d);
+ readFirst = filterOut(getReadFirst(shouldRead, justRead), justRead);
+ assertExactMatch(readFirst, cTernary.getIfTrue());
+ Collections.addAll(justRead, cTernary.getIfTrue());
readFirst = filterOut(getReadFirst(shouldRead, justRead), justRead);
assertExactMatch(readFirst, cTernary);
@@ -590,8 +610,33 @@
}
@Test
+ public void testInterExprDeepDependency() {
+ MockLayoutBinder lb = new MockLayoutBinder();
+ mExprModel = lb.getModel();
+ IdentifierExpr a = lb.addVariable("a", boolean.class.getCanonicalName(), null);
+ IdentifierExpr b = lb.addVariable("b", boolean.class.getCanonicalName(), null);
+ IdentifierExpr c = lb.addVariable("c", boolean.class.getCanonicalName(), null);
+ final TernaryExpr t1 = parse(lb, "c ? (a ? b : true) : false", TernaryExpr.class);
+ final TernaryExpr t2 = parse(lb, "c ? (b ? a : false) : true", TernaryExpr.class);
+ final TernaryExpr abTernary = (TernaryExpr) t1.getIfTrue();
+ final TernaryExpr baTernary = (TernaryExpr) t2.getIfTrue();
+ mExprModel.seal();
+ List<Expr> shouldRead = getShouldRead();
+ assertExactMatch(shouldRead, c);
+ assertTrue(mExprModel.markBitsRead());
+ shouldRead = getShouldRead();
+ assertExactMatch(shouldRead, a, b);
+ assertTrue(mExprModel.markBitsRead());
+ shouldRead = getShouldRead();
+ assertExactMatch(shouldRead, a, b, t1.getIfTrue(), t2.getIfTrue(), t1, t2);
+ assertFlags(b, abTernary.getRequirementFlagIndex(true));
+ assertFlags(a, baTernary.getRequirementFlagIndex(true));
+ assertFalse(mExprModel.markBitsRead());
+ }
+
+ @Test
public void testInterExprDependencyNotReadyYet() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr a = lb.addVariable("a", boolean.class.getCanonicalName(), null);
IdentifierExpr b = lb.addVariable("b", boolean.class.getCanonicalName(), null);
@@ -615,7 +660,7 @@
@Test
public void testNoFlagsForNonBindingStatic() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
lb.addVariable("a", int.class.getCanonicalName(), null);
final MathExpr parsed = parse(lb, "a * (3 + 2)", MathExpr.class);
@@ -630,7 +675,7 @@
@Test
public void testFlagsForBindingStatic() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
lb.addVariable("a", int.class.getCanonicalName(), null);
final Expr staticParsed = parse(lb, "3 + 2", MathExpr.class);
@@ -648,7 +693,7 @@
@Test
public void testFinalFieldOfAVariable() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
IdentifierExpr user = lb.addVariable("user", User.class.getCanonicalName(),
null);
@@ -664,7 +709,7 @@
@Test
public void testFinalFieldOfAField() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
lb.addVariable("user", User.class.getCanonicalName(), null);
Expr finalFieldGet = parse(lb, "user.subObj.finalField", FieldAccessExpr.class);
@@ -683,7 +728,7 @@
@Test
public void testFinalFieldOfAMethod() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
lb.addVariable("user", User.class.getCanonicalName(), null);
Expr finalFieldGet = parse(lb, "user.anotherSubObj.finalField", FieldAccessExpr.class);
@@ -702,7 +747,7 @@
@Test
public void testFinalOfAClass() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
mExprModel.addImport("View", "android.view.View", null);
FieldAccessExpr fieldAccess = parse(lb, "View.VISIBLE", FieldAccessExpr.class);
@@ -713,7 +758,7 @@
@Test
public void testStaticFieldOfInstance() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
lb.addVariable("myView", "android.view.View", null);
FieldAccessExpr fieldAccess = parse(lb, "myView.VISIBLE", FieldAccessExpr.class);
@@ -730,7 +775,7 @@
@Test
public void testOnDemandImportConflict() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
final IdentifierExpr myView = lb.addVariable("u", "android.view.View",
null);
@@ -745,7 +790,7 @@
@Test
public void testOnDemandImportAlreadyImported() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
final StaticIdentifierExpr ux = mExprModel.addImport("UX", User.class.getCanonicalName(),
null);
@@ -759,7 +804,7 @@
@Test
public void testStaticMethodOfInstance() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
lb.addVariable("user", User.class.getCanonicalName(), null);
MethodCallExpr methodCall = parse(lb, "user.ourStaticMethod()", MethodCallExpr.class);
@@ -773,7 +818,7 @@
@Test
public void testFinalOfStaticField() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
mExprModel.addImport("UX", User.class.getCanonicalName(), null);
FieldAccessExpr fieldAccess = parse(lb, "UX.innerStaticInstance.finalStaticField",
@@ -786,7 +831,7 @@
@Test
public void testFinalOfFinalStaticField() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
mExprModel.addImport("User", User.class.getCanonicalName(), null);
FieldAccessExpr fieldAccess = parse(lb, "User.innerFinalStaticInstance.finalStaticField",
@@ -798,7 +843,7 @@
@Test
public void testLocationTracking() {
- LayoutBinder lb = new MockLayoutBinder();
+ MockLayoutBinder lb = new MockLayoutBinder();
mExprModel = lb.getModel();
final String input = "a > 3 ? b : c";
TernaryExpr ternaryExpr = parse(lb, input, TernaryExpr.class);
@@ -849,7 +894,7 @@
// TODO uncomment when we have inner static access
// @Test
// public void testFinalOfInnerStaticClass() {
-// LayoutBinder lb = new MockLayoutBinder();
+// MockLayoutBinder lb = new MockLayoutBinder();
// mExprModel = lb.getModel();
// mExprModel.addImport("User", User.class.getCanonicalName());
// FieldAccessExpr fieldAccess = parse(lb, "User.InnerStaticClass.finalStaticField", FieldAccessExpr.class);
@@ -884,7 +929,9 @@
private void assertExactMatch(List<Expr> iterable, Expr... exprs) {
int i = 0;
- String log = Arrays.toString(iterable.toArray());
+ String listLog = Arrays.toString(iterable.toArray());
+ String itemsLog = Arrays.toString(exprs);
+ String log = "list: " + listLog + "\nitems: " + itemsLog;
log("list", iterable);
for (Expr expr : exprs) {
assertTrue((i++) + ":must contain " + expr.getUniqueKey() + "\n" + log,
@@ -892,7 +939,7 @@
}
i = 0;
for (Expr expr : iterable) {
- assertTrue((i++) + ":must be expected " + expr.getUniqueKey(),
+ assertTrue((i++) + ":must be expected " + expr.getUniqueKey() + "\n" + log,
ArrayUtils.contains(exprs, expr));
}
}
diff --git a/compiler/src/test/java/android/databinding/tool/reflection/java/JavaAnalyzer.java b/compiler/src/test/java/android/databinding/tool/reflection/java/JavaAnalyzer.java
index dea092b..ee2b0e5 100644
--- a/compiler/src/test/java/android/databinding/tool/reflection/java/JavaAnalyzer.java
+++ b/compiler/src/test/java/android/databinding/tool/reflection/java/JavaAnalyzer.java
@@ -13,13 +13,19 @@
package android.databinding.tool.reflection.java;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
+
import android.databinding.tool.reflection.ModelAnalyzer;
import android.databinding.tool.reflection.ModelClass;
import android.databinding.tool.reflection.SdkUtil;
import android.databinding.tool.reflection.TypeUtil;
import android.databinding.tool.util.L;
+import android.databinding.tool.util.Preconditions;
import java.io.File;
+import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLClassLoader;
@@ -124,13 +130,40 @@
}
}
- public static void initForTests() {
+ private static String loadAndroidHome() {
Map<String, String> env = System.getenv();
for (Map.Entry<String, String> entry : env.entrySet()) {
L.d("%s %s", entry.getKey(), entry.getValue());
}
- String androidHome = env.get("ANDROID_HOME");
- if (androidHome == null) {
+ if(env.containsKey("ANDROID_HOME")) {
+ return env.get("ANDROID_HOME");
+ }
+ // check for local.properties file
+ File folder = new File(".").getAbsoluteFile();
+ while (folder != null && folder.exists()) {
+ File f = new File(folder, "local.properties");
+ if (f.exists() && f.canRead()) {
+ try {
+ for (String line : FileUtils.readLines(f)) {
+ String[] keyValue = StringUtils.split(line, '=');
+ if (keyValue.length == 2) {
+ String key = keyValue[0].trim();
+ if (key.equals("sdk.dir")) {
+ return keyValue[1].trim();
+ }
+ }
+ }
+ } catch (IOException ignored) {}
+ }
+ folder = folder.getParentFile();
+ }
+
+ return null;
+ }
+
+ public static void initForTests() {
+ String androidHome = loadAndroidHome();
+ if (StringUtils.isEmpty(androidHome) || !new File(androidHome).exists()) {
throw new IllegalStateException(
"you need to have ANDROID_HOME set in your environment"
+ " to run compiler tests");
diff --git a/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/UnnecessaryCalculationTest.java b/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/UnnecessaryCalculationTest.java
new file mode 100644
index 0000000..986156c
--- /dev/null
+++ b/integration-tests/TestApp/app/src/androidTestApi7/java/android/databinding/testapp/UnnecessaryCalculationTest.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package android.databinding.testapp;
+
+import android.databinding.testapp.databinding.UnnecessaryCalculationBinding;
+import android.databinding.testapp.vo.BasicObject;
+import android.support.annotation.UiThread;
+import android.test.UiThreadTest;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class UnnecessaryCalculationTest extends BaseDataBinderTest<UnnecessaryCalculationBinding> {
+
+ public UnnecessaryCalculationTest() {
+ super(UnnecessaryCalculationBinding.class);
+ }
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ initBinder();
+ }
+
+ @UiThreadTest
+ public void testDontSetUnnecessaryFlags() {
+ BasicObjWithCounter obja = new BasicObjWithCounter();
+ BasicObjWithCounter objb = new BasicObjWithCounter();
+ BasicObjWithCounter objc = new BasicObjWithCounter();
+ mBinder.setObja(obja);
+ mBinder.setObjb(objb);
+ mBinder.setObjc(objc);
+ mBinder.setA(true);
+ mBinder.setB(true);
+ mBinder.setC(false);
+ mBinder.executePendingBindings();
+ assertEquals("true", mBinder.textView.getText().toString());
+ assertEquals("true", mBinder.textView2.getText().toString());
+ assertEquals(1, obja.counter);
+ assertEquals(1, objb.counter);
+ assertEquals(0, objc.counter);
+ obja = new BasicObjWithCounter();
+ mBinder.setObja(obja);
+ mBinder.executePendingBindings();
+ assertEquals("true", mBinder.textView.getText().toString());
+ assertEquals("true", mBinder.textView2.getText().toString());
+ assertEquals(1, obja.counter);
+ assertEquals(1, objb.counter);
+ assertEquals(0, objc.counter);
+ }
+
+ private static class BasicObjWithCounter extends BasicObject {
+ int counter = 0;
+
+ @Override
+ public String boolMethod(boolean value) {
+ counter ++;
+ return super.boolMethod(value);
+ }
+ }
+
+}
diff --git a/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java b/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java
index 450f7fb..4ec76c5 100644
--- a/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java
+++ b/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/BasicObject.java
@@ -42,4 +42,8 @@
this.mField2 = field2;
notifyPropertyChanged(BR.field1);
}
+
+ public String boolMethod(boolean value) {
+ return value ? "true" : "false";
+ }
}
diff --git a/integration-tests/TestApp/app/src/main/res/layout/unnecessary_calculation.xml b/integration-tests/TestApp/app/src/main/res/layout/unnecessary_calculation.xml
new file mode 100644
index 0000000..0410189
--- /dev/null
+++ b/integration-tests/TestApp/app/src/main/res/layout/unnecessary_calculation.xml
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+ ~ Copyright (C) 2015 The Android Open Source Project
+ ~ Licensed under the Apache License, Version 2.0 (the "License");
+ ~ you may not use this file except in compliance with the License.
+ ~ You may obtain a copy of the License at
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<layout xmlns:android="http://schemas.android.com/apk/res/android">
+ <data>
+ <variable name="obja" type="android.databinding.testapp.vo.BasicObject"/>
+ <variable name="objb" type="android.databinding.testapp.vo.BasicObject"/>
+ <variable name="objc" type="android.databinding.testapp.vo.BasicObject"/>
+ <variable name="a" type="boolean"/>
+ <variable name="b" type="boolean"/>
+ <variable name="c" type="boolean"/>
+ </data>
+ <LinearLayout
+ android:orientation="vertical"
+ android:layout_width="match_parent"
+ android:layout_height="match_parent">
+ <TextView
+ android:id="@+id/textView"
+ android:text="@{obja.boolMethod(a)}"
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"/>
+ <android.databinding.testapp.view.MyTextView
+ android:id="@+id/textView2"
+ android:text="@{a ? objb.boolMethod(b) : objc.boolMethod(c)}"
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"/>
+ </LinearLayout>
+</layout>
\ No newline at end of file