Bug 171861

Summary: Force StaticStringImpl constructor to use the constexpr versions of StringImplShape constructors.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, cdumez, cmarcelo, dbates, fujii.hironori, pvollan, saam, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 171800    
Attachments:
Description Flags
proposed patch.
ysuzuki: review+
patch for landing. none

Mark Lam
Reported 2017-05-09 08:50:47 PDT
This is strictly necessary for correctness of the StaticStringImpl implementation.
Attachments
proposed patch. (3.33 KB, patch)
2017-05-09 09:12 PDT, Mark Lam 		ysuzuki: review+
DetailsFormatted DiffDiff
patch for landing. (3.52 KB, patch)
2017-05-09 09:25 PDT, Mark Lam 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2017-05-09 09:12:04 PDT
Created attachment 309503 [details] proposed patch.
Yusuke Suzuki
Comment 2 2017-05-09 09:15:21 PDT
Comment on attachment 309503 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=309503&action=review r=me with nits. > Source/WTF/wtf/text/StringImpl.h:150 > + constexpr StringImplShape(unsigned refCount, unsigned length, const char (&characters)[charactersCount], unsigned hashAndFlags, int) I like to use enum for these taggings like, CreateSymbolTag in StringImpl.h. > Source/WTF/wtf/text/StringImpl.h:158 > + constexpr StringImplShape(unsigned refCount, unsigned length, const char16_t (&characters)[charactersCount], unsigned hashAndFlags, int) Ditto.
Mark Lam
Comment 3 2017-05-09 09:16:31 PDT
(In reply to Yusuke Suzuki from comment #2) > Comment on attachment 309503 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309503&action=review > > r=me with nits. > > > Source/WTF/wtf/text/StringImpl.h:150 > > + constexpr StringImplShape(unsigned refCount, unsigned length, const char (&characters)[charactersCount], unsigned hashAndFlags, int) > > I like to use enum for these taggings like, CreateSymbolTag in StringImpl.h. Good idea. Will do.
Mark Lam
Comment 4 2017-05-09 09:25:12 PDT
Created attachment 309505 [details] patch for landing.
Mark Lam
Comment 5 2017-05-09 10:12:09 PDT
Thanks for the review. Landed in r216512: <http://trac.webkit.org/r216512>.
Saam Barati
Comment 6 2017-05-09 12:05:23 PDT
Comment on attachment 309505 [details] patch for landing. View in context: https://bugs.webkit.org/attachment.cgi?id=309505&action=review > Source/WTF/wtf/text/StringImpl.h:160 > + constexpr StringImplShape(unsigned refCount, unsigned length, const char16_t (&characters)[charactersCount], unsigned hashAndFlags, ConstructWithConstExprTag) Style nit: I feel like we usually put these tags as the first argument, not the last.
Note You need to log in before you can comment on or make changes to this bug.