Summary: | Add WTF::PoisonedUniquePtr to replace std::unique_ptr when poisoning is desired. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||
Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, darin, ews-watchlist, fpizlo, jfbastien, keith_miller, msaboff, rmorisset, saam, webkit-bug-importer, ysuzuki | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Mark Lam
2017-12-20 16:08:19 PST
Created attachment 330047 [details]
proposed patch.
Attachment 330047 [details] did not pass style-queue:
ERROR: Source/WTF/wtf/PoisonedUniquePtr.h:38: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WTF/wtf/PoisonedUniquePtr.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WTF/wtf/PoisonedUniquePtr.h:116: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WTF/wtf/PoisonedUniquePtr.h:117: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
Total errors found: 4 in 14 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 330047 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=330047&action=review r=me > Source/WTF/wtf/PoisonedUniquePtr.h:38 > + PoisonedUniquePtr() : Base() { } PoisonedUniquePtr() = default; ? > Source/WTF/wtf/PoisonedUniquePtr.h:57 > + return PoisonedUniquePtr(result); Seems like return result; would just work since this constructor is not marked as explicit. > Source/WTF/wtf/PoisonedUniquePtr.h:72 > + ASSERT(this == reinterpret_cast<void*>(&ptr) || this->unpoisoned() != ptr.unpoisoned()); I believe reinterpret_cast<void*>() can change the address, no? wouldn't static_cast<>() be more adequate? > Source/WTF/wtf/PoisonedUniquePtr.h:73 > + if (LIKELY(this != reinterpret_cast<void*>(&ptr))) { ditto. > Source/WTF/wtf/PoisonedUniquePtr.h:116 > + PoisonedUniquePtr() : Base() { } = default; ? > Source/WTF/wtf/PoisonedUniquePtr.h:152 > + ASSERT(this == reinterpret_cast<void*>(&ptr) || this->unpoisoned() != ptr.unpoisoned()); same comment about static_cast. > Source/WTF/wtf/PoisonedUniquePtr.h:153 > + if (LIKELY(this != reinterpret_cast<void*>(&ptr))) { ditto. > Tools/TestWebKitAPI/Tests/WTF/PoisonedUniquePtr.cpp:43 > + ~Logger() { destructCount++; } I believe we usually pre-increment when possible. > Tools/TestWebKitAPI/Tests/WTF/PoisonedUniquePtrForNonTriviallyDestructibleArrays.cpp:44 > + ~Logger() { (*destructCount)++; } pre-increment? > Tools/TestWebKitAPI/Tests/WTF/PoisonedUniquePtrForNonTriviallyDestructibleArrays.cpp:59 > +static const int arraySize = 2; I do not believe we need the static here. Created attachment 330063 [details]
patch for landing.
Thanks for the review. I've updated the patch with the feedback. Wil land when the EWS bots are green.
Attachment 330063 [details] did not pass style-queue:
ERROR: Source/WTF/wtf/PoisonedUniquePtr.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WTF/wtf/PoisonedUniquePtr.h:116: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
Total errors found: 2 in 14 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Landed in r226247: <http://trac.webkit.org/r226247>. |