Bug 60532 – Fixed up some #include dependencies so the WriteBarrier class can actually call Heap::writeBarrier

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Geoffrey Garen]

Fixed up some #include dependencies so the WriteBarrier class can actually call Heap::writeBarrier

[Geoffrey Garen]

Created attachment 92917 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 92917 [details]
Patch

Attachment 92917 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/8655802

[Geoffrey Garen]

Created attachment 92922 [details]
Patch

[WebKit Review Bot]

Comment on attachment 92917 [details]
Patch

Attachment 92917 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8655819

[WebKit Review Bot]

Comment on attachment 92922 [details]
Patch

Attachment 92922 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8645819

[Darin Adler]

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.

[Geoffrey Garen]

> 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.

[Geoffrey Garen]

Committed r86209: <http://trac.webkit.org/changeset/86209>