WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
164761
[JSC] WTF::TemporaryChange with WTF::SetForScope
https://bugs.webkit.org/show_bug.cgi?id=164761
Summary
[JSC] WTF::TemporaryChange with WTF::SetForScope
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
Details
Formatted Diff
Diff
Patch
(97.87 KB, patch)
2016-11-16 23:45 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(97.97 KB, patch)
2016-11-17 00:17 PST
,
Yusuke Suzuki
saam
: review+
Details
Formatted Diff
Diff
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.
Top of Page
Format For Printing
XML
Clone This Bug