[Webkit-unassigned] [Bug 188598] [JSC] Remove gcc warnings on mips and armv7

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188598

--- Comment #20 from Guillaume Emont <guijemont at igalia.com> ---
(In reply to Mark Lam from comment #19)
> Comment on attachment 349756 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349756&action=review
> 
> > Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp:140
> > +        // assuming memory is not malformed, it originately pointed to a value
> 
> /originately/originally/

Sorry, I don't know why, I keep on convincing myself that this is a word ;).

> 
> > Source/JavaScriptCore/interpreter/Interpreter.cpp:814
> > +        IGNORE_CAST_ALIGN_WARNINGS_BEGIN
> >          EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(error));
> > +        IGNORE_CAST_ALIGN_WARNINGS_END
> 
> Instead of doing this, can you just change prepareForExecution() to return a
> std::optional<Exception*>?  That should make the code cleaner and better
> document what will actually be returned.  That also removes the need for all
> these reinterpret_casts.  I think the JSObject* return value is a holdover
> from ancient times.

I'd be happy to do so. I did not do it *originally* because I assumed (wrongly I guess, from your request) that there was a reason-I-don't-understand for prepareForExecution() to be that way and didn't want to risk breaking something.

> 
> > Source/bmalloc/bmalloc/IsoPageInlines.h:197
> > +        // the cast below is safe on these platforms as long as
> > +        // both Config::objectSize and the alignment of this are multiples of
> > +        // the required alignment of FreeCell
> > +        static_assert(!(Config::objectSize % alignof(FreeCell)), "Config::objectSize should respect alignment of FreeCell");
> > +        static_assert(!(alignof(IsoPage<Config>) % alignof(FreeCell)), "Alignment of IsoPage<Config> should match that of FreeCell");
> 
> Shouldn't these be always true?  I think we can get rid of the #if CPU(ARM)
> || CPU(MIPS) here and below.  You can also remove the comment.  The
> assertions are sufficient to document the invariant / requirement.

Right. I will do that.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180917/81fe3a88/attachment.html>