Bug 60532 - Fixed up some #include dependencies so the WriteBarrier class can actually call Heap::writeBarrier
Summary: Fixed up some #include dependencies so the WriteBarrier class can actually ca...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-09 21:54 PDT by Geoffrey Garen
Modified: 2011-05-10 19:29 PDT (History)
1 user (show)

See Also:

Attachments
Patch (15.05 KB, patch)
2011-05-09 21:58 PDT, Geoffrey Garen 		webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (17.50 KB, patch)
2011-05-09 22:35 PDT, Geoffrey Garen 		darin: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2011-05-09 21:54:11 PDT 
Fixed up some #include dependencies so the WriteBarrier class can actually call Heap::writeBarrier
Comment 1 Geoffrey Garen 2011-05-09 21:58:18 PDT 
Created attachment 92917 [details]
Patch
Comment 2 Gyuyoung Kim 2011-05-09 22:14:05 PDT 
Comment on attachment 92917 [details]
Patch

Attachment 92917 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/8655802
Comment 3 Geoffrey Garen 2011-05-09 22:35:43 PDT 
Created attachment 92922 [details]
Patch
Comment 4 WebKit Review Bot 2011-05-09 23:00:15 PDT 
Comment on attachment 92917 [details]
Patch

Attachment 92917 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8655819
Comment 5 WebKit Review Bot 2011-05-09 23:44:57 PDT 
Comment on attachment 92922 [details]
Patch

Attachment 92922 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8645819
Comment 6 Darin Adler 2011-05-10 08:54:34 PDT 
Comment on attachment 92922 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=92922&action=review

r=me, but I think you need to find out why this failed on the Mac EWS

> Source/JavaScriptCore/heap/MarkStack.h:42
> +    template <typename T> class WriteBarrierBase;

I think we should consider a style guideline of omitting the space between the word template and the <> arguments, by analogy with how there is no space after a function name before its () arguments.

> Source/JavaScriptCore/heap/MarkStack.h:65
> +        inline void appendValues(WriteBarrierBase<Unknown>*, size_t count, MarkSetProperties = NoNullValues);

I believe the inline keyword here has no effect and so should be omitted.
Comment 7 Geoffrey Garen 2011-05-10 14:04:46 PDT 
> r=me, but I think you need to find out why this failed on the Mac EWS

This builds locally, so I think the EWS may have gotten confused with the other path attached here. I'll watch the bots to make sure.

> > Source/JavaScriptCore/heap/MarkStack.h:42
> > +    template <typename T> class WriteBarrierBase;
> 
> I think we should consider a style guideline of omitting the space between the word template and the <> arguments, by analogy with how there is no space after a function name before its () arguments.

You keep telling me this, and I keep forgetting. Maybe today's the day I remember?

> > Source/JavaScriptCore/heap/MarkStack.h:65
> > +        inline void appendValues(WriteBarrierBase<Unknown>*, size_t count, MarkSetProperties = NoNullValues);
> 
> I believe the inline keyword here has no effect and so should be omitted.

Actually, it opts you in to this warning in the case where you forget to #include the defining header: "scratch.cc:9: warning: inline function ‘void f()’ used but never defined". I think that's a little better than just getting a linker error, since linker errors on Mac OS X can be cryptic.
Comment 8 Geoffrey Garen 2011-05-10 19:29:55 PDT 
Committed r86209: <http://trac.webkit.org/changeset/86209>