Bug 40128 - Regressions in debug functionality
Summary: Regressions in debug functionality
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Trivial
Assignee: Nathan Lawrence
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-03 10:13 PDT by Nathan Lawrence
Modified: 2010-06-24 11:29 PDT (History)
5 users (show)

See Also:


Attachments
suggested patch (2.81 KB, patch)
2010-06-03 10:13 PDT, Nathan Lawrence
no flags Details | Formatted Diff | Diff
fixed style issue (2.81 KB, patch)
2010-06-03 10:44 PDT, Nathan Lawrence
levin: review-
Details | Formatted Diff | Diff
Changelogged (3.80 KB, patch)
2010-06-03 15:22 PDT, Nathan Lawrence
no flags Details | Formatted Diff | Diff
changed #if !defined to #ifndef (3.78 KB, patch)
2010-06-21 13:12 PDT, Nathan Lawrence
ggaren: review+
ggaren: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nathan Lawrence 2010-06-03 10:13:31 PDT
Created attachment 57785 [details]
suggested patch

There are a few things that don't work anymore but are useful for debugging:

* JS_ZOMBIES doesn't compile anymore.

* The dumpCallFrame function in the interpreter causes a runtime error because JSValue::description does not match the current state of JSValue

* The dumpRegisters function in the interpreter is not up to date with the most recent call frame patches.

* Additionally it would be nice if JSValue::isCell checked that the value was aligned.
Comment 1 WebKit Review Bot 2010-06-03 10:16:04 PDT
Attachment 57785 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/runtime/JSImmediate.h:47:  CELL_MASK is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/runtime/JSImmediate.h:602:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 2 Nathan Lawrence 2010-06-03 10:44:04 PDT
Created attachment 57788 [details]
fixed style issue

I did not fix the issue with having underscores in the name because it is a constant defined in Collector.h
Comment 3 WebKit Review Bot 2010-06-03 10:46:15 PDT
Attachment 57788 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/runtime/JSImmediate.h:47:  CELL_MASK is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 David Levin 2010-06-03 13:46:24 PDT
Comment on attachment 57788 [details]
fixed style issue

I won't be able to review this, but I can let you know that you'll need a ChangeLog to make it through the review.
Comment 5 Nathan Lawrence 2010-06-03 15:22:16 PDT
Created attachment 57820 [details]
Changelogged
Comment 6 WebKit Review Bot 2010-06-03 15:23:30 PDT
Attachment 57820 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/runtime/JSImmediate.h:47:  CELL_MASK is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Darin Adler 2010-06-12 19:07:05 PDT
Comment on attachment 57820 [details]
Changelogged

> +#if !defined(NDEBUG)

#ifndef NDEBUG is more idiomatic for this sort of thing.

The rest of this looks fine.
Comment 8 Nathan Lawrence 2010-06-21 13:12:28 PDT
Created attachment 59280 [details]
changed #if !defined to #ifndef
Comment 9 WebKit Review Bot 2010-06-21 13:14:22 PDT
Attachment 59280 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/runtime/JSImmediate.h:47:  CELL_MASK is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Geoffrey Garen 2010-06-21 13:16:32 PDT
Comment on attachment 59280 [details]
changed #if !defined to #ifndef

r=me

When might JSValue::description() want to print INVALID()? (That's the kind of information that can be useful in a ChangeLog -- an explanation of "why", in addition to "what".)
Comment 11 Eric Seidel (no email) 2010-06-22 13:23:10 PDT
Comment on attachment 57820 [details]
Changelogged

Cleared Darin Adler's review+ from obsolete attachment 57820 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 12 Steve Falkenburg 2010-06-24 11:29:50 PDT
http://trac.webkit.org/changeset/61778