Bug 164761

Summary: [JSC] WTF::TemporaryChange with WTF::SetForScope
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: saam, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch saam: review+

Yusuke Suzuki
Reported 2016-11-14 20:12:10 PST
It's completely the same.
Attachments
Patch (97.88 KB, patch)
2016-11-16 23:20 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (97.87 KB, patch)
2016-11-16 23:45 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (97.97 KB, patch)
2016-11-17 00:17 PST, Yusuke Suzuki 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2016-11-14 22:49:59 PST
(In reply to comment #0) > It's completely the same. Lol. FWIW, I prefer the name "SetForScope" over "TemporaryChange"
Yusuke Suzuki
Comment 2 2016-11-16 23:20:29 PST
Created attachment 295033 [details] Patch
Yusuke Suzuki
Comment 3 2016-11-16 23:45:39 PST
Created attachment 295035 [details] Patch
Yusuke Suzuki
Comment 4 2016-11-17 00:17:15 PST
Created attachment 295038 [details] Patch
Saam Barati
Comment 5 2016-11-17 00:23:04 PST
Comment on attachment 295038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295038&action=review r=me > Source/WTF/wtf/SetForScope.h:51 > + SetForScope(T& scopedVariable, T newValue) Might be nice to use forward semantics here. > Source/WTF/wtf/SetForScope.h:59 > + m_scopedVariable = m_originalValue; Maybe move semantics?
Yusuke Suzuki
Comment 6 2016-11-17 00:43:38 PST
Comment on attachment 295038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295038&action=review Thanks! >> Source/WTF/wtf/SetForScope.h:51 >> + SetForScope(T& scopedVariable, T newValue) > > Might be nice to use forward semantics here. Yeah, fixed. Using argument deduction and std::forward here. >> Source/WTF/wtf/SetForScope.h:59 >> + m_scopedVariable = m_originalValue; > > Maybe move semantics? Yeah, we can use `WTFMove()`.
Yusuke Suzuki
Comment 7 2016-11-17 00:48:44 PST
Committed r208841: <http://trac.webkit.org/changeset/208841>
Simon Fraser (smfr)
Comment 8 2016-12-22 16:34:54 PST
I think this rename should have been communicated with webkit-dev.
Saam Barati
Comment 9 2016-12-22 18:20:48 PST
(In reply to comment #8) > I think this rename should have been communicated with webkit-dev. Fair enough. We can always name it back.
Saam Barati
Comment 10 2016-12-22 18:23:54 PST
(In reply to comment #9) > (In reply to comment #8) > > I think this rename should have been communicated with webkit-dev. > > Fair enough. We can always name it back. I'll send out an email to webkit-dev now.
Simon Fraser (smfr)
Comment 11 2016-12-23 11:17:07 PST
Would have also been nice to have some discussion about naming. This comes through as picking the JSC name by fiat.
Note You need to log in before you can comment on or make changes to this bug.