LGTM On Thu, Jul 2, 2009 at 5:19 PM, Christian Plesner Hansen<[email protected]> wrote: > Mads, > Rietveld seems to be on the fritz and I'm in a hurry so here's a > manual code review. > > Stack trace fixes > Changed stack trace format to be more regular and less redundant. > Changed tests to reflect changes. > > diff --git a/src/messages.js b/src/messages.js > index 6bfdd45..6157874 100644 > --- a/src/messages.js > +++ b/src/messages.js > @@ -609,7 +609,6 @@ CallSite.prototype.getTypeName = function () { > CallSite.prototype.isToplevel = function () { > if (this.receiver == null) > return true; > - var className = $Object.prototype.toString.call(this.receiver); > return IS_GLOBAL(this.receiver); > }; > > @@ -626,6 +625,10 @@ CallSite.prototype.getEvalOrigin = function () { > script.eval_from_position); > }; > > +CallSite.prototype.getFunction = function () { > + return this.fun; > +}; > + > CallSite.prototype.getFunctionName = function () { > // See if the function knows its own name > var name = this.fun.name; > @@ -644,6 +647,11 @@ CallSite.prototype.getFunctionName = function () { > CallSite.prototype.getMethodName = function () { > // See if we can find a unique property on the receiver that holds > // this function. > + var ownName = this.fun.name; > + if (ownName && this.receiver && this.receiver[ownName] === this.fun) > + // To handle DontEnum properties we guess that the method has > + // the same name as the function. > + return ownName; > var name = null; > for (var prop in this.receiver) { > if (this.receiver[prop] === this.fun) { > @@ -725,15 +733,23 @@ function FormatSourcePosition(frame) { > fileLocation = "unknown source"; > } > var line = ""; > + var functionName = frame.getFunction().name; > var methodName = frame.getMethodName(); > - var functionName = frame.getFunctionName(); > var addPrefix = true; > - if (frame.isToplevel()) { > - line += functionName; > - } else if (frame.isConstructor()) { > - line += "new " + functionName; > - } else if (methodName) { > - line += frame.getTypeName() + "." + methodName; > + var isConstructor = frame.isConstructor(); > + var isMethodCall = !(frame.isToplevel() || isConstructor); > + if (isMethodCall) { > + line += frame.getTypeName() + "."; > + if (functionName) { > + line += functionName; > + if (methodName && (methodName != functionName)) { > + line += " [as " + methodName + "]"; > + } > + } else { > + line += methodName || "<anonymous>"; > + } > + } else if (isConstructor) { > + line += "new " + (functionName || "<anonymous>"); > } else if (functionName) { > line += functionName; > } else { > @@ -741,11 +757,7 @@ function FormatSourcePosition(frame) { > addPrefix = false; > } > if (addPrefix) { > - line += " ("; > - if (functionName) { > - line += functionName + " @ "; > - } > - line += fileLocation + ")"; > + line += " (" + fileLocation + ")"; > } > return line; > } > diff --git a/test/mjsunit/stack-traces.js b/test/mjsunit/stack-traces.js > index d708951..e457ece 100644 > --- a/test/mjsunit/stack-traces.js > +++ b/test/mjsunit/stack-traces.js > @@ -73,6 +73,17 @@ function testConstructor() { > new Plonk(); > } > > +function testRenamedMethod() { > + function a$b$c$d() { return FAIL; } > + function Wookie() { } > + Wookie.prototype.d = a$b$c$d; > + (new Wookie).d(); > +} > + > +function testAnonymousMethod() { > + (function () { FAIL }).call([1, 2, 3]); > +} > + > // Utility function for testing that the expected strings occur > // in the stack trace produced when running the given function. > function testTrace(fun, expected) { > @@ -149,9 +160,11 @@ testTrace(testNested, ["at one", "at two", "at three"]); > testTrace(testMethodNameInference, ["at Foo.bar"]); > testTrace(testImplicitConversion, ["at Nirk.valueOf"]); > testTrace(testEval, ["at Doo (eval at testEval"]); > -testTrace(testNestedEval, ["at eval (eval at Inner (eval at Outer"]); > +testTrace(testNestedEval, ["eval at Inner (eval at Outer"]); > testTrace(testValue, ["at Number.causeError"]); > testTrace(testConstructor, ["new Plonk"]); > +testTrace(testRenamedMethod, ["Wookie.a$b$c$d [as d]"]); > +testTrace(testAnonymousMethod, ["Array.<anonymous>"]); > > testCallerCensorship(); > testUnintendedCallerCensorship(); > diff --git a/test/mozilla/mozilla.status b/test/mozilla/mozilla.status > index 760ed41..13ae29c 100644 > --- a/test/mozilla/mozilla.status > +++ b/test/mozilla/mozilla.status > @@ -476,12 +476,11 @@ js1_2/Array/array_split_1: FAIL_OK > js1_5/Array/regress-313153: FAIL_OK > > > -# Properties stack, fileName, and lineNumber of Error instances are > +# Properties fileName, and lineNumber of Error instances are > # not supported. Mozilla specific extension. > js1_5/Exceptions/errstack-001: FAIL_OK > js1_5/Exceptions/regress-257751: FAIL_OK > js1_5/Regress/regress-119719: FAIL_OK > -js1_5/Regress/regress-139316: FAIL_OK > js1_5/Regress/regress-167328: FAIL_OK > js1_5/Regress/regress-243869: FAIL_OK >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
