Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make array and object prototype singletons immutable for now #4142

Merged
merged 3 commits into from Jun 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/ast/nodes/ArrayExpression.ts
Expand Up @@ -79,10 +79,14 @@ export default class ArrayExpression extends NodeBase {
const properties: ObjectProperty[] = [
{ key: 'length', kind: 'init', property: UNKNOWN_LITERAL_NUMBER }
];
let hasSpread = false;
for (let index = 0; index < this.elements.length; index++) {
const element = this.elements[index];
if (element instanceof SpreadElement) {
properties.unshift({ key: UnknownInteger, kind: 'init', property: element });
if (element instanceof SpreadElement || hasSpread) {
if (element) {
hasSpread = true;
properties.unshift({ key: UnknownInteger, kind: 'init', property: element });
}
} else if (!element) {
properties.push({ key: String(index), kind: 'init', property: UNDEFINED_EXPRESSION });
} else {
Expand Down
3 changes: 2 additions & 1 deletion src/ast/nodes/shared/ArrayPrototype.ts
Expand Up @@ -148,5 +148,6 @@ export const ARRAY_PROTOTYPE = new ObjectEntity(
unshift: METHOD_MUTATES_SELF_RETURNS_NUMBER,
values: METHOD_DEOPTS_SELF_RETURNS_UNKNOWN
} as unknown as PropertyMap,
OBJECT_PROTOTYPE
OBJECT_PROTOTYPE,
true
);
36 changes: 16 additions & 20 deletions src/ast/nodes/shared/ObjectEntity.ts
Expand Up @@ -50,7 +50,8 @@ export class ObjectEntity extends ExpressionEntity {
// and we assume there are no setters or getters
constructor(
properties: ObjectProperty[] | PropertyMap,
private prototypeExpression: ExpressionEntity | null
private prototypeExpression: ExpressionEntity | null,
private immutable = false
) {
super();
if (Array.isArray(properties)) {
Expand Down Expand Up @@ -96,7 +97,7 @@ export class ObjectEntity extends ExpressionEntity {
}

deoptimizePath(path: ObjectPath): void {
if (this.hasUnknownDeoptimizedProperty) return;
if (this.hasUnknownDeoptimizedProperty || this.immutable) return;
const key = path[0];
if (path.length === 1) {
if (typeof key !== 'string') {
Expand Down Expand Up @@ -136,9 +137,6 @@ export class ObjectEntity extends ExpressionEntity {
thisParameter: ExpressionEntity,
recursionTracker: PathTracker
): void {
if (path.length === 0) {
return;
}
const [key, ...subPath] = path;

if (
Expand Down Expand Up @@ -171,7 +169,9 @@ export class ObjectEntity extends ExpressionEntity {
property.deoptimizeThisOnEventAtPath(event, subPath, thisParameter, recursionTracker);
}
}
this.thisParametersToBeDeoptimized.add(thisParameter);
if (!this.immutable) {
this.thisParametersToBeDeoptimized.add(thisParameter);
}
return;
}
for (const property of relevantUnmatchableProperties) {
Expand All @@ -194,7 +194,9 @@ export class ObjectEntity extends ExpressionEntity {
property.deoptimizeThisOnEventAtPath(event, subPath, thisParameter, recursionTracker);
}
}
this.thisParametersToBeDeoptimized.add(thisParameter);
if (!this.immutable) {
this.thisParametersToBeDeoptimized.add(thisParameter);
}
this.prototypeExpression?.deoptimizeThisOnEventAtPath(
event,
path,
Expand Down Expand Up @@ -283,7 +285,9 @@ export class ObjectEntity extends ExpressionEntity {
return false;
}
for (const getter of this.unmatchableGetters) {
if (getter.hasEffectsWhenAccessedAtPath(subPath, context)) return true;
if (getter.hasEffectsWhenAccessedAtPath(subPath, context)) {
return true;
}
}
} else {
for (const getters of Object.values(this.gettersByKey).concat([this.unmatchableGetters])) {
Expand Down Expand Up @@ -315,6 +319,7 @@ export class ObjectEntity extends ExpressionEntity {
}

if (this.hasUnknownDeoptimizedProperty) return true;
// We do not need to test for unknown properties as in that case, hasUnknownDeoptimizedProperty is true
if (typeof key === 'string') {
if (this.propertiesAndSettersByKey[key]) {
const setters = this.settersByKey[key];
Expand All @@ -326,12 +331,8 @@ export class ObjectEntity extends ExpressionEntity {
return false;
}
for (const property of this.unmatchableSetters) {
if (property.hasEffectsWhenAssignedAtPath(subPath, context)) return true;
}
} else {
for (const setters of Object.values(this.settersByKey).concat([this.unmatchableSetters])) {
for (const setter of setters) {
if (setter.hasEffectsWhenAssignedAtPath(subPath, context)) return true;
if (property.hasEffectsWhenAssignedAtPath(subPath, context)) {
return true;
}
}
}
Expand Down Expand Up @@ -399,11 +400,6 @@ export class ObjectEntity extends ExpressionEntity {
}
if (!propertiesAndGettersByKey[key]) {
propertiesAndGettersByKey[key] = [property, ...unmatchablePropertiesAndGetters];
if (INTEGER_REG_EXP.test(key)) {
for (const integerProperty of unknownIntegerProps) {
propertiesAndGettersByKey[key].push(integerProperty);
}
}
}
}
}
Expand Down Expand Up @@ -467,7 +463,7 @@ export class ObjectEntity extends ExpressionEntity {
return UNKNOWN_EXPRESSION;
}
const expression = this.getMemberExpression(key);
if (expression !== UNKNOWN_EXPRESSION) {
if (!(expression === UNKNOWN_EXPRESSION || this.immutable)) {
const expressionsToBeDeoptimized = (this.expressionsToBeDeoptimizedByKey[key] =
this.expressionsToBeDeoptimizedByKey[key] || []);
expressionsToBeDeoptimized.push(origin);
Expand Down
3 changes: 2 additions & 1 deletion src/ast/nodes/shared/ObjectPrototype.ts
Expand Up @@ -15,5 +15,6 @@ export const OBJECT_PROTOTYPE = new ObjectEntity(
toString: METHOD_RETURNS_STRING,
valueOf: METHOD_RETURNS_UNKNOWN
} as unknown as PropertyMap,
null
null,
true
);
@@ -0,0 +1,3 @@
module.exports = {
description: 'correctly deoptimizes when there is no proto'
};
@@ -0,0 +1,7 @@
const obj = { __proto__: null };

obj.flag = true;

if (obj.flag) {
console.log('mutated');
}
@@ -0,0 +1,7 @@
const obj = { __proto__: null };

obj.flag = true;

if (obj.flag) {
console.log('mutated');
}
@@ -0,0 +1,4 @@
module.exports = {
description: 'removes unknown getter access without side effect',
options: { external: ['external'] }
};
@@ -0,0 +1 @@
import 'external';
@@ -0,0 +1,7 @@
import { unknown } from 'external';

const obj = {
get [unknown]() {}
};

obj.prop;
@@ -0,0 +1,4 @@
module.exports = {
description: 'removes unknown setter access without side effect',
options: { external: ['external'] }
};
@@ -0,0 +1 @@
import 'external';
@@ -0,0 +1,7 @@
import { unknown } from 'external';

const obj = {
set [unknown](value) {}
};

obj.prop = true;
3 changes: 3 additions & 0 deletions test/function/samples/array-double-spread/_config.js
@@ -0,0 +1,3 @@
module.exports = {
description: 'correctly deoptimizes when there is no proto'
};
19 changes: 19 additions & 0 deletions test/function/samples/array-double-spread/main.js
@@ -0,0 +1,19 @@
const a = [false, , true];
const b = [false, , true, ...a, false, , true, ...a];

let count = 0;

b[0] ? count+= 10: count++;
b[1] ? count+= 10: count++;
b[2] ? count+= 10: count++;
b[3] ? count+= 10: count++;
b[4] ? count+= 10: count++;
b[5] ? count+= 10: count++;
b[6] ? count+= 10: count++;
b[7] ? count+= 10: count++;
b[8] ? count+= 10: count++;
b[9] ? count+= 10: count++;
b[10] ? count+= 10: count++;
b[11] ? count+= 10: count++;

assert.strictEqual(count, 48);
@@ -1,7 +1,7 @@
module.exports = {
description: 'handles unknown getters that modify "this"',
context: {
require(id) {
require() {
return { unknown: 'prop' };
}
},
Expand Down
19 changes: 19 additions & 0 deletions test/function/samples/object-deep-access-effect/_config.js
@@ -0,0 +1,19 @@
const assert = require('assert');

module.exports = {
description: 'throws when an nested property of an unknown object property is accessed',
context: {
require() {
return { unknown: 'prop' };
}
},
options: {
external: ['external']
},
exports({ expectError }) {
assert.throws(expectError, {
name: 'TypeError',
message: "Cannot read property 'prop' of undefined"
});
}
};
6 changes: 6 additions & 0 deletions test/function/samples/object-deep-access-effect/main.js
@@ -0,0 +1,6 @@
import { unknown } from 'external';

export function expectError() {
const obj = {};
obj[unknown].prop;
}
3 changes: 3 additions & 0 deletions test/function/samples/returned-array-mutation/_config.js
@@ -0,0 +1,3 @@
module.exports = {
description: 'tracks array mutations'
};
13 changes: 13 additions & 0 deletions test/function/samples/returned-array-mutation/main.js
@@ -0,0 +1,13 @@
let push = false;

const getArray = () => {
const array = [];
if (push) {
array.push(true);
}
return array;
};

assert.strictEqual(getArray()[0] || false, false);
push = true;
assert.strictEqual(getArray()[0] || false, true);