Use the 'nullptr' instead of '0' and 'NULL' in WebCore/editing.
Created attachment 264042 [details] Patch
Comment on attachment 264042 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264042&action=review > Source/WebCore/editing/ApplyStyleCommand.cpp:133 > - , m_isInlineElementToRemoveFunction(0) > + , m_isInlineElementToRemoveFunction(nullptr) Better in a case like this to move the initialization to nullptr to the class definition and remove it from all of the constructors. > Source/WebCore/editing/ApplyStyleCommand.cpp:563 > + RefPtr<ContainerNode> startDummySpanAncestor = nullptr; > + RefPtr<ContainerNode> endDummySpanAncestor = nullptr; No need to initialize RefPtr to null; it’s null even if you don’t specify anything. It’s only raw pointers that need nullptr. > Source/WebCore/editing/DeleteSelectionCommand.cpp:108 > + , m_startBlock(nullptr) > + , m_endBlock(nullptr) > + , m_typingStyle(nullptr) > + , m_deleteIntoBlockquoteStyle(nullptr) Better in a case like this to move the initialization to nullptr to the class definition and remove it from the constructor. > Source/WebCore/editing/EditCommand.cpp:43 > + , m_parent(nullptr) Better in a case like this to move the initialization to nullptr to the class definition and remove it from all of the constructors. > Source/WebCore/editing/EditingStyle.cpp:178 > + , m_tagName(nullptr) Better in a case like this to move the initialization to nullptr to the class definition and remove it from all of the constructors. > Source/WebCore/editing/EditorCommand.cpp:1680 > + : m_command(nullptr) Better in a case like this to move the initialization to nullptr to the class definition and remove it from all of the constructors. > Source/WebCore/editing/RenderedPosition.cpp:68 > + : m_renderer(nullptr) > + , m_inlineBox(nullptr) Better in a case like this to move the initialization to nullptr to the class definition and remove it from all of the constructors. > Source/WebCore/editing/RenderedPosition.h:96 > + : m_renderer(nullptr) > + , m_inlineBox(nullptr) Better in a case like this to move the initialization to nullptr to the class definition and remove it from all of the constructors. > Source/WebCore/editing/SmartReplaceCF.cpp:40 > + static CFMutableCharacterSetRef preSmartSet = nullptr; > + static CFMutableCharacterSetRef postSmartSet = nullptr; No need to initialize a global variable (static) to nullptr. That will happen even if it’s not specified. Better to just remove the initialization entirely. > Source/WebCore/editing/SpellChecker.cpp:47 > + : m_checker(nullptr) Better in a case like this to move the initialization to nullptr to the class definition and remove it from the constructor. > Source/WebCore/editing/VisibleUnits.cpp:140 > +CachedLogicallyOrderedLeafBoxes::CachedLogicallyOrderedLeafBoxes() : m_rootInlineBox(nullptr) { }; Better in a case like this to move the initialization to nullptr to the class definition and remove it from the constructor. Also, there’s an extra semicolon here. And no good reason to format this all on one long line instead of normal formatting. > Source/WebCore/editing/ios/EditorIOS.mm:125 > + const char *newValue = nil; This use of nil is incorrect. You should only use nil for Objective-C object pointers, not plain C ones. It would be OK to use nullptr here. > Source/WebCore/editing/ios/EditorIOS.mm:207 > + const Font* font = nil; This use of nil is incorrect. You should only use nil for Objective-C object pointers, not C++ object pointers. It would be OK to use nullptr here. > Source/WebCore/editing/mac/EditorMac.mm:130 > + const Font* font = nil; This use of nil is incorrect. You should only use nil for Objective-C object pointers, not C++ object pointers. It would be OK to use nullptr here.
Created attachment 264125 [details] Patch
(In reply to comment #2) > Comment on attachment 264042 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=264042&action=review > > > Source/WebCore/editing/ApplyStyleCommand.cpp:133 > > - , m_isInlineElementToRemoveFunction(0) > > + , m_isInlineElementToRemoveFunction(nullptr) > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from all of the constructors. I moved the initialization to nullptr to the class definition and remove it from all of the constructors except the 'm_isInlineElementToRemoveFunction(isInlineElementToRemoveFunction)' in the last constructor. > > > Source/WebCore/editing/ApplyStyleCommand.cpp:563 > > + RefPtr<ContainerNode> startDummySpanAncestor = nullptr; > > + RefPtr<ContainerNode> endDummySpanAncestor = nullptr; > > No need to initialize RefPtr to null; it’s null even if you don’t specify > anything. It’s only raw pointers that need nullptr. I see. Do I leave the RefPtr without initialization in this case? > > > Source/WebCore/editing/DeleteSelectionCommand.cpp:108 > > + , m_startBlock(nullptr) > > + , m_endBlock(nullptr) > > + , m_typingStyle(nullptr) > > + , m_deleteIntoBlockquoteStyle(nullptr) > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from the constructor. I removed the initialization to nullptr from all of the contructors but I didn't move the initialization to class definition because it was RefPtr. > > > Source/WebCore/editing/EditCommand.cpp:43 > > + , m_parent(nullptr) > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from all of the constructors. I moved the initialization to nullptr to the class definition and removed it from all of the contructors. > > > Source/WebCore/editing/EditingStyle.cpp:178 > > + , m_tagName(nullptr) > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from all of the constructors. I moved the initialization to nullptr to the class definition and removed it from the contructor. > > > Source/WebCore/editing/EditorCommand.cpp:1680 > > + : m_command(nullptr) > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from all of the constructors. I moved the initialization to nullptr to the class definition and removed it from the contructor. > > > Source/WebCore/editing/RenderedPosition.cpp:68 > > + : m_renderer(nullptr) > > + , m_inlineBox(nullptr) > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from all of the constructors. I moved the initialization to nullptr to the class definition and removed it from all of the contructors. > > > Source/WebCore/editing/RenderedPosition.h:96 > > + : m_renderer(nullptr) > > + , m_inlineBox(nullptr) > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from all of the constructors. I moved the initialization to nullptr to the class definition and removed it from all of the contructors. > > > Source/WebCore/editing/SmartReplaceCF.cpp:40 > > + static CFMutableCharacterSetRef preSmartSet = nullptr; > > + static CFMutableCharacterSetRef postSmartSet = nullptr; > > No need to initialize a global variable (static) to nullptr. That will > happen even if it’s not specified. Better to just remove the initialization > entirely. I see. I removed the initialization entirely. > > > Source/WebCore/editing/SpellChecker.cpp:47 > > + : m_checker(nullptr) > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from the constructor. I moved the initialization to nullptr to the class definition and removed it from the contructor. > > > Source/WebCore/editing/VisibleUnits.cpp:140 > > +CachedLogicallyOrderedLeafBoxes::CachedLogicallyOrderedLeafBoxes() : m_rootInlineBox(nullptr) { }; > > Better in a case like this to move the initialization to nullptr to the > class definition and remove it from the constructor. Also, there’s an extra > semicolon here. And no good reason to format this all on one long line > instead of normal formatting. I moved the initialization to nullptr to the class definition and removed it from the constructor. And changed the format to follow the normal formatting. > > > Source/WebCore/editing/ios/EditorIOS.mm:125 > > + const char *newValue = nil; > > This use of nil is incorrect. You should only use nil for Objective-C object > pointers, not plain C ones. It would be OK to use nullptr here. Changed to nullptr. > > > Source/WebCore/editing/ios/EditorIOS.mm:207 > > + const Font* font = nil; > > This use of nil is incorrect. You should only use nil for Objective-C object > pointers, not C++ object pointers. It would be OK to use nullptr here. Ditto. > > > Source/WebCore/editing/mac/EditorMac.mm:130 > > + const Font* font = nil; > > This use of nil is incorrect. You should only use nil for Objective-C object > pointers, not C++ object pointers. It would be OK to use nullptr here. Ditto. Darin, Thanks for the review. I always learn many things from you. Thanks.
Comment on attachment 264125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264125&action=review > Source/WebCore/editing/mac/AlternativeTextUIController.mm:53 > HashMapType::const_iterator itr = m_alternativesObjectMap.find(context); > if (itr == m_alternativesObjectMap.end()) > - return NULL; > + return nil; > return itr->value.get(); I just realized that all this logic is doing the same thing get would do, since the default value is nil. We can just write: return m_alternativesObjectMap.get(context);
Created attachment 264198 [details] Patch
(In reply to comment #5) > Comment on attachment 264125 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=264125&action=review > > > Source/WebCore/editing/mac/AlternativeTextUIController.mm:53 > > HashMapType::const_iterator itr = m_alternativesObjectMap.find(context); > > if (itr == m_alternativesObjectMap.end()) > > - return NULL; > > + return nil; > > return itr->value.get(); > > I just realized that all this logic is doing the same thing get would do, > since the default value is nil. We can just write: > > return m_alternativesObjectMap.get(context); If we used the 'return m_alternativesObjectMap.get(context)', we got the build break because alternativesObjectMap value is RetainPtr. Is it correct to use the 'm_alternativesObjectMap.get(context).get()'?
Created attachment 264201 [details] Patch
Comment on attachment 264201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264201&action=review > Source/WebCore/editing/mac/AlternativeTextUIController.mm:50 > + return m_alternativesObjectMap.get(context).get(); Yes, this is correct. For RefPtr, we have traits that make hash maps know how to return raw pointers instead of returning RetainPtr, but I guess we didn’t do that for RetainPtr. Someone can come back and fix that at some point.
Comment on attachment 264201 [details] Patch Clearing flags on attachment: 264201 Committed r191671: <http://trac.webkit.org/changeset/191671>
All reviewed patches have been landed. Closing bug.