Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Remove parameter binding logic as it has abysmal performance in its
current form. This also makes bindCallAtPath obsolete again. Instead
we always assume that parameters have unknown values when determining
side effects.
The hack that calling a member of an included variable is always a side
effect also needs to stay in place until we find a way to determine with
absolute certainty it was not overridden e.g. by assuming unknown
function calls always mutate their parameters.
  • Loading branch information
lukastaegert committed Nov 8, 2017
1 parent eb1c581 commit 75f36d1
Show file tree
Hide file tree
Showing 32 changed files with 113 additions and 300 deletions.
9 changes: 0 additions & 9 deletions src/ast/Node.js
Expand Up @@ -31,15 +31,6 @@ export default class Node {
*/
bindAssignmentAtPath ( path, expression, options ) {}

/**
* Binds the arguments a node is called with to this node and possibly its parameters.
* Should usually be overridden together with hasEffectsWhenCalled.
* @param {String[]} path
* @param {CallOptions} callOptions
* @param {ExecutionPathOptions} options
*/
bindCallAtPath ( path, callOptions, options ) {}

/**
* Override to control on which children "bind" is called.
*/
Expand Down
10 changes: 2 additions & 8 deletions src/ast/nodes/ArrowFunctionExpression.js
Expand Up @@ -3,11 +3,6 @@ import Scope from '../scopes/Scope';
import ReturnValueScope from '../scopes/ReturnValueScope';

export default class ArrowFunctionExpression extends Node {
bindCallAtPath ( path, { args } ) {
path.length === 0
&& this.scope.bindCallArguments( args );
}

bindNode () {
this.body.bindImplicitReturnExpressionToScope
? this.body.bindImplicitReturnExpressionToScope()
Expand Down Expand Up @@ -35,9 +30,8 @@ export default class ArrowFunctionExpression extends Node {
if ( path.length > 0 ) {
return true;
}
const innerOptions = this.scope.getOptionsWithReplacedParameters( callOptions.args, options );
return this.params.some( param => param.hasEffects( innerOptions ) )
|| this.body.hasEffects( innerOptions );
return this.params.some( param => param.hasEffects( options ) )
|| this.body.hasEffects( options );
}

initialiseChildren () {
Expand Down
7 changes: 0 additions & 7 deletions src/ast/nodes/CallExpression.js
Expand Up @@ -9,12 +9,6 @@ export default class CallExpression extends Node {
node.bindAssignmentAtPath( path, expression, innerOptions.addAssignedReturnExpressionAtPath( path, this ) ), options );
}

bindCallAtPath ( path, callOptions, options ) {
!options.hasReturnExpressionBeenCalledAtPath( path, this )
&& this.callee.forEachReturnExpressionWhenCalledAtPath( [], this._callOptions, innerOptions => node =>
node.bindCallAtPath( path, callOptions, innerOptions.addCalledReturnExpressionAtPath( path, this ) ), options );
}

bindNode () {
if ( this.callee.type === 'Identifier' ) {
const variable = this.scope.findVariable( this.callee.name );
Expand All @@ -34,7 +28,6 @@ export default class CallExpression extends Node {
}, this.start );
}
}
this.callee.bindCallAtPath( [], this._callOptions, ExecutionPathOptions.create() );
}

forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
Expand Down
6 changes: 0 additions & 6 deletions src/ast/nodes/ClassBody.js
@@ -1,12 +1,6 @@
import Node from '../Node';

export default class ClassBody extends Node {
bindCallAtPath ( path, callOptions, options ) {
path.length === 0
&& this.classConstructor
&& this.classConstructor.bindCallAtPath( path, callOptions, options );
}

hasEffectsWhenCalledAtPath ( path, callOptions, options ) {
if ( path.length > 0 ) {
return true;
Expand Down
4 changes: 0 additions & 4 deletions src/ast/nodes/ConditionalExpression.js
Expand Up @@ -7,10 +7,6 @@ export default class ConditionalExpression extends Node {
&& this._forEachRelevantBranch( node => node.bindAssignmentAtPath( path, expression, options ) );
}

bindCallAtPath ( path, callOptions, options ) {
this._forEachRelevantBranch( node => node.bindCallAtPath( path, callOptions, options ) );
}

forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
this._forEachRelevantBranch( node => node.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback, options ) );
}
Expand Down
6 changes: 0 additions & 6 deletions src/ast/nodes/Identifier.js
Expand Up @@ -9,12 +9,6 @@ export default class Identifier extends Node {
&& this.variable.bindAssignmentAtPath( path, expression, options );
}

bindCallAtPath ( path, callOptions, options ) {
this._bindVariableIfMissing();
this.variable
&& this.variable.bindCallAtPath( path, callOptions, options );
}

bindNode () {
this._bindVariableIfMissing();
}
Expand Down
4 changes: 0 additions & 4 deletions src/ast/nodes/LogicalExpression.js
Expand Up @@ -7,10 +7,6 @@ export default class LogicalExpression extends Node {
&& this._forEachRelevantBranch( node => node.bindAssignmentAtPath( path, expression, options ) );
}

bindCallAtPath ( path, callOptions, options ) {
this._forEachRelevantBranch( node => node.bindCallAtPath( path, callOptions, options ) );
}

forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
this._forEachRelevantBranch( node => node.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback, options ) );
}
Expand Down
9 changes: 0 additions & 9 deletions src/ast/nodes/MemberExpression.js
Expand Up @@ -85,15 +85,6 @@ export default class MemberExpression extends Node {
}
}

bindCallAtPath ( path, callOptions, options ) {
if ( !this._bound ) this.bind();
if ( this.variable ) {
this.variable.bindCallAtPath( path, callOptions, options );
} else {
this.object.bindCallAtPath( [ this._getPathSegment(), ...path ], callOptions, options );
}
}

forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
if ( !this._bound ) this.bind();
if ( this.variable ) {
Expand Down
5 changes: 0 additions & 5 deletions src/ast/nodes/MethodDefinition.js
@@ -1,11 +1,6 @@
import Node from '../Node';

export default class MethodDefinition extends Node {
bindCallAtPath ( path, callOptions, options ) {
path.length === 0
&& this.value.bindCallAtPath( path, callOptions, options );
}

hasEffects ( options ) {
return this.key.hasEffects( options );
}
Expand Down
4 changes: 0 additions & 4 deletions src/ast/nodes/NewExpression.js
Expand Up @@ -3,10 +3,6 @@ import CallOptions from '../CallOptions';
import ExecutionPathOptions from '../ExecutionPathOptions';

export default class NewExpression extends Node {
bindNode () {
this.callee.bindCallAtPath( [], this._callOptions, ExecutionPathOptions.create() );
}

hasEffects ( options ) {
return this.arguments.some( child => child.hasEffects( options ) )
|| this.callee.hasEffectsWhenCalledAtPath( [], this._callOptions, options.getHasEffectsWhenCalledOptions() );
Expand Down
8 changes: 0 additions & 8 deletions src/ast/nodes/ObjectExpression.js
Expand Up @@ -15,14 +15,6 @@ export default class ObjectExpression extends Node {
&& property.bindAssignmentAtPath( path.slice( 1 ), expression, options ) );
}

bindCallAtPath ( path, callOptions, options ) {
if ( path.length === 0 ) return;

const { properties, hasCertainHit } = this._getPossiblePropertiesWithName( path[ 0 ], PROPERTY_KINDS_READ );
hasCertainHit && properties.forEach( property =>
property.bindCallAtPath( path.slice( 1 ), callOptions, options ) );
}

forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
if ( path.length === 0 ) return;

Expand Down
17 changes: 1 addition & 16 deletions src/ast/nodes/Property.js
Expand Up @@ -8,28 +8,13 @@ export default class Property extends Node {
path.length > 0
&& this.value.forEachReturnExpressionWhenCalledAtPath( [], this._accessorCallOptions, innerOptions => node =>
node.bindAssignmentAtPath( path, expression, innerOptions.addAssignedReturnExpressionAtPath( path, this ) ), options );
} else if ( this.kind === 'set' ) {
path.length === 0
&& this.value.bindCallAtPath( [], CallOptions.create( { withNew: false, args: [ expression ], caller: this } ), options );
} else {
} else if ( this.kind !== 'set' ) {
this.value.bindAssignmentAtPath( path, expression, options );
}
}

bindCallAtPath ( path, callOptions, options ) {
if ( this.kind === 'get' ) {
this.value.bindCallAtPath( [], this._accessorCallOptions, options );
!options.hasReturnExpressionBeenCalledAtPath( path, this )
&& this.value.forEachReturnExpressionWhenCalledAtPath( [], this._accessorCallOptions, innerOptions => node =>
node.bindCallAtPath( path, callOptions, innerOptions.addCalledReturnExpressionAtPath( path, this ) ), options );
} else {
this.value.bindCallAtPath( path, callOptions, options );
}
}

forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
if ( this.kind === 'get' ) {
this.value.bindCallAtPath( [], this._accessorCallOptions, options );
this.value.forEachReturnExpressionWhenCalledAtPath( [], this._accessorCallOptions, innerOptions => node =>
node.forEachReturnExpressionWhenCalledAtPath( path, callOptions, callback, innerOptions ), options );
} else {
Expand Down
5 changes: 0 additions & 5 deletions src/ast/nodes/shared/ClassNode.js
Expand Up @@ -2,11 +2,6 @@ import Node from '../../Node.js';
import Scope from '../../scopes/Scope';

export default class ClassNode extends Node {
bindCallAtPath ( path, callOptions, options ) {
this.body.bindCallAtPath( path, callOptions, options );
this.superClass && this.superClass.bindCallAtPath( path, callOptions, options );
}

hasEffectsWhenAccessedAtPath ( path ) {
return path.length > 1;
}
Expand Down
6 changes: 0 additions & 6 deletions src/ast/nodes/shared/FunctionNode.js
Expand Up @@ -3,12 +3,6 @@ import FunctionScope from '../../scopes/FunctionScope';
import VirtualObjectExpression from './VirtualObjectExpression';

export default class FunctionNode extends Node {
bindCallAtPath ( path, { args } ) {
if ( path.length === 0 ) {
this.scope.bindCallArguments( args );
}
}

bindNode () {
this.body.bindImplicitReturnExpressionToScope();
}
Expand Down
4 changes: 2 additions & 2 deletions src/ast/scopes/FunctionScope.js
Expand Up @@ -16,8 +16,8 @@ export default class FunctionScope extends ReturnValueScope {
}

getOptionsWhenCalledWith ( { args, withNew }, options ) {
return super.getOptionsWithReplacedParameters( args, options )
return options
.replaceVariableInit( this.variables.this, withNew ? new VirtualObjectExpression() : UNKNOWN_ASSIGNMENT )
.setArgumentsVariables( args.map( ( parameter, index ) => super.getParameterVariables()[index] || parameter) );
.setArgumentsVariables( args.map( ( parameter, index ) => super.getParameterVariables()[ index ] || parameter ) );
}
}
14 changes: 0 additions & 14 deletions src/ast/scopes/ParameterScope.js
@@ -1,6 +1,5 @@
import Scope from './Scope';
import ParameterVariable from '../variables/ParameterVariable';
import { UNKNOWN_ASSIGNMENT } from '../values';

export default class ParameterScope extends Scope {
constructor ( options = {} ) {
Expand All @@ -21,19 +20,6 @@ export default class ParameterScope extends Scope {
return variable;
}

bindCallArguments ( args ) {
this._parameters.forEach( ( parameter, index ) =>
parameter.bindInitialization( args[ index ] || UNKNOWN_ASSIGNMENT ) );
}

getOptionsWithReplacedParameters ( args, options ) {
let newOptions = options;
this._parameters.forEach( ( parameter, index ) =>
newOptions = newOptions.replaceVariableInit( parameter, args[ index ] || UNKNOWN_ASSIGNMENT )
);
return newOptions;
}

getParameterVariables () {
return this._parameters;
}
Expand Down
6 changes: 2 additions & 4 deletions src/ast/scopes/ReturnValueScope.js
Expand Up @@ -11,12 +11,10 @@ export default class ReturnValueScope extends ParameterScope {
}

forEachReturnExpressionWhenCalled ( callOptions, callback, options ) {
const innerOptions = this.getOptionsWithReplacedParameters( callOptions.args, options );
this._returnExpressions.forEach( callback( innerOptions ) );
this._returnExpressions.forEach( callback( options ) );
}

someReturnExpressionWhenCalled ( callOptions, predicateFunction, options ) {
const innerOptions = this.getOptionsWithReplacedParameters( callOptions.args, options );
return Array.from( this._returnExpressions ).some( predicateFunction( innerOptions ) );
return Array.from( this._returnExpressions ).some( predicateFunction( options ) );
}
}
1 change: 0 additions & 1 deletion src/ast/values.js
Expand Up @@ -3,7 +3,6 @@ export const UNKNOWN_VALUE = { toString: () => '[[UNKNOWN]]' };
export const UNKNOWN_ASSIGNMENT = {
type: 'UNKNOWN',
bindAssignmentAtPath: () => {},
bindCallAtPath: () => {},
forEachReturnExpressionWhenCalledAtPath: () => {},
hasEffectsWhenAccessedAtPath: path => path.length > 0,
hasEffectsWhenAssignedAtPath: path => path.length > 0,
Expand Down
12 changes: 1 addition & 11 deletions src/ast/variables/LocalVariable.js
Expand Up @@ -2,7 +2,7 @@ import Variable from './Variable';
import StructuredAssignmentTracker from './StructuredAssignmentTracker';

// To avoid infinite recursions
const MAX_PATH_LENGTH = 8;
const MAX_PATH_LENGTH = 6;

export default class LocalVariable extends Variable {
constructor ( name, declarator, init ) {
Expand All @@ -12,7 +12,6 @@ export default class LocalVariable extends Variable {
this.declarations = new Set( declarator ? [ declarator ] : null );
this.boundExpressions = new StructuredAssignmentTracker();
init && this.boundExpressions.addAtPath( [], init );
this.boundCalls = new StructuredAssignmentTracker();
}

addDeclaration ( identifier ) {
Expand All @@ -32,15 +31,6 @@ export default class LocalVariable extends Variable {
} else {
this.isReassigned = true;
}
this.boundCalls.forEachAtPath( path, ( relativePath, callOptions ) =>
expression.bindCallAtPath( relativePath, callOptions, options ) );
}

bindCallAtPath ( path, callOptions, options ) {
if ( path.length > MAX_PATH_LENGTH || this.boundCalls.hasAtPath( path, callOptions ) ) return;
this.boundCalls.addAtPath( path, callOptions );
this.boundExpressions.forEachAtPath( path, ( relativePath, node ) =>
node.bindCallAtPath( relativePath, callOptions, options ) );
}

forEachReturnExpressionWhenCalledAtPath ( path, callOptions, callback, options ) {
Expand Down
9 changes: 0 additions & 9 deletions src/ast/variables/Variable.js
Expand Up @@ -23,15 +23,6 @@ export default class Variable {
*/
bindAssignmentAtPath ( path, expression, options ) {}

/**
* Binds a call to this node. Necessary to be able to bind arguments
* to parameters.
* @param {String[]} path
* @param {CallOptions} callOptions
* @param {ExecutionPathOptions} options
*/
bindCallAtPath ( path, callOptions, options ) {}

/**
* @param {String[]} path
* @param {CallOptions} callOptions
Expand Down
36 changes: 18 additions & 18 deletions test/form/samples/arrow-function-call-parameters/_expected/amd.js
@@ -1,33 +1,33 @@
define(function () { 'use strict';

const callArg2 = arg => arg();
callArg2( () => console.log( 'effect' ) );
const callArg = arg => arg();
callArg( () => console.log( 'effect' ) );

const assignArg2 = arg => arg.foo.bar = 1;
assignArg2( {} );
const assignArg = arg => arg.foo.bar = 1;
assignArg( {} );

const returnArg = arg => arg;
returnArg( () => console.log( 'effect' ) )();

const returnArg2 = arg => arg;
returnArg2( () => console.log( 'effect' ) )();
returnArg2( {} ).foo.bar = 1;

const returnArg4 = arg => arg;
returnArg4( {} ).foo.bar = 1;
const returnArg3 = arg => arg;
returnArg3( () => () => console.log( 'effect' ) )()();

const returnArg6 = arg => arg;
returnArg6( () => () => console.log( 'effect' ) )()();
const returnArgReturn = arg => arg();
returnArgReturn( () => () => console.log( 'effect' ) )();

const returnArgReturn2 = arg => arg();
returnArgReturn2( () => () => console.log( 'effect' ) )();
returnArgReturn2( () => ({}) ).foo.bar = 1;

const returnArgReturn4 = arg => arg();
returnArgReturn4( () => ({}) ).foo.bar = 1;
const returnArgReturn3 = arg => arg();
returnArgReturn3( () => () => () => console.log( 'effect' ) )()();

const returnArgReturn6 = arg => arg();
returnArgReturn6( () => () => () => console.log( 'effect' ) )()();
const multiArgument = ( func, obj ) => func( obj );
multiArgument( obj => obj(), () => console.log( 'effect' ) );

const multiArgument2 = ( func, obj ) => func( obj );
multiArgument2( obj => obj(), () => console.log( 'effect' ) );

const multiArgument4 = ( func, obj ) => func( obj );
multiArgument4( obj => obj.foo.bar = 1, {} );
multiArgument2( obj => obj.foo.bar = 1, {} );

});

0 comments on commit 75f36d1

Please sign in to comment.