Bug 160344

Summary: Make StringView capable of being passed /returned in only 2 registers.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: Web Template FrameworkAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, benjamin, cdumez, cmarcelo, commit-queue, darin, ddkilzer, fpizlo, ggaren, keith_miller, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 160350    
Bug Blocks:    
Attachments:
Description Flags
proposed patch. ggaren: review+

Description Mark Lam 2016-07-29 10:12:52 PDT
We just need to #if out copy and move constructors and assignment operators.
Comment 1 Mark Lam 2016-07-29 10:30:33 PDT
Created attachment 284872 [details]
proposed patch.
Comment 2 Geoffrey Garen 2016-07-29 10:49:08 PDT
Comment on attachment 284872 [details]
proposed patch.

/Volumes/Data/EWS/WebKit/Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:615:13: error: unused variable 'selectionTextRun' [-Werror,-Wunused-variable]
    TextRun selectionTextRun = constructTextRun(selectionStyle, fragment);
Comment 3 Mark Lam 2016-07-29 10:51:56 PDT
(In reply to comment #2)
> Comment on attachment 284872 [details]
> proposed patch.
> 
> /Volumes/Data/EWS/WebKit/Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:
> 615:13: error: unused variable 'selectionTextRun' [-Werror,-Wunused-variable]
>     TextRun selectionTextRun = constructTextRun(selectionStyle, fragment);

Does not appear to be related to my patch at all.
Comment 4 Geoffrey Garen 2016-07-29 10:57:56 PDT
Comment on attachment 284872 [details]
proposed patch.

r=me
Comment 5 Mark Lam 2016-07-29 11:32:10 PDT
Comment on attachment 284872 [details]
proposed patch.

I'll land this manually later.
Comment 6 Mark Lam 2016-07-29 11:54:43 PDT
Thanks for the review.  Landed in r203911: <http://trac.webkit.org/r203911>.
Comment 7 Darin Adler 2016-07-29 12:01:11 PDT
Thanks, Mark. This is great!
Comment 8 Mark Lam 2016-07-29 12:30:13 PDT
Follow up build fix in r203915: <http://trac.webkit.org/r203915>.
Comment 9 Radar WebKit Bug Importer 2016-07-29 12:32:16 PDT
<rdar://problem/27613944>
Comment 10 Alexey Proskuryakov 2016-07-29 12:43:02 PDT
> #if defined(NDEBUG) || COMPILER(MSVC) || 1
> #define CHECK_STRINGVIEW_LIFETIME 0

This is pre-existing code, however I'm curious anyway - is there a bug tracking re-enabling the checks?
Comment 11 Darin Adler 2016-07-29 12:54:51 PDT
Wow, did not realize these were turned off. Do we know exactly when we turned them off and why? Should find out when that || 1 was added.
Comment 12 Mark Lam 2016-07-29 12:57:13 PDT
(In reply to comment #11)
> Wow, did not realize these were turned off. Do we know exactly when we
> turned them off and why? Should find out when that || 1 was added.

See https://trac.webkit.org/changeset/174397 from Oct 2014.

(In reply to comment #10)
> This is pre-existing code, however I'm curious anyway - is there a bug
> tracking re-enabling the checks?

Apparently, no.
Comment 13 Mark Lam 2016-07-30 14:24:36 PDT
Will work on re-enabling the StringView life-cycle checks in https://bugs.webkit.org/show_bug.cgi?id=160384.