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, Hironori.Fujii, 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

Description Mark Lam 2017-05-09 08:50:47 PDT
This is strictly necessary for correctness of the StaticStringImpl implementation.
Comment 1 Mark Lam 2017-05-09 09:12:04 PDT
Created attachment 309503 [details]
proposed patch.
Comment 2 Yusuke Suzuki 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.
Comment 3 Mark Lam 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.
Comment 4 Mark Lam 2017-05-09 09:25:12 PDT
Created attachment 309505 [details]
patch for landing.
Comment 5 Mark Lam 2017-05-09 10:12:09 PDT
Thanks for the review.  Landed in r216512: <http://trac.webkit.org/r216512>.
Comment 6 Saam Barati 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.