Handle constant predicate in ternary

If a ternary expressions's predicate is constant, we would never evaluate it
which means we would never evaluate the ternary unless some other expression
depends on it.

This CL changes ExprModel to move such constant expressions into pending list
so that they can be evaluated + necessary flags are set.
We can actually avoid this process by replacing TernaryExpression with something
else when this case is detected but that would be a bigger change and not safe
shortly before the release.

Hopefully, codegen logic will be refactored into a more well defined process.

Bug: 24768154
Change-Id: I0918568414b64d64f070978f1f8e77cc3b6c85fd

4 files changed, 32 insertions, 12 deletions

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 fc7129ca..b321ed1d 100644
--- a/compiler/src/main/java/android/databinding/tool/expr/Expr.java
+++ b/compiler/src/main/java/android/databinding/tool/expr/Expr.java
@@ -408,7 +408,7 @@ abstract public class Expr implements VersionProvider, LocationScopeProvider {
      */
     public int getRequirementFlagIndex(boolean expectedOutput) {
         Preconditions.check(mRequirementId != NO_ID, "If this is an expression w/ conditional"
-                + " dependencies, it must be assigned a requirement ID");
+                + " dependencies, it must be assigned a requirement ID. %s", this);
         return expectedOutput ? mRequirementId + 1 : mRequirementId;
     }
 
@@ -673,6 +673,20 @@ abstract public class Expr implements VersionProvider, LocationScopeProvider {
 
     protected abstract KCode generateCode();
 
+    /**
+     * This expression is the predicate for 1 or more ternary expressions.
+     */
+    public boolean hasConditionalDependant() {
+        for (Dependency dependency : getDependants()) {
+            Expr dependant = dependency.getDependant();
+            if (dependant.isConditional() && dependant instanceof TernaryExpr) {
+                TernaryExpr ternary = (TernaryExpr) dependant;
+                return ternary.getPred() == this;
+            }
+        }
+        return false;
+    }
+
     static class Node {
 
         BitSet mBitSet = new BitSet();
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 5939510f..c92b79d0 100644
--- a/compiler/src/main/java/android/databinding/tool/expr/ExprModel.java
+++ b/compiler/src/main/java/android/databinding/tool/expr/ExprModel.java
@@ -476,7 +476,9 @@ public class ExprModel {
         if (mPendingExpressions == null) {
             mPendingExpressions = new ArrayList<>();
             for (Expr expr : mExprMap.values()) {
-                if (!expr.isRead() && expr.isDynamic()) {
+                // if an expression is NOT dynanic but has conditional dependants, still return it
+                // so that conditional flags can be set
+                if (!expr.isRead() && (expr.isDynamic() || expr.hasConditionalDependant())) {
                     mPendingExpressions.add(expr);
                 }
             }
diff --git a/compiler/src/main/java/android/databinding/tool/expr/TernaryExpr.java b/compiler/src/main/java/android/databinding/tool/expr/TernaryExpr.java
index a1a4432a..12379c20 100644
--- a/compiler/src/main/java/android/databinding/tool/expr/TernaryExpr.java
+++ b/compiler/src/main/java/android/databinding/tool/expr/TernaryExpr.java
@@ -69,11 +69,10 @@ public class TernaryExpr extends Expr {
     protected List<Dependency> constructDependencies() {
         List<Dependency> deps = new ArrayList<>();
         Expr predExpr = getPred();
-        if (predExpr.isDynamic()) {
-            final Dependency pred = new Dependency(this, predExpr);
-            pred.setMandatory(true);
-            deps.add(pred);
-        }
+        final Dependency pred = new Dependency(this, predExpr);
+        pred.setMandatory(true);
+        deps.add(pred);
+
         Expr ifTrueExpr = getIfTrue();
         if (ifTrueExpr.isDynamic()) {
             deps.add(new Dependency(this, ifTrueExpr, predExpr, true));
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 53b04346..daee2add 100644
--- a/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt
+++ b/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt
@@ -21,6 +21,7 @@ import android.databinding.tool.expr.FieldAccessExpr
 import android.databinding.tool.expr.IdentifierExpr
 import android.databinding.tool.expr.ListenerExpr
 import android.databinding.tool.expr.TernaryExpr
+import android.databinding.tool.expr.ResourceExpr
 import android.databinding.tool.ext.androidId
 import android.databinding.tool.ext.br
 import android.databinding.tool.ext.joinToCamelCaseAsVar
@@ -86,6 +87,11 @@ fun ExprModel.getConstructorParamName(base : String) : String = ext.getUniqueNam
 
 fun ExprModel.localizeFlag(set : FlagSet, base : String) : FlagSet = ext.localizeFlag(set, base)
 
+val Expr.needsLocalField by Delegates.lazy { expr : Expr ->
+    expr.canBeEvaluatedToAVariable() && !(expr.isVariable() && !expr.isUsed()) && (expr.isDynamic() || expr is ResourceExpr)
+}
+
+
 // not necessarily unique. Uniqueness is solved per scope
 val BindingTarget.readableName by Delegates.lazy { target: BindingTarget ->
     if (target.getId() == null) {
@@ -156,7 +162,8 @@ val Expr.oldValueName by Delegates.lazy { expr : Expr ->
 }
 
 val Expr.executePendingLocalName by Delegates.lazy { expr : Expr ->
-    "${expr.getModel().ext.getUniqueName(expr.readableName, Scope.EXECUTE_PENDING_METHOD, false)}"
+    if(expr.needsLocalField) "${expr.getModel().ext.getUniqueName(expr.readableName, Scope.EXECUTE_PENDING_METHOD, false)}"
+    else expr.toCode().generate()
 }
 
 val Expr.setterName by Delegates.lazy { expr : Expr ->
@@ -726,7 +733,7 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) {
                     tab("${mDirtyFlags.localValue(i)} = 0;")
                 }
             } tab("}")
-            model.getPendingExpressions().filterNot { !it.canBeEvaluatedToAVariable() || (it.isVariable() && !it.isUsed()) }.forEach {
+            model.getPendingExpressions().filter { it.needsLocalField }.forEach {
                 tab("${it.getResolvedType().toJavaCode()} ${it.executePendingLocalName} = ${if (it.isVariable()) it.fieldName else it.getDefaultValue()};")
             }
             L.d("writing executePendingBindings for %s", className)
@@ -835,9 +842,7 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) {
                 val dependants = ArrayList<Expr>()
                 expressions.groupBy { condition(it) }.forEach {
                     val condition = it.key
-                    val assignedValues = it.value.filter {
-                        it.canBeEvaluatedToAVariable() && !it.isVariable()
-                    }
+                    val assignedValues = it.value.filter { it.needsLocalField }
                     if (!assignedValues.isEmpty()) {
                         val assignment = kcode("") {
                             assignedValues.forEach { expr: Expr ->