From 6ce8281a4d014a315f80bf4fcce6648cad1b143c Mon Sep 17 00:00:00 2001 From: Chris Blappert Date: Fri, 20 Jul 2018 16:52:34 -0700 Subject: [PATCH 1/4] Improved reported error for PP1003 --- src/serializer/functions.js | 47 +++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/src/serializer/functions.js b/src/serializer/functions.js index 8bfb5ce975..7f6121e8b3 100644 --- a/src/serializer/functions.js +++ b/src/serializer/functions.js @@ -38,6 +38,7 @@ import { handleReportedSideEffect } from "./utils.js"; import { ShapeInformation } from "../utils/ShapeInformation"; import type { ArgModel } from "../utils/ShapeInformation"; import * as t from "@babel/types"; +import { stringOfLocation } from "../utils/babelhelpers"; type AdditionalFunctionEntry = { value: ECMAScriptSourceFunctionValue | AbstractValue, @@ -299,7 +300,12 @@ export class Functions { let conflicts: Map = new Map(); for (let fun1 of additionalFunctions) { invariant(fun1 instanceof FunctionValue); - let fun1Name = this.functionExpressions.get(fun1) || fun1.intrinsicName || "(unknown function)"; + let fun1Location = fun1.expressionLocation; + let fun1Name = + this.functionExpressions.get(fun1) || + fun1.intrinsicName || + fun1.__originalName || + `(unknown function ${fun1Location ? stringOfLocation(fun1Location) : ""})`; // Also do argument validation here let additionalFunctionEffects = this.writeEffects.get(fun1); invariant(additionalFunctionEffects !== undefined); @@ -318,8 +324,14 @@ export class Functions { for (let fun2 of additionalFunctions) { if (fun1 === fun2) continue; invariant(fun2 instanceof FunctionValue); + let fun2Location = fun2.expressionLocation; + let fun2Name = + this.functionExpressions.get(fun2) || + fun2.intrinsicName || + fun2.__originalName || + `(unknown function ${fun2Location ? stringOfLocation(fun2Location) : ""})`; let reportFn = () => { - this.reportWriteConflicts(fun1Name, conflicts, e1.modifiedProperties, this._callOfFunction(fun2)); + this.reportWriteConflicts(fun1Name, fun2Name, conflicts, e1.modifiedProperties, this._callOfFunction(fun2)); return null; }; let fun2Effects = this.writeEffects.get(fun2); @@ -344,14 +356,25 @@ export class Functions { } reportWriteConflicts( - fname: string, + f1name: string, + f2name: string, conflicts: Map, pbs: PropertyBindings, call2: void => Value ): void { - let reportConflict = (location: BabelNodeSourceLocation) => { + let reportConflict = ( + location: BabelNodeSourceLocation, + object: string = "", + key?: string, + originalLocation?: BabelNodeSourceLocation + ) => { + let firstLocationString = originalLocation ? ` ${stringOfLocation(originalLocation)}` : ""; + let propString = key ? `${object ? "" : ""}.${key} ` : ""; + let objectString = ` ${object}${propString}`; let error = new CompilerDiagnostic( - `Property access conflicts with write in optimized function ${fname}`, + `Property access${objectString}at${firstLocationString} in "${f1name}" conflicts with write at ${ + location.start.line + }:${location.start.column} in optimized function "${f2name}"`, location, "PP1003", "FatalError" @@ -366,14 +389,24 @@ export class Functions { this.realm.reportObjectGetOwnProperties = (ob: ObjectValue) => { let location = this.realm.currentLocation; invariant(location); - if (writtenObjects.has(ob) && !conflicts.has(location)) reportConflict(location); + if (writtenObjects.has(ob) && !conflicts.has(location)) + reportConflict(location, ob.intrinsicName || ob.__originalName, undefined, ob.expressionLocation); }; let oldReportPropertyAccess = this.realm.reportPropertyAccess; this.realm.reportPropertyAccess = (pb: PropertyBinding) => { if (ObjectValue.refuseSerializationOnPropertyBinding(pb)) return; let location = this.realm.currentLocation; if (!location) return; // happens only when accessing an additional function property - if (pbs.has(pb) && !conflicts.has(location)) reportConflict(location); + if (pbs.has(pb) && !conflicts.has(location)) { + let originalLocation = + pb.descriptor && pb.descriptor.value ? pb.descriptor.value.expressionLocation : undefined; + reportConflict( + location, + pb.object ? pb.object.intrinsicName || pb.object.__originalName : undefined, + pb.key, + originalLocation + ); + } }; try { ignoreErrorsIn(this.realm, () => this.realm.evaluateForEffectsInGlobalEnv(call2)); From c268db46df0cba9298e85ba738764e257d955335 Mon Sep 17 00:00:00 2001 From: Chris Blappert Date: Fri, 20 Jul 2018 17:01:43 -0700 Subject: [PATCH 2/4] Fix wording --- src/serializer/functions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/serializer/functions.js b/src/serializer/functions.js index 7f6121e8b3..5cd996e243 100644 --- a/src/serializer/functions.js +++ b/src/serializer/functions.js @@ -372,7 +372,7 @@ export class Functions { let propString = key ? `${object ? "" : ""}.${key} ` : ""; let objectString = ` ${object}${propString}`; let error = new CompilerDiagnostic( - `Property access${objectString}at${firstLocationString} in "${f1name}" conflicts with write at ${ + `Property modification${objectString}at${firstLocationString} in "${f1name}" conflicts with access at ${ location.start.line }:${location.start.column} in optimized function "${f2name}"`, location, From 592a3ea97e908051f6ef6876e0b3611efdae550f Mon Sep 17 00:00:00 2001 From: Chris Blappert Date: Fri, 20 Jul 2018 17:09:06 -0700 Subject: [PATCH 3/4] Fix flow --- src/serializer/functions.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/serializer/functions.js b/src/serializer/functions.js index 5cd996e243..3859017820 100644 --- a/src/serializer/functions.js +++ b/src/serializer/functions.js @@ -366,7 +366,7 @@ export class Functions { location: BabelNodeSourceLocation, object: string = "", key?: string, - originalLocation?: BabelNodeSourceLocation + originalLocation: BabelNodeSourceLocation | void | null ) => { let firstLocationString = originalLocation ? ` ${stringOfLocation(originalLocation)}` : ""; let propString = key ? `${object ? "" : ""}.${key} ` : ""; @@ -399,11 +399,14 @@ export class Functions { if (!location) return; // happens only when accessing an additional function property if (pbs.has(pb) && !conflicts.has(location)) { let originalLocation = - pb.descriptor && pb.descriptor.value ? pb.descriptor.value.expressionLocation : undefined; + pb.descriptor && pb.descriptor.value && !Array.isArray(pb.descriptor.value) + ? pb.descriptor.value.expressionLocation + : undefined; + let keyString = pb.key instanceof Value ? pb.key.toDisplayString() : pb.key; reportConflict( location, pb.object ? pb.object.intrinsicName || pb.object.__originalName : undefined, - pb.key, + keyString, originalLocation ); } From 75b4229fcb73ec1fbe9bddcfca3ec1392f92cc8d Mon Sep 17 00:00:00 2001 From: Chris Blappert Date: Mon, 23 Jul 2018 18:43:39 -0700 Subject: [PATCH 4/4] Moved getDebugName out --- src/serializer/functions.js | 26 ++++++++++---------------- src/values/FunctionValue.js | 4 ++++ src/values/Value.js | 4 ++++ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/serializer/functions.js b/src/serializer/functions.js index 3859017820..6e97e56808 100644 --- a/src/serializer/functions.js +++ b/src/serializer/functions.js @@ -303,8 +303,7 @@ export class Functions { let fun1Location = fun1.expressionLocation; let fun1Name = this.functionExpressions.get(fun1) || - fun1.intrinsicName || - fun1.__originalName || + fun1.getDebugName() || `(unknown function ${fun1Location ? stringOfLocation(fun1Location) : ""})`; // Also do argument validation here let additionalFunctionEffects = this.writeEffects.get(fun1); @@ -327,8 +326,7 @@ export class Functions { let fun2Location = fun2.expressionLocation; let fun2Name = this.functionExpressions.get(fun2) || - fun2.intrinsicName || - fun2.__originalName || + fun2.getDebugName() || `(unknown function ${fun2Location ? stringOfLocation(fun2Location) : ""})`; let reportFn = () => { this.reportWriteConflicts(fun1Name, fun2Name, conflicts, e1.modifiedProperties, this._callOfFunction(fun2)); @@ -368,13 +366,14 @@ export class Functions { key?: string, originalLocation: BabelNodeSourceLocation | void | null ) => { - let firstLocationString = originalLocation ? ` ${stringOfLocation(originalLocation)}` : ""; - let propString = key ? `${object ? "" : ""}.${key} ` : ""; - let objectString = ` ${object}${propString}`; + let firstLocationString = originalLocation ? `${stringOfLocation(originalLocation)}` : ""; + let propString = key ? ` "${key}"` : ""; + let objectString = object ? ` on object "${object}" ` : ""; + if (!objectString && key) objectString = " on "; let error = new CompilerDiagnostic( - `Property modification${objectString}at${firstLocationString} in "${f1name}" conflicts with access at ${ + `Write to property${propString}${objectString}at optimized function ${f1name}[${firstLocationString}] conflicts with access in function ${f2name}[${ location.start.line - }:${location.start.column} in optimized function "${f2name}"`, + }:${location.start.column}]`, location, "PP1003", "FatalError" @@ -390,7 +389,7 @@ export class Functions { let location = this.realm.currentLocation; invariant(location); if (writtenObjects.has(ob) && !conflicts.has(location)) - reportConflict(location, ob.intrinsicName || ob.__originalName, undefined, ob.expressionLocation); + reportConflict(location, ob.getDebugName(), undefined, ob.expressionLocation); }; let oldReportPropertyAccess = this.realm.reportPropertyAccess; this.realm.reportPropertyAccess = (pb: PropertyBinding) => { @@ -403,12 +402,7 @@ export class Functions { ? pb.descriptor.value.expressionLocation : undefined; let keyString = pb.key instanceof Value ? pb.key.toDisplayString() : pb.key; - reportConflict( - location, - pb.object ? pb.object.intrinsicName || pb.object.__originalName : undefined, - keyString, - originalLocation - ); + reportConflict(location, pb.object ? pb.object.getDebugName() : undefined, keyString, originalLocation); } }; try { diff --git a/src/values/FunctionValue.js b/src/values/FunctionValue.js index ff677f2095..2c603ecc26 100644 --- a/src/values/FunctionValue.js +++ b/src/values/FunctionValue.js @@ -53,4 +53,8 @@ export default class FunctionValue extends ObjectValue { hasDefaultLength(): boolean { invariant(false, "abstract method; please override"); } + + getDebugName(): string { + return super.getDebugName() || this.getName(); + } } diff --git a/src/values/Value.js b/src/values/Value.js index 0e32523a68..6c1fb10251 100644 --- a/src/values/Value.js +++ b/src/values/Value.js @@ -193,4 +193,8 @@ export default class Value { _serialize(set: Function, stack: Map): any { invariant(false, "abstract method; please override"); } + + getDebugName(): string | void { + return this.intrinsicName || this.__originalName; + } }