[webkit-reviews] review granted: [Bug 20175] Fixes for Windows and non-AllInOne file build with SamplingTool, plus review fixes. : [Attachment 22487] minor fix - add missing padding for DUMP_OPCODE_STATS

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jul 29 14:43:26 PDT 2008


Geoffrey Garen <ggaren at apple.com> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 20175: Fixes for Windows and non-AllInOne file build with SamplingTool,
plus review fixes.
https://bugs.webkit.org/show_bug.cgi?id=20175

Attachment 22487: minor fix - add missing padding for DUMP_OPCODE_STATS
https://bugs.webkit.org/attachment.cgi?id=22487&action=edit

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
+// Set to 1 to enable the sampler, SamplingTool.
+#define ENABLE_SAMPLING_TOOL 0

It's best to put #defines like this in wtf/Platform.h, rather than config.h.
config.h is just there for crazy header hacks -- for example, the MSVC rand_s
hack.

--- JavaScriptCore/VM/SamplingTool.cpp	(revision 0)
+++ JavaScriptCore/VM/SamplingTool.cpp	(revision 0)

I think this means that SamplingTool.cpp made its way into the project through
a local filesystem copy, followed by an "svn add." It's better to use "svn cp
Opcode.h SamplingTool.cpp," because the latter will preserve the history of
your original commits within the new source file.

+    class SamplingTool
+    {

Our style guidelines call for the "{" in a class declaration to go on the same
line as the declaration, as in "class SamplingTool {."

r=me -- great improvements!


More information about the webkit-reviews mailing list