[webkit-reviews] review granted: [Bug 18774] SQUIRRELFISH: print meaningful error messages : [Attachment 22359] Patch o' doom

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 18 14:02:18 PDT 2008

Cameron Zwarich (cpst) <cwzwarich at uwaterloo.ca> has granted Oliver Hunt
<oliver at apple.com>'s request for review:
Bug 18774: SQUIRRELFISH: print meaningful error messages

Attachment 22359: Patch o' doom

------- Additional Comments from Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>
+int CodeBlock::lineNumberForVPC(const Instruction* vPC) {

The opening brace should be on the next line.

+	 * kjs/SourceRange.h:

This is in the wrong position of the header list.

+		 divot = 0; // Overflow has occurred, we can only give line
number info for errors for this region

Might as well put the comment on its own line(s) like all the others in this
if-else statement.

+    JSObject* exception = Error::create(exec, TypeError,
createErrorMessage(exec, codeBlock, line, divotPoint, divotPoint + endOffset,
value, message), line, codeBlock->ownerNode->sourceId(),

Maybe it would be better to break off the createErrorMessage() call into its
own line, because this line is already quite long. This goes for the other
creations of errors as well.

+	 int32_t m_startOffset;

I don't think you want the extra line here.

Other than that, it looks good, so r=me with those changes. Do you have any
tests for your new exception messages and line/column info?

More information about the webkit-reviews mailing list