Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 266aa36b authored by Yigit Boyar's avatar Yigit Boyar
Browse files

Fix ternary handling and generate better code 

This CL fixes an issue about ternary expressions where
a ternary expression would be evaluated with its last
evaluated dependency. This would create a problem where
ternary expressions would not be evaluated if other branch
of the conditional is chosen, This bug is fixed by checking
outher flags such that we'll still calculate it together
if all dependencies are behind the same flag vs we'll
calculate it independently if its dependency flags are different.

This CL also improves the generated code in two ways:
  - When there is an if inside if, we don't add flag check (the if)
    if all of its conditions are covered in the parent if.
  - I replaced flag names with binary values. This looks more
    readable then generated names.

Bug: 20073197
Change-Id: I9d07868206a5393d6509ab0a205b30a796e11107
parent 3695b1aa

tools/data-binding/compiler/src/main/java/android/databinding/tool/writer/FlagSet.java

+59 −4
Original line number Diff line number Diff line
@@ -32,7 +32,6 @@ public class FlagSet { 


    public FlagSet(BitSet bitSet, int bucketCount) {

        buckets = new long[bucketCount];

        Arrays.fill(buckets, 0L);

        for (int i = bitSet.nextSetBit(0);

                i != -1; i = bitSet.nextSetBit(i + 1)) {

            buckets[i / sBucketSize] |= 1 << (i % sBucketSize);
@@ -42,9 +41,13 @@ public class FlagSet { 


    public FlagSet(long[] buckets) {

        this.buckets = new long[buckets.length];

        for (int i = 0; i < buckets.length; i ++) {

            this.buckets[i] = buckets[i];

        System.arraycopy(buckets, 0, this.buckets, 0, buckets.length);

        type = "long";

    }



    public FlagSet(long[] buckets, int minBucketCount) {

        this.buckets = new long[Math.max(buckets.length, minBucketCount)];

        System.arraycopy(buckets, 0, this.buckets, 0, buckets.length);

        type = "long";

    }
@@ -84,4 +87,56 @@ public class FlagSet { 
    public void setDynamic(boolean isDynamic) {

        mIsDynamic = isDynamic;

    }



    public FlagSet andNot(FlagSet other) {

        FlagSet result = new FlagSet(buckets);

        final int min = Math.min(buckets.length, other.buckets.length);

        for (int i = 0; i < min; i ++) {

            result.buckets[i] &= ~(other.buckets[i]);

        }

        return result;

    }



    public FlagSet or(FlagSet other) {

        final FlagSet result = new FlagSet(buckets, other.buckets.length);

        for (int i = 0; i < other.buckets.length; i ++) {

            result.buckets[i] |= other.buckets[i];

        }

        return result;

    }



    public boolean isEmpty() {

        for (int i = 0; i < buckets.length; i ++) {

            if (buckets[i] != 0) {

                return false;

            }

        }

        return true;

    }



    @Override

    public String toString() {

        StringBuilder sb = new StringBuilder();

        for (int i = 0; i < buckets.length; i ++) {

            sb.append(Long.toBinaryString(buckets[i])).append(" ");

        }

        return sb.toString();

    }



    private long getBucket(int bucketIndex) {

        if (bucketIndex >= buckets.length) {

            return 0;

        }

        return buckets[bucketIndex];

    }



    public boolean bitsEqual(FlagSet other) {

        final int max = Math.max(buckets.length, other.buckets.length);

        for (int i = 0; i < max; i ++) {

            if (getBucket(i) != other.getBucket(i)) {

                return false;

            }

        }

        return true;

    }

}

tools/data-binding/compiler/src/main/kotlin/android/databinding/tool/writer/LayoutBinderWriter.kt

+35 −33
Original line number Diff line number Diff line
@@ -258,26 +258,20 @@ fun Expr.conditionalFlagName(output : Boolean, suffix : String) = "${dirtyFlagNa 




val Expr.dirtyFlagSet by Delegates.lazy { expr : Expr ->

    val fs = FlagSet(expr.getInvalidFlags(), expr.getModel().getFlagBucketCount())

    expr.getModel().localizeFlag(fs, expr.dirtyFlagName)

    FlagSet(expr.getInvalidFlags(), expr.getModel().getFlagBucketCount())

}



val Expr.invalidateFlagSet by Delegates.lazy { expr : Expr ->

    val fs = FlagSet(expr.getId())

    expr.getModel().localizeFlag(fs, expr.invalidateFlagName)

    FlagSet(expr.getId())

}



val Expr.shouldReadFlagSet by Delegates.lazy { expr : Expr ->

    val fs = FlagSet(expr.getShouldReadFlags(), expr.getModel().getFlagBucketCount())

    expr.getModel().localizeFlag(fs, expr.shouldReadFlagName)

    FlagSet(expr.getShouldReadFlags(), expr.getModel().getFlagBucketCount())

}



val Expr.conditionalFlags by Delegates.lazy { expr : Expr ->

    val model = expr.getModel()

    arrayListOf(model.localizeFlag(FlagSet(expr.getRequirementFlagIndex(false)),

            "${expr.conditionalFlagPrefix}False"),

            model.localizeFlag(FlagSet(expr.getRequirementFlagIndex(true)),

                    "${expr.conditionalFlagPrefix}True"))

    arrayListOf(FlagSet(expr.getRequirementFlagIndex(false)),

            FlagSet(expr.getRequirementFlagIndex(true)))

}



fun Expr.getRequirementFlagSet(expected : Boolean) : FlagSet = conditionalFlags[if(expected) 1 else 0]
@@ -300,18 +294,14 @@ fun FlagSet.getWordSuffix(wordIndex : Int) : String { 
}



fun FlagSet.localValue(bucketIndex : Int) =

        if (getLocalName() == null) buckets[bucketIndex]

        if (getLocalName() == null) binaryCode(bucketIndex)

        else "${getLocalName()}${getWordSuffix(bucketIndex)}"



fun FlagSet.or(other : FlagSet, cb : (suffix : String) -> Unit) {

    val min = Math.min(buckets.size(), other.buckets.size())

    for (i in 0..(min - 1)) {

        // if these two can match by any chance, call the callback

        if (intersect(other, i)) {

            cb(getWordSuffix(i))

        }

    }

}

fun FlagSet.binaryCode(bucketIndex : Int) = longToBinary(buckets[bucketIndex])





fun longToBinary(l : Long) =

        "0b${java.lang.Long.toBinaryString(l)}L"



fun <T> FlagSet.mapOr(other : FlagSet, cb : (suffix : String, index : Int) -> T) : List<T> {

    val min = Math.min(buckets.size(), other.buckets.size())
@@ -649,7 +639,7 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) { 
            flag.notEmpty { suffix, value ->

                nl("private")

                app(" ", if(flag.isDynamic()) null else "static final");

                app(" ", " ${flag.type} ${flag.getLocalName()}$suffix = $value;")

                app(" ", " ${flag.type} ${flag.getLocalName()}$suffix = ${longToBinary(value)};")

            }

        }

    }
@@ -673,7 +663,6 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) { 
                tab("${tmpDirtyFlags.type} ${tmpDirtyFlags.localValue(i)} = ${mDirtyFlags.localValue(i)};")

                tab("${mDirtyFlags.localValue(i)} = 0;")

            }

            //tab("""log("dirty flags", mDirtyFlags);""")

            model.getPendingExpressions().filterNot {it.isVariable()}.forEach {

                tab("${it.getResolvedType().toJavaCode()} ${it.localName} = ${it.getDefaultValue()};")

            }
@@ -728,7 +717,6 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) { 
                        }

                        tab("}")

                    }

            //

            includedBinders.filter{it.isUsed()}.forEach { binder ->

                tab("${binder.fieldName}.executePendingBindings();")

            }
@@ -744,14 +732,18 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) { 
        nl("}")

    }



    fun readWithDependants(expr : Expr, mJustRead : MutableList<Expr>, batch : MutableList<Expr>, tmpDirtyFlags : FlagSet) : KCode = kcode("") {

    fun readWithDependants(expr : Expr, mJustRead : MutableList<Expr>, batch : MutableList<Expr>,

            tmpDirtyFlags : FlagSet, inheritedFlags : FlagSet? = null) : KCode = kcode("") {

        mJustRead.add(expr)

        Log.d { expr.getUniqueKey() }

        val flagSet = expr.shouldReadFlagSet

        tab("if (${tmpDirtyFlags.mapOr(flagSet){ suffix, index ->

        val needsIfWrapper = inheritedFlags == null || !flagSet.bitsEqual(inheritedFlags)

        val ifClause = "if (${tmpDirtyFlags.mapOr(flagSet){ suffix, index ->

            "(${tmpDirtyFlags.localValue(index)} & ${flagSet.localValue(index)}) != 0"

        }.joinToString(" || ")

        }) {") {

        })"



        val readCode = kcode("") {

            if (!expr.isVariable()) {

                // it is not a variable read it.

                tab("// read ${expr.getUniqueKey()}")
@@ -762,12 +754,10 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) { 
                if (!expr.isEqualityCheck() && nullables.isNotEmpty()) {

                    tab ("if ( ${nullables.map { "${it.localName} != null" }.joinToString(" && ")}) {") {

                        tab("${expr.localName}").app(" = ", expr.toCode(true)).app(";")

                        //tab("""log("${expr}" + ${expr.localName},0);""")

                    }

                    tab("}")

                } else {

                    tab("${expr.localName}").app(" = ", expr.toCode(true)).app(";")

                    //tab("""log("${expr}" + ${expr.localName},0);""")

                }

                if (expr.isObservable()) {

                    tab("updateRegistration(${expr.getId()}, ${expr.localName});")
@@ -799,15 +789,27 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) { 
            }



            val chosen = expr.getDependants().filter {

                batch.contains(it.getDependant()) && it.getDependant().shouldReadNow(mJustRead)

                val dependant = it.getDependant()

                batch.contains(dependant) &&

                        dependant.shouldReadFlagSet.andNot(flagSet).isEmpty() &&

                        dependant.shouldReadNow(mJustRead)

            }

            if (chosen.isNotEmpty()) {

                val nextInheritedFlags = if (needsIfWrapper) flagSet else inheritedFlags

                chosen.forEach {

                    nl(readWithDependants(it.getDependant(), mJustRead, batch, tmpDirtyFlags))

                    nl(readWithDependants(it.getDependant(), mJustRead, batch, tmpDirtyFlags, nextInheritedFlags))

                }

            }

        }

        if (needsIfWrapper) {

            tab(ifClause) {

                app(" {")

                nl(readCode)

            }

            tab("}")

        } else {

            nl(readCode)

        }

    }



    fun declareFactories() = kcode("") {
@@ -821,7 +823,7 @@ class LayoutBinderWriter(val layoutBinder : LayoutBinder) { 
        nl("}")

        nl("public static ${baseClassName} bind(android.view.View view) {") {

            tab("if (!\"${layoutBinder.getId()}\".equals(view.getTag())) {") {

                tab("throw new RuntimeException(\"view tag doesn't isn't correct on view\");")

                tab("throw new RuntimeException(\"view tag isn't correct on view\");")

            }

            tab("}")

            tab("return new ${baseClassName}(view);")

tools/data-binding/integration-tests/TestApp/app/src/androidTest/java/android/databinding/testapp/ReadComplexTernaryTest.java

0 → 100644
+22 −0
Original line number Diff line number Diff line 
package android.databinding.testapp;



import android.test.UiThreadTest;

import android.databinding.testapp.databinding.ReadComplexTernaryBinding;



import android.databinding.testapp.vo.User;



public class ReadComplexTernaryTest extends BaseDataBinderTest<ReadComplexTernaryBinding> {

    public ReadComplexTernaryTest() {

        super(ReadComplexTernaryBinding.class);

    }



    @UiThreadTest

    public void testWhenNull() {

        User user = new User();

        user.setName("a");

        user.setFullName("a b");

        mBinder.setUser(user);

        mBinder.executePendingBindings();

        assertEquals("?", mBinder.textView.getText().toString());

    }

}

tools/data-binding/integration-tests/TestApp/app/src/main/java/android/databinding/testapp/vo/User.java

0 → 100644
+36 −0
Original line number Diff line number Diff line 
package android.databinding.testapp.vo;



import android.databinding.Bindable;



public class User {

    @Bindable

    private User friend;

    @Bindable

    private String name;

    @Bindable

    private String fullName;



    public User getFriend() {

        return friend;

    }



    public void setFriend(User friend) {

        this.friend = friend;

    }



    public String getName() {

        return name;

    }



    public void setName(String name) {

        this.name = name;

    }



    public String getFullName() {

        return fullName;

    }



    public void setFullName(String fullName) {

        this.fullName = fullName;

    }

}

tools/data-binding/integration-tests/TestApp/app/src/main/res/layout/read_complex_ternary.xml

0 → 100644
+12 −0
Original line number Diff line number Diff line 
<?xml version="1.0" encoding="utf-8"?>

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"

    android:orientation="vertical" android:layout_width="match_parent"

    android:layout_height="match_parent">

    <variable name="user" type="android.databinding.testapp.vo.User"/>

    <TextView

        android:id="@+id/text_view"

        android:text='@{user.friend == null ? "?" : (user.friend.name + "is friend of " + user.name)}'

        android:layout_width="wrap_content"

        android:layout_height="wrap_content"/>



</LinearLayout>
 
 No newline at end of file