Propagate volatile variables in AsNode previsit

[jack-work]

When a global variable is referenced many times throughout a variadic expression, e.g.

Notify(x); Set(y, x+1); Collect(c, x); // etc.

the code generator may chose to generate a variable lookup each time the variable is accessed in the expression. Variable lookups come as some runtime expense, and the value does not change at any point in the example expression. As such, the compiler employs a strategy termed in-code as "variable lifting". The compiled output of such an expression may instead create a local variable that caches the value of x, and the accesses may be translated to reference this cached value. This allows the expression to be run while only performing a lookup once, which improves runtime performance at scale.

This is implemented during binding. When a variadic node is visited and a global variable reference is encountered, a local cache variable will be generated to contain the value. The problem arises when the variable is altered within the variadic expression. For example, suppose that x is 10 when this expression is executed:

Notify(x); Set(x, 1); Set(y, x + 1);

If variable lifting is employed, the value of x, 10, will be cached and used in the Notify, which is correct, but also in Set(y, x + 1). This is incorrect, as semantically, x was set to one in the Set expression previous to Set(y, x + 1). x in this case constitutes a volatile variable, one whose features are different depending on the index of the variadic expression.

To account for this, when variadic expressions are bound, each semicolon separated sub-expression is visited. During visitation, the binder is meant to track Set invocations of any global variables. If a set is encountered, the set variable is propagated via a node map in TexlBinding.VolatileVariables, through which it propagates up the tree on ascension (PostVisit) and is remapped until the variadic parent is reencountered. When the next expression is to be bound, the volatile variables are propagated into it, and if any such variable is referenced by subsequent expressions, this node is flagged as "Unliftable".

The problem this PR solves is that when using a ForAll(variable As Test, ...), variable As Test is deemed unliftable, but it fails to propagate the volatile variables to its left child, variable. As a result, variable is never categorized as unliftable, and the code generator will apply the volatile variable cache lifting pattern, which will cause incorrect behavior at runtime.

DottedNameNode already accounts for this behavior, but we missed it when added the As keyword, which was implemented after the variable weight system was put into place.

---

Working, without As. The global variable is referenced lazily, after it is set, and passed into the ForAll lambda.

With(
    {dummyWith: colDummyTable},
    Set(
        gblNotWorking,
        {DummyField: dummyWith}
    );
    ForAll(
        gblNotWorking.DummyField,
        Collect(
            colNotWorking2,
            ThisRecord
        )
    )
)

function (bc) {
    return AppMagic.Functions.with_R({"dummyWith": (AppMagic.getLanguageRuntime().getOrCreateScopeCollectionValue("0", "colDummyTable", bc))}, function (param1) {
        return AppMagic.getLanguageRuntime().collectErrors([AppMagic.Functions.set(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 6, {
            span: {
                start: 42,
                end: 107
            }
        }), bc, "1", "gblNotWorking", {"DummyField": (AppMagic.getLanguageRuntime().getFieldValue(param1, "dummyWith"))}), AppMagic.Functions.forAll(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 14, {
            span: {
                start: 113,
                end: 238
            }
        }), AppMagic.getLanguageRuntime().getPropertyValue(AppMagic.getLanguageRuntime().getScopeVariableValue("1", "gblNotWorking", AppMagic.AuthoringTool.Runtime.getGlobalBindingContext()), "DummyField", bc), function (param2) {
            return AppMagic.Functions.collect(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 12, {
                span: {
                    start: 163,
                    end: 232
                }
            }), AppMagic.getLanguageRuntime().getOrCreateScopeCollectionValue("0", "colNotWorking2", bc), param2);
        }, false)]);
    });
}

Not working, with As, and without my fix. The local1_0 is the cached value of the global variable. Since it is cached before it is accessed, the previous value is used.

With(
    {dummyWith: colDummyTable},
    Set(
        gblNotWorking,
        {DummyField: dummyWith}
    );
    ForAll(
        gblNotWorking.DummyField As currentRecord,
        Collect(
            colNotWorking,
            currentRecord
        )
    )
)

function (bc) {
    return AppMagic.Functions.with_R({"dummyWith": (AppMagic.getLanguageRuntime().getOrCreateScopeCollectionValue("0", "colDummyTable", bc))}, function () {
        var local1_0 = AppMagic.getLanguageRuntime().getPropertyValue(AppMagic.getLanguageRuntime().getScopeVariableValue("1", "gblNotWorking", AppMagic.AuthoringTool.Runtime.getGlobalBindingContext()), "DummyField", bc);
        return function (param1) {
            return AppMagic.getLanguageRuntime().collectErrors([AppMagic.Functions.set(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 6, {
                span: {
                    start: 42,
                    end: 107
                }
            }), bc, "1", "gblNotWorking", {"DummyField": (AppMagic.getLanguageRuntime().getFieldValue(param1, "dummyWith"))}), AppMagic.Functions.forAll(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 15, {
                span: {
                    start: 113,
                    end: 257
                }
            }), local1_0, function (param2) {
                return AppMagic.Functions.collect(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 13, {
                    span: {
                        start: 180,
                        end: 251
                    }
                }), AppMagic.getLanguageRuntime().getOrCreateScopeCollectionValue("0", "colNotWorking", bc), param2);
            }, false)]);
        }
    }());
}

Here is what the codegen looks like with this fix. Notice that local1_0 is gone and it now exactly matches the working code

function (bc) {
    return AppMagic.Functions.with_R({"dummyWith": (AppMagic.getLanguageRuntime().getOrCreateScopeCollectionValue("0", "colDummyTable", bc))}, function (param1) {
        return AppMagic.getLanguageRuntime().collectErrors([AppMagic.Functions.set(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 6, {
            span: {
                start: 42,
                end: 107
            }
        }), bc, "1", "gblNotWorking", {"DummyField": (AppMagic.getLanguageRuntime().getFieldValue(param1, "dummyWith"))}), AppMagic.Functions.forAll(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 15, {
            span: {
                start: 113,
                end: 257
            }
        }), AppMagic.getLanguageRuntime().getPropertyValue(AppMagic.getLanguageRuntime().getScopeVariableValue("1", "gblNotWorking", AppMagic.AuthoringTool.Runtime.getGlobalBindingContext()), "DummyField", bc), function (param2) {
            return AppMagic.Functions.collect(AppMagic.getLanguageRuntime().cloneAndUpdateErrorContext(ec, 13, {
                span: {
                    start: 180,
                    end: 251
                }
            }), AppMagic.getLanguageRuntime().getOrCreateScopeCollectionValue("0", "colNotWorking", bc), param2);
        }, false)]);
    });
}

[adithyaselv]

probably good to add a test without Set, where the expression is liftable

[adithyaselv]