RESOLVED FIXED 127276
Move TreeScope, IdTargetObserverRegistry to std::unique_ptr
https://bugs.webkit.org/show_bug.cgi?id=127276
Summary Move TreeScope, IdTargetObserverRegistry to std::unique_ptr
Zan Dobersek
Reported 2014-01-20 02:28:56 PST
Move TreeScope, IdTargetObserverRegistry to std::unique_ptr
Attachments
Patch (8.41 KB, patch)
2014-01-20 02:31 PST, Zan Dobersek
no flags
Patch (8.37 KB, patch)
2014-01-20 22:34 PST, Zan Dobersek
kling: review+
Zan Dobersek
Comment 1 2014-01-20 02:31:22 PST
Zan Dobersek
Comment 2 2014-01-20 22:34:55 PST
Brent Fulgham
Comment 3 2014-01-21 12:08:08 PST
Comment on attachment 221721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221721&action=review This looks great! I would prefer we not use the 'get()' method in the shouldCacheLabelsByForAttribute. Can you confirm it must be implemented the way you have written that? > Source/WebCore/dom/TreeScope.h:83 > + bool shouldCacheLabelsByForAttribute() const { return m_labelsByForAttribute.get(); } std::unique_ptr has an operator (bool). Why is the 'get()' necessary here? At the worst, you should be doing something like "static_cast<bool>(m_labelsByForAttribute)".
Anders Carlsson
Comment 4 2014-01-21 12:38:46 PST
(In reply to comment #3) > (From update of attachment 221721 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221721&action=review > > This looks great! I would prefer we not use the 'get()' method in the shouldCacheLabelsByForAttribute. Can you confirm it must be implemented the way you have written that? > > > Source/WebCore/dom/TreeScope.h:83 > > + bool shouldCacheLabelsByForAttribute() const { return m_labelsByForAttribute.get(); } > > std::unique_ptr has an operator (bool). Why is the 'get()' necessary here? At the worst, you should be doing something like "static_cast<bool>(m_labelsByForAttribute)". the bool operator is explicit. I think .get() is better than an explicit static cast.
Andreas Kling
Comment 5 2014-02-06 12:57:31 PST
Comment on attachment 221721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221721&action=review r=me >>> Source/WebCore/dom/TreeScope.h:83 >>> + bool shouldCacheLabelsByForAttribute() const { return m_labelsByForAttribute.get(); } >> >> std::unique_ptr has an operator (bool). Why is the 'get()' necessary here? At the worst, you should be doing something like "static_cast<bool>(m_labelsByForAttribute)". > > the bool operator is explicit. I think .get() is better than an explicit static cast. How about "return !!m_labelsByForAttribute;"
Zan Dobersek
Comment 6 2014-02-08 11:58:30 PST
Comment on attachment 221721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221721&action=review >>>> Source/WebCore/dom/TreeScope.h:83 >>>> + bool shouldCacheLabelsByForAttribute() const { return m_labelsByForAttribute.get(); } >>> >>> std::unique_ptr has an operator (bool). Why is the 'get()' necessary here? At the worst, you should be doing something like "static_cast<bool>(m_labelsByForAttribute)". >> >> the bool operator is explicit. I think .get() is better than an explicit static cast. > > How about "return !!m_labelsByForAttribute;" Sure.
Zan Dobersek
Comment 7 2014-02-08 12:05:19 PST
Note You need to log in before you can comment on or make changes to this bug.