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

Commit

Permalink
Re-land typed descriptors and printer (#2511)
Browse files Browse the repository at this point in the history
Summary:
Release notes: landing two previously reverted PRs

I found the issue.

Flow doesn't support optional fields in classes. This effectively becomes enforced with Babel 7 since it treats the type annotations as field initializers. Which means that these fields always gets created.

We happened to only use this newer plugin for class fields internally which broke our builds only there.

2b4546d Use `undefined` instead of missing to represent absent field in descriptors. Fixed all the callsites that checks for `hasOwnProperty` or `in` that I could find.

922d40c Fixes a Flow issue in the text printer.

I filed a [follow up issue for ObjectValue](#2510) since it has the same problem.
Pull Request resolved: #2511

Reviewed By: trueadm

Differential Revision: D9569949

Pulled By: sebmarkbage

fbshipit-source-id: f8bf84c4385de4f0ff6bcd45badacd3b8c88c533
  • Loading branch information
sebmarkbage authored and facebook-github-bot committed Aug 31, 2018
1 parent 69699c2 commit 54be9b5
Show file tree
Hide file tree
Showing 77 changed files with 3,130 additions and 1,494 deletions.
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 @@ -507,6 +509,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 @@ -583,7 +593,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 @@ -660,6 +674,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 @@ -829,6 +847,7 @@ class ProgramArgs {
noLazySupport: boolean;
fast: boolean;
cpuprofilePath: string;
ir: boolean;
constructor(
debugNames: boolean,
debugScopes: boolean,
Expand All @@ -839,7 +858,8 @@ class ProgramArgs {
lazyObjectsRuntime: string,
noLazySupport: boolean,
fast: boolean,
cpuProfilePath: string
cpuProfilePath: string,
ir: boolean
) {
this.debugNames = debugNames;
this.debugScopes = debugScopes;
Expand All @@ -851,6 +871,7 @@ class ProgramArgs {
this.noLazySupport = noLazySupport;
this.fast = fast;
this.cpuprofilePath = cpuProfilePath;
this.ir = ir;
}
}

Expand Down Expand Up @@ -885,7 +906,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 @@ -913,6 +934,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,
cpuprofilePath: "",
},
Expand Down Expand Up @@ -949,6 +971,9 @@ function argsParse(): ProgramArgs {
if (typeof parsedArgs.cpuprofilePath !== "string") {
throw new ArgsParseError("cpuprofilePath must be a string");
}
if (typeof parsedArgs.ir !== "boolean") {
throw new ArgsParseError("ir must be a string");
}
let programArgs = new ProgramArgs(
parsedArgs.debugNames,
parsedArgs.debugScopes,
Expand All @@ -959,7 +984,8 @@ function argsParse(): ProgramArgs {
parsedArgs.lazyObjectsRuntime,
parsedArgs.noLazySupport,
parsedArgs.fast,
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) {
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

0 comments on commit 54be9b5

Please sign in to comment.