WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
150788
Replace 0 and NULL with nullptr in WebCore/dom.
https://bugs.webkit.org/show_bug.cgi?id=150788
Summary
Replace 0 and NULL with nullptr in WebCore/dom.
Hunseop Jeong
Reported
2015-11-01 18:12:50 PST
Use the 'nullptr' instead of '0' and 'NULL' in WebCore/dom.
Attachments
Patch
(48.58 KB, patch)
2015-11-01 18:17 PST
,
Hunseop Jeong
no flags
Details
Formatted Diff
Diff
Patch
(47.97 KB, patch)
2015-11-01 18:46 PST
,
Hunseop Jeong
no flags
Details
Formatted Diff
Diff
Patch
(53.17 KB, patch)
2015-11-02 23:08 PST
,
Hunseop Jeong
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Hunseop Jeong
Comment 1
2015-11-01 18:17:36 PST
Created
attachment 264548
[details]
Patch
Hunseop Jeong
Comment 2
2015-11-01 18:46:30 PST
Created
attachment 264549
[details]
Patch
Darin Adler
Comment 3
2015-11-01 22:00:58 PST
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.
Hunseop Jeong
Comment 4
2015-11-02 23:08:11 PST
Created
attachment 264670
[details]
Patch
Hunseop Jeong
Comment 5
2015-11-02 23:09:58 PST
(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?
Darin Adler
Comment 6
2015-11-03 09:37:59 PST
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.
WebKit Commit Bot
Comment 7
2015-11-03 10:25:27 PST
Comment on
attachment 264670
[details]
Patch Clearing flags on attachment: 264670 Committed
r191955
: <
http://trac.webkit.org/changeset/191955
>
WebKit Commit Bot
Comment 8
2015-11-03 10:25:31 PST
All reviewed patches have been landed. Closing bug.
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