[WIP] Drop the notion of a "template" in an AbstractObjectValue

[sebmarkbage]

There are still a few more little things to fix before this can work.

There is no practical difference between an abstract object with a single object in its values domain and a concrete object. The only difference is whether we have an "operation description". However, in practice that is not even how it works in practice. We actually generate an intrinsic name from that operation and then the abstract object has the intrinsic name.

There is also a notion of multiple templates in an AbstractObjectValue domain but that code path is not exhausted because we instead special case conditional objects to work slightly differently. AbstractObjectValue of the conditional kind also isn't enough to support the case where one condition is a not an object but the other one is.

After discussion we know that we'll want to split the AbstractObjectValue into multiple classes to support its other use cases:

• Conditional values (maybe just make this use plain AbstractValue).
• Top val objects (a new specific type that deals with object operations on unknown identities).
• Widened objects (a new specific type that deals with widened object identities).

So that leaves the question of what happens with the template form.

This PR explores the consequence of just dropping the notion of an abstract object "template". Instead, all existing use cases just become concrete object values with intrinsic names. This is correct because the identity is indeed concrete.

It simplifies a lot of things and fixes a few places where we unnecessarily fatal today. Because throwIfNotConcrete and instanceof ObjectValue checks will fail on AbstractObjectValue with a template even though they have the same semantics so we shouldn't need to.

This change turns out to be pretty complicated because we have many little fragile assumptions in the modeling, but it also found a number of bugs where the abstract object template and concrete template differs.

The internal mechanism I use is the existing deriveConcreteObject mechanism which lets me generate an intrinsic name from an abstract operation.

In terms of the public API. The first step is that __abstract({}, name) just returns the template and assigns the name to it without the wrapper. The next step could be to drop the template and intrinsic name argument in __abstract and instead the API would be:

let intrinsicObject = __intrinsic({}, name); // concrete object with intrinsic name
let intrinsicTopValObject = __intrinsic(__abstract("object"), name2);
let intrinsicString = __intrinsic(__abstract("string"), name3);

That would require more work to upgrade all our tests and internal model though. So I'd only do that once this first PR is stable and landed.

[hermanventer]

I've looked at this some more and I'm wrong about the JSON.parse(JSON.stringify(ob)) cloning special case being about materialization.

[sebmarkbage]

Yea I'll definitely break this up into stages. I just wanted to initially see if it was feasible. Looks like it should be doable.

[matthargett]

@sebmarkbage it looks like some simple lint fixes will make this go green:

/home/circleci/project/src/intrinsics/dom/document.js
  16:8  error  'invariant' is defined but never used  no-unused-vars

/home/circleci/project/src/intrinsics/ecma262/JSON.js
  28:10  error  'ValuesDomain' is defined but never used  no-unused-vars

/home/circleci/project/src/intrinsics/prepack/global.js
  29:10  error  'ValuesDomain' is defined but never used  no-unused-vars

/home/circleci/project/src/intrinsics/prepack/utils.js
  23:10  error  'ValuesDomain' is defined but never used  no-unused-vars

/home/circleci/project/src/values/ObjectValue.js
  13:10  error  'ValuesDomain' is defined but never used               no-unused-vars
  52:10  error  'createOperationDescriptor' is defined but never used  no-unused-vars

[facebook-github-bot]

Hi @sebmarkbage!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!