[webkit-reviews] review canceled: [Bug 185631] TestWebKitAPI: Fix warnings found by new clang compiler : [Attachment 340365] Patch v1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 14 15:44:06 PDT 2018


David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has canceled  review:
Bug 185631: TestWebKitAPI: Fix warnings found by new clang compiler
https://bugs.webkit.org/show_bug.cgi?id=185631

Attachment 340365: Patch v1

https://bugs.webkit.org/attachment.cgi?id=340365&action=review




--- Comment #5 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 340365
  --> https://bugs.webkit.org/attachment.cgi?id=340365
Patch v1

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

>> Tools/TestWebKitAPI/Tests/WTF/NakedPtr.cpp:178
>> +#pragma clang diagnostic ignored "-Wself-assign-overloaded"
> 
> I think it would be good form to use two pops now to return the diagnostic
stack to its original state, not that it matters.

That's not how #pragma clang diagnostic push works.   See the next function
below this.

>> Tools/TestWebKitAPI/Tests/WebKit/FindMatches.mm:88
>> +		unusedRect =
WKRectGetValue(reinterpret_cast<WKRectRef>(items));
> 
> Can you not just drop the return value? Or cast the result to void?

Since it returns a struct, I didn't check whether I could cast to void.

I also toyed with adding some assertions (like EXPECT_TRUE(rect.size.width != 0
&& rect.size.height != 0).  Maybe I'll do that instead.


More information about the webkit-reviews mailing list