Summary: | Replace 0 and NULL with nullptr in WebCore/dom. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hunseop Jeong <hs85.jeong> | ||||||||
Component: | WebCore Misc. | Assignee: | Hunseop Jeong <hs85.jeong> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cmarcelo, commit-queue, darin, esprehn+autocc, kangil.han | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Hunseop Jeong
2015-11-01 18:12:50 PST
Created attachment 264548 [details]
Patch
Created attachment 264549 [details]
Patch
Comment on attachment 264549 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264549&action=review Looks good. > Source/WebCore/dom/Attr.cpp:49 > + , m_element(nullptr) This initialization of m_element to nullptr should be done in the header, as you did with m_ignoreChildrenChanged. It’s OK if other constructors initialize it to something else. > Source/WebCore/dom/ContainerNode.cpp:67 > +ChildNodesLazySnapshot* ChildNodesLazySnapshot::latestSnapshot = nullptr; This isn’t needed. A static data member like this will automatically be initialized to null without saying "= nullptr". > Source/WebCore/dom/DocumentOrderedMap.h:-77 > - : element(0) I would have initialized count to zero where it’s defined below too, just like element. > Source/WebCore/dom/Event.cpp:-53 > - , m_currentTarget(0) I would have initialized all those other data members where they are defined too, just like we are doing for m_currentTarget. > Source/WebCore/dom/EventListenerMap.cpp:-201 > - : m_map(0) I would have initialized the other data members where they are defined too. > Source/WebCore/dom/Node.h:707 > + DataUnion() { } I don’t think we need to write this constructor out explicitly. I think it’s what will automatically be generated. > Source/WebCore/dom/PositionIterator.h:40 > - : m_anchorNode(0) > - , m_nodeAfterPositionInAnchor(0) > - , m_offsetInAnchor(0) > + : m_offsetInAnchor(0) I would have initialized m_offsetInAnchor where it's defined too. > Source/WebCore/dom/ProcessingInstruction.cpp:-45 > - , m_cachedSheet(0) I would have initialized those other data members where they are defined too. > Source/WebCore/dom/RangeBoundaryPoint.h:-70 > - , m_childBeforeBoundary(0) I would have initialized m_offsetInContainer where it's defined too. > Source/WebCore/dom/ScriptElement.cpp:60 > + , m_cachedScript(nullptr) This isn’t needed. CachedResourceHandle already initializes itself to null without explicit initialization. > Source/WebCore/dom/StyledElement.cpp:49 > + PresentationAttributeCacheKey() { } I think you can omit this entirely now that it’s empty. You should at least try it. Created attachment 264670 [details]
Patch
(In reply to comment #3) > Comment on attachment 264549 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=264549&action=review > > Looks good. > > > Source/WebCore/dom/Attr.cpp:49 > > + , m_element(nullptr) > > This initialization of m_element to nullptr should be done in the header, as > you did with m_ignoreChildrenChanged. It’s OK if other constructors > initialize it to something else. Okay. I moved the constructor initializer to the header. > > > Source/WebCore/dom/ContainerNode.cpp:67 > > +ChildNodesLazySnapshot* ChildNodesLazySnapshot::latestSnapshot = nullptr; > > This isn’t needed. A static data member like this will automatically be > initialized to null without saying "= nullptr". Oops, I will be kept in mind. > > > Source/WebCore/dom/DocumentOrderedMap.h:-77 > > - : element(0) > > I would have initialized count to zero where it’s defined below too, just > like element. I moved the initialized count where it's defined. > > > Source/WebCore/dom/Event.cpp:-53 > > - , m_currentTarget(0) > > I would have initialized all those other data members where they are defined > too, just like we are doing for m_currentTarget. I moved the constructor iniailizers to they are defined. I will keep working to change the codes that not only using the 'nullptr' but also moving the constructor initailizers to they are defined. > > > Source/WebCore/dom/EventListenerMap.cpp:-201 > > - : m_map(0) > > I would have initialized the other data members where they are defined too. Done. And I removed the default constructor because it is empty. > > > Source/WebCore/dom/Node.h:707 > > + DataUnion() { } > > I don’t think we need to write this constructor out explicitly. I think it’s > what will automatically be generated. I deleted it. > > > Source/WebCore/dom/PositionIterator.h:40 > > - : m_anchorNode(0) > > - , m_nodeAfterPositionInAnchor(0) > > - , m_offsetInAnchor(0) > > + : m_offsetInAnchor(0) > > I would have initialized m_offsetInAnchor where it's defined too. Done. And I removed the default constructor of PositionIterator because it is empty. > > > Source/WebCore/dom/ProcessingInstruction.cpp:-45 > > - , m_cachedSheet(0) > > I would have initialized those other data members where they are defined too. Done. > > > Source/WebCore/dom/RangeBoundaryPoint.h:-70 > > - , m_childBeforeBoundary(0) > > I would have initialized m_offsetInContainer where it's defined too. Done. > > > Source/WebCore/dom/ScriptElement.cpp:60 > > + , m_cachedScript(nullptr) > > This isn’t needed. CachedResourceHandle already initializes itself to null > without explicit initialization. I removed the explicit initailaization. And initialized other data members where they are defined. > > > Source/WebCore/dom/StyledElement.cpp:49 > > + PresentationAttributeCacheKey() { } > > I think you can omit this entirely now that it’s empty. You should at least > try it. Done. Could you check again? Comment on attachment 264670 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264670&action=review > Source/WebCore/dom/IdTargetObserverRegistry.h:43 > + IdTargetObserverRegistry() { } I think this can be removed. Comment on attachment 264670 [details] Patch Clearing flags on attachment: 264670 Committed r191955: <http://trac.webkit.org/changeset/191955> All reviewed patches have been landed. Closing bug. |