Eliminate child-parent read-write conflict errors

[cblappert]

Release Notes: None

This PR makes it so we no longer report conflicts for child optimized functions reading from values that their parents have written.

Couple of things for discussion:

• This creates somewhat confusing behavior because the child functions will take the concrete value after the parent functions' effects have been applied:

(function () {
    let obj = {p: 42};
    function f() {
        obj.p += 3;
        function g() { return obj.p; } // Will optimize to `return 47;`
        obj.p += 1;
        __optimize(g);
        obj.p += 1;
        return g;
    }
    __optimize(f);
    global.f = f;
})();

• This PR creates the Parent/Child relationship based off AdditionalFunctionEffects.parentAdditionalFunction which goes off of syntactic nesting of functions instead of nesting of __optimize calls as in the issue. I believe basing the nesting off of __optimize calls could lead to somewhat unintuitive results (especially considering we store parentAdditionalFunction in AdditionalFunctionEffects to be the syntactic parent).

I am not sure if it is needed by Instant Render @NTillmann? If it is, we may want to consider changing AdditionalFunctionEffects.parentAdditionalFunction to be based off of __optimize call nesting as well for consistency.

As an example a slight modification on the example above:

(function () {
    let obj = {p: 42};
    function g() { return obj.p; }
    function f() {
        obj.p += 3;
        __optimize(g);
        obj.p += 1;
        return [g, obj];
    }
    __optimize(f);
    global.f = f;
})();

With this PR, we report an error. Based off of __optimize nesting, we would optimize g to return 46;. I would expect to see return 42; return 45; or return obj.p; in this case.

• To resolve the above issues, in the future, we could make any values modified by parent optimized functions into abstract values during evaluation for child optimized functions to force their accesses to be recorded in generators .

Resolves #2351

[hermanventer]

I'm not comfortable with this. I think it is fine to use the value of an outer scope variable, as it is at the time __optimize is being called, but any attempt to modify the variable after that should produce a warning.

[hermanventer]

!== undefined

[hermanventer]

Should be possibleParent instead of fun. Please add a test case that will fail with the code as it is now.

[cblappert]

@NTillmann This doesn't change the behavior of Prepack -- it's consistent with how we deal with captured global variables, but I want to make sure that this is the behavior you want for nested functions.

With this PR, we would allow:

(function () {
    let p = 42;
    function f() {
        p += 3;
        function g() { return p; } // Will optimize to `return 47;`
        p += 1;
        __optimize(g);
        p += 1;
        return g;
    }
    __optimize(f);
    global.f = f;
})();

as well as the example at the top of the page. This could produce incorrect code in the case that p (or obj in the example at the top of the page) is leaked or modified by another residual function (or even if f is called more than once).

A more conservative assumption would be to say that we emit a Recoverable error if g or p/obj.p is leaked because then our return 47; code could be incorrect.

[NTillmann]

I think the combination of leaking and optimized functions is generally not working quite right (there are a few open issues related to that), so I don't expect this PR that aims at addressing #2351 to fix all that.

[NTillmann]

LGTM

[facebook-github-bot]

cblappert is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.