Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Re-land typed descriptors and printer #2511

Closed
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ jobs:
yarn test-react
yarn test-sourcemaps
yarn test-std-in
yarn test-test262 --expectedCounts 11944,5641,0 --statusFile ~/artifacts/test262-status.txt --timeout 120 --cpuScale 0.25 --verbose
yarn test-test262 --expectedCounts 11944,5641,0 --statusFile ~/artifacts/test262-status.txt --timeout 140 --cpuScale 0.25 --verbose
#yarn test-test262-new --statusFile ~/artifacts/test262-new-status.txt --timeout 120 --verbose
- store_artifacts:
path: ~/artifacts/
Expand Down
34 changes: 30 additions & 4 deletions scripts/test-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ let execSpec;
let JSONTokenizer = require("../lib/utils/JSONTokenizer.js").default;
let { Linter } = require("eslint");
let lintConfig = require("./lint-config");
let TextPrinter = require("../lib/utils/TextPrinter.js").TextPrinter;

function transformWithBabel(code, plugins, presets) {
return babel.transform(code, {
Expand Down Expand Up @@ -431,6 +432,7 @@ function runTest(name, code, options: PrepackOptions, args) {
return Promise.resolve(false);
} else {
let codeIterations = [];
let irIterations = [];
let markersToFind = [];
for (let [positive, marker] of [[true, "// does contain:"], [false, "// does not contain:"]]) {
for (let i = code.indexOf(marker); i >= 0; i = code.indexOf(marker, i + 1)) {
Expand Down Expand Up @@ -509,6 +511,14 @@ function runTest(name, code, options: PrepackOptions, args) {
options.errorHandler = getErrorHandlerWithWarningCapture(diagnosticOutput, args.verbose);
}

let ir = "";
if (args.ir)
options.onExecute = (realm, optimizedFunctions) => {
new TextPrinter(line => {
ir += line + "\n";
}).print(realm, optimizedFunctions);
};

let serialized = prepackSources([{ filePath: name, fileContents: code, sourceMapContents: "" }], options);
if (serialized.statistics && serialized.statistics.delayedValues > 0) anyDelayedValues = true;
if (!serialized) {
Expand Down Expand Up @@ -585,7 +595,11 @@ function runTest(name, code, options: PrepackOptions, args) {
if (!execSpec && options.lazyObjectsRuntime !== undefined) {
codeToRun = augmentCodeWithLazyObjectSupport(codeToRun, args.lazyObjectsRuntime);
}
if (args.verbose) console.log(codeToRun);
if (args.verbose) {
if (args.ir) console.log(`=== ir\n${ir}\n`);
console.log(`=== generated code\n${codeToRun}\n`);
}
irIterations.push(unescapeUniqueSuffix(ir, options.uniqueSuffix));
codeIterations.push(unescapeUniqueSuffix(codeToRun, options.uniqueSuffix));
if (args.es5) {
codeToRun = transformWithBabel(
Expand Down Expand Up @@ -662,6 +676,10 @@ function runTest(name, code, options: PrepackOptions, args) {
console.error(chalk.underline("output of inspect() on original code"));
console.error(expected);
for (let ii = 0; ii < codeIterations.length; ii++) {
if (args.ir) {
console.error(chalk.underline(`ir in iteration ${ii}`));
console.error(irIterations[ii]);
}
console.error(chalk.underline(`generated code in iteration ${ii}`));
console.error(codeIterations[ii]);
}
Expand Down Expand Up @@ -833,6 +851,7 @@ class ProgramArgs {
fast: boolean;
residual: boolean;
cpuprofilePath: string;
ir: boolean;
constructor(
debugNames: boolean,
debugScopes: boolean,
Expand All @@ -844,7 +863,8 @@ class ProgramArgs {
noLazySupport: boolean,
fast: boolean,
residual: boolean,
cpuProfilePath: string
cpuProfilePath: string,
ir: boolean
) {
this.debugNames = debugNames;
this.debugScopes = debugScopes;
Expand All @@ -857,6 +877,7 @@ class ProgramArgs {
this.fast = fast;
this.residual = residual;
this.cpuprofilePath = cpuProfilePath;
this.ir = ir;
}
}

Expand Down Expand Up @@ -891,7 +912,7 @@ function usage(): string {
return (
`Usage: ${process.argv[0]} ${process.argv[1]} ` +
EOL +
`[--debugNames] [--debugScopes] [--es5] [--fast] [--noLazySupport] [--verbose] [--filter <string>] [--outOfProcessRuntime <path>] `
`[--debugNames] [--debugScopes] [--es5] [--fast] [--noLazySupport] [--verbose] [--ir] [--filter <string>] [--outOfProcessRuntime <path>] `
);
}

Expand Down Expand Up @@ -919,6 +940,7 @@ function argsParse(): ProgramArgs {
// to run tests. If not a separate node context used.
lazyObjectsRuntime: LAZY_OBJECTS_RUNTIME_NAME,
noLazySupport: false,
ir: false,
fast: false,
residual: false,
cpuprofilePath: "",
Expand Down Expand Up @@ -953,6 +975,9 @@ function argsParse(): ProgramArgs {
if (typeof parsedArgs.noLazySupport !== "boolean") {
throw new ArgsParseError("noLazySupport must be a boolean (either --noLazySupport or not)");
}
if (typeof parsedArgs.ir !== "boolean") {
throw new ArgsParseError("ir must be a boolean (either --ir or not)");
}
if (typeof parsedArgs.residual !== "boolean") {
throw new ArgsParseError("residual must be a boolean (either --residual or not)");
}
Expand All @@ -970,7 +995,8 @@ function argsParse(): ProgramArgs {
parsedArgs.noLazySupport,
parsedArgs.fast,
parsedArgs.residual,
parsedArgs.cpuprofilePath
parsedArgs.cpuprofilePath,
parsedArgs.ir
);
return programArgs;
}
Expand Down
136 changes: 136 additions & 0 deletions src/descriptors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/**
* Copyright (c) 2017-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

/* @flow */

import invariant from "./invariant.js";
import type { AbstractValue, UndefinedValue, Value } from "./values/index.js";
import { CompilerDiagnostic, FatalError } from "./errors.js";
import type { CallableObjectValue } from "./types.js";
import type { Realm } from "./realm.js";

export class Descriptor {
constructor() {
invariant(this.constructor !== Descriptor, "Descriptor is an abstract base class");
}
throwIfNotConcrete(realm: Realm): PropertyDescriptor {
let error = new CompilerDiagnostic(
"only known descriptors supported",
realm.currentLocation,
"PP0042",
"FatalError"
);
realm.handleError(error);
throw new FatalError();
}
mightHaveBeenDeleted(): boolean {
invariant(false, "should have been overridden by subclass");
}
}

export type DescriptorInitializer = {|
writable?: boolean,
enumerable?: boolean,
configurable?: boolean,

value?: Value,

get?: UndefinedValue | CallableObjectValue | AbstractValue,
set?: UndefinedValue | CallableObjectValue | AbstractValue,
|};

// Normal descriptors are returned just like spec descriptors
export class PropertyDescriptor extends Descriptor {
writable: void | boolean;
enumerable: void | boolean;
configurable: void | boolean;

// If value instanceof EmptyValue, then this descriptor indicates that the
// corresponding property has been deleted.
value: void | Value;

get: void | UndefinedValue | CallableObjectValue | AbstractValue;
set: void | UndefinedValue | CallableObjectValue | AbstractValue;

constructor(desc: DescriptorInitializer | PropertyDescriptor) {
super();
this.writable = desc.writable;
this.enumerable = desc.enumerable;
this.configurable = desc.configurable;
this.value = desc.value;
this.get = desc.get;
this.set = desc.set;
}

throwIfNotConcrete(realm: Realm): PropertyDescriptor {
return this;
}
mightHaveBeenDeleted(): boolean {
if (this.value === undefined) return false;
return this.value.mightHaveBeenDeleted();
}
}

// Only internal properties (those starting with $ / where internalSlot of owning property binding is true) will ever have array values.
export class InternalSlotDescriptor extends Descriptor {
value: void | Value | Array<any>;

constructor(value?: void | Value | Array<any>) {
super();
this.value = Array.isArray(value) ? value.slice(0) : value;
}

mightHaveBeenDeleted(): boolean {
return false;
}
}

// Only used if the result of a join of two descriptors is not a data descriptor with identical attribute values.
// When present, any update to the property must produce effects that are the join of updating both descriptors,
// using joinCondition as the condition of the join.
export class AbstractJoinedDescriptor extends Descriptor {
joinCondition: AbstractValue;
// An undefined descriptor means it might be empty in this branch.
descriptor1: void | Descriptor;
descriptor2: void | Descriptor;

constructor(joinCondition: AbstractValue, descriptor1?: Descriptor, descriptor2?: Descriptor) {
super();
this.joinCondition = joinCondition;
this.descriptor1 = descriptor1;
this.descriptor2 = descriptor2;
}
mightHaveBeenDeleted(): boolean {
if (!this.descriptor1 || this.descriptor1.mightHaveBeenDeleted()) {
return true;
}
if (!this.descriptor2 || this.descriptor2.mightHaveBeenDeleted()) {
return true;
}
return false;
}
}

export function cloneDescriptor(d: void | PropertyDescriptor): void | PropertyDescriptor {
if (d === undefined) return undefined;
return new PropertyDescriptor(d);
}

// does not check if the contents of value properties are the same
export function equalDescriptors(d1: PropertyDescriptor, d2: PropertyDescriptor): boolean {
if (d1.writable !== d2.writable) return false;
if (d1.enumerable !== d2.enumerable) return false;
if (d1.configurable !== d2.configurable) return false;
if (d1.value !== undefined) {
Copy link
Contributor

@NTillmann NTillmann Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some asymmetry here, causing equalDescriptors not to be commutative when one value is undefined and the other one isn't. That issue already existed in the original code. I doubt that it's intentional? Maybe it gets rescued by the other invariant that the existance of value and get/set are mutually exclusive. (We could use more such invariants in places.)

Anyway, for clarity, I'd suggest rewriting this case to

if ((d1.value === undefined) !== (d2.value === undefined)) return false;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three types of descriptors at the spec level. Generic, Data and Accessors. Lack of value and get/set means it's generic. So if one has a value but the other doesn't, it means that they're not even in the same category of descriptors.

Undefined value doesn't mean empty btw. That's an empty value.

This helper function, for reasons that preceded me, doesn't compare whether the values themselves are equivalent.

Basically, this function is only meant to determine if this needs to use joined Descriptor or if it's enough to use a joined Value.

In theory, accessors doesn't need to use a joined Descriptor neither. We can just use a joined value of the getter and a joined value of the setter.

if (d2.value === undefined) return false;
}
if (d1.get !== d2.get) return false;
if (d1.set !== d2.set) return false;
return true;
}
33 changes: 22 additions & 11 deletions src/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import PrimitiveValue from "./values/PrimitiveValue.js";
import { createOperationDescriptor } from "./utils/generator.js";

import { SourceMapConsumer, type NullableMappedPosition } from "source-map";
import { PropertyDescriptor } from "./descriptors.js";

function deriveGetBinding(realm: Realm, binding: Binding) {
let types = TypesDomain.topVal;
Expand Down Expand Up @@ -448,12 +449,17 @@ export class ObjectEnvironmentRecord extends EnvironmentRecord {
// 4. Return ? DefinePropertyOrThrow(bindings, N, PropertyDescriptor{[[Value]]: undefined, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: configValue}).
return new BooleanValue(
realm,
Properties.DefinePropertyOrThrow(realm, bindings, N, {
value: realm.intrinsics.undefined,
writable: true,
enumerable: true,
configurable: configValue,
})
Properties.DefinePropertyOrThrow(
realm,
bindings,
N,
new PropertyDescriptor({
value: realm.intrinsics.undefined,
writable: true,
enumerable: true,
configurable: configValue,
})
)
);
}

Expand Down Expand Up @@ -898,7 +904,8 @@ export class GlobalEnvironmentRecord extends EnvironmentRecord {

// 5. If existingProp is undefined, return false.
if (!existingProp) return false;
Properties.ThrowIfMightHaveBeenDeleted(existingProp.value);
Properties.ThrowIfMightHaveBeenDeleted(existingProp);
existingProp = existingProp.throwIfNotConcrete(globalObject.$Realm);

// 6. If existingProp.[[Configurable]] is true, return false.
if (existingProp.configurable) return false;
Expand Down Expand Up @@ -948,7 +955,8 @@ export class GlobalEnvironmentRecord extends EnvironmentRecord {

// 5. If existingProp is undefined, return ? IsExtensible(globalObject).
if (!existingProp) return IsExtensible(realm, globalObject);
Properties.ThrowIfMightHaveBeenDeleted(existingProp.value);
Properties.ThrowIfMightHaveBeenDeleted(existingProp);
existingProp = existingProp.throwIfNotConcrete(globalObject.$Realm);

// 6. If existingProp.[[Configurable]] is true, return true.
if (existingProp.configurable) return true;
Expand Down Expand Up @@ -1015,17 +1023,20 @@ export class GlobalEnvironmentRecord extends EnvironmentRecord {

// 4. Let existingProp be ? globalObject.[[GetOwnProperty]](N).
let existingProp = globalObject.$GetOwnProperty(N);
if (existingProp) {
existingProp = existingProp.throwIfNotConcrete(globalObject.$Realm);
}

// 5. If existingProp is undefined or existingProp.[[Configurable]] is true, then
let desc;
if (!existingProp || existingProp.configurable) {
// a. Let desc be the PropertyDescriptor{[[Value]]: V, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: D}.
desc = { value: V, writable: true, enumerable: true, configurable: D };
desc = new PropertyDescriptor({ value: V, writable: true, enumerable: true, configurable: D });
} else {
// 6. Else,
Properties.ThrowIfMightHaveBeenDeleted(existingProp.value);
Properties.ThrowIfMightHaveBeenDeleted(existingProp);
// a. Let desc be the PropertyDescriptor{[[Value]]: V }.
desc = { value: V };
desc = new PropertyDescriptor({ value: V });
}

// 7. Perform ? DefinePropertyOrThrow(globalObject, N, desc).
Expand Down
2 changes: 1 addition & 1 deletion src/evaluators/ForInStatement.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ function emitResidualLoopIfSafe(
if (key.object.unknownProperty === key) {
targetObject = key.object;
invariant(desc !== undefined);
let sourceValue = desc.value;
let sourceValue = desc.throwIfNotConcrete(realm).value;
if (sourceValue instanceof AbstractValue) {
// because sourceValue was written to key.object.unknownProperty it must be that
let cond = sourceValue.args[0];
Expand Down
16 changes: 11 additions & 5 deletions src/evaluators/FunctionDeclaration.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { MakeConstructor } from "../methods/construct.js";
import { Create, Functions, Properties } from "../singletons.js";
import { StringValue } from "../values/index.js";
import IsStrict from "../utils/strict.js";
import { PropertyDescriptor } from "../descriptors.js";
import type { BabelNodeFunctionDeclaration } from "@babel/types";

// ECMA262 14.1.20
Expand Down Expand Up @@ -44,11 +45,16 @@ export default function(
let prototype = Create.ObjectCreate(realm, realm.intrinsics.GeneratorPrototype);

// 5. Perform DefinePropertyOrThrow(F, "prototype", PropertyDescriptor{[[Value]]: prototype, [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: false}).
Properties.DefinePropertyOrThrow(realm, F, "prototype", {
value: prototype,
writable: true,
configurable: false,
});
Properties.DefinePropertyOrThrow(
realm,
F,
"prototype",
new PropertyDescriptor({
value: prototype,
writable: true,
configurable: false,
})
);

// 6. Perform SetFunctionName(F, name).
Functions.SetFunctionName(realm, F, name);
Expand Down
Loading