Difference between revisions of "Documentation/Nightly/Developers/Tutorials/MigrationGuide/ObsoleteCodeRemoval"
Tag: 2017 source edit |
|||
(47 intermediate revisions by 2 users not shown) | |||
Line 5: | Line 5: | ||
===Qt>=5.0.0: Remove obsolete code supporting Qt4 plugin infrastructure (C++)=== | ===Qt>=5.0.0: Remove obsolete code supporting Qt4 plugin infrastructure (C++)=== | ||
− | ====qSlicerNAMEModuleWidgetsAbstractPlugin.h | + | These changes apply to Qt loadable modules and designer plugins |
+ | |||
+ | ====qSlicerNAMEModuleWidgetsPlugin.h==== | ||
+ | |||
+ | '''Before:''' | ||
+ | |||
+ | <pre> | ||
+ | #include "vtkSlicerConfigure.h" // For Slicer_HAVE_QT5 | ||
+ | |||
+ | // Qt includes | ||
+ | #ifdef Slicer_HAVE_QT5 | ||
+ | #include <QtUiPlugin/QDesignerCustomWidgetCollectionInterface> | ||
+ | #else | ||
+ | #include <QDesignerCustomWidgetCollectionInterface> | ||
+ | #endif | ||
+ | |||
+ | [...] | ||
+ | |||
+ | class Q_SLICER_MODULE_SUBJECTHIERARCHY_WIDGETS_PLUGINS_EXPORT qSlicerSubjectHierarchyModuleWidgetsPlugin | ||
+ | : public QObject | ||
+ | , public QDesignerCustomWidgetCollectionInterface | ||
+ | { | ||
+ | Q_OBJECT | ||
+ | #ifdef Slicer_HAVE_QT5 | ||
+ | Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QDesignerCustomWidgetCollectionInterface") | ||
+ | #endif | ||
+ | Q_INTERFACES(QDesignerCustomWidgetCollectionInterface); | ||
+ | </pre> | ||
+ | |||
+ | '''After:''' | ||
+ | |||
+ | <pre> | ||
+ | // Qt includes | ||
+ | #include <QtUiPlugin/QDesignerCustomWidgetCollectionInterface> | ||
+ | |||
+ | [...] | ||
+ | |||
+ | class Q_SLICER_MODULE_SUBJECTHIERARCHY_WIDGETS_PLUGINS_EXPORT qSlicerSubjectHierarchyModuleWidgetsPlugin | ||
+ | : public QObject | ||
+ | , public QDesignerCustomWidgetCollectionInterface | ||
+ | { | ||
+ | Q_OBJECT | ||
+ | Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QDesignerCustomWidgetCollectionInterface") | ||
+ | Q_INTERFACES(QDesignerCustomWidgetCollectionInterface); | ||
+ | </pre> | ||
+ | |||
+ | ====qSlicerNAMEModule.h==== | ||
+ | |||
+ | '''Before:''' | ||
+ | |||
+ | <pre> | ||
+ | class Q_SLICER_QTMODULES_SUBJECTHIERARCHY_EXPORT qSlicerSubjectHierarchyModule : | ||
+ | public qSlicerLoadableModule | ||
+ | { | ||
+ | Q_OBJECT | ||
+ | #ifdef Slicer_HAVE_QT5 | ||
+ | Q_PLUGIN_METADATA(IID "org.slicer.modules.loadable.qSlicerLoadableModule/1.0"); | ||
+ | #endif | ||
+ | </pre> | ||
+ | |||
+ | '''After:''' | ||
+ | |||
+ | <pre> | ||
+ | class Q_SLICER_QTMODULES_SUBJECTHIERARCHY_EXPORT qSlicerSubjectHierarchyModule : | ||
+ | public qSlicerLoadableModule | ||
+ | { | ||
+ | Q_OBJECT | ||
+ | Q_PLUGIN_METADATA(IID "org.slicer.modules.loadable.qSlicerLoadableModule/1.0"); | ||
+ | </pre> | ||
+ | |||
+ | ====qSlicerNAMEModuleWidgetsAbstractPlugin.h==== | ||
'''Before:''' | '''Before:''' | ||
Line 45: | Line 115: | ||
− | ====qSlicerNAMEModule.cxx and qSlicerNAMEModuleWidgetsAbstractPlugin.cxx | + | ====qSlicerNAMEModule.cxx and qSlicerNAMEModuleWidgetsAbstractPlugin.cxx==== |
'''Remove:''' | '''Remove:''' | ||
Line 54: | Line 124: | ||
#endif | #endif | ||
+ | ===Qt>=5.0.0: Simpler use of QHeaderView::setSectionResizeMode=== | ||
− | + | This migration guide entry is obsolete: [[Documentation/Nightly/Developers/Tutorials/MigrationGuide#Qt5:_Fix_error:_.E2.80.98class_QHeaderView.E2.80.99_has_no_member_named_.E2.80.98setResizeMode.E2.80.99]] | |
− | + | '''Solution for Cpp''' | |
− | |||
− | ''' | + | Before: |
+ | |||
+ | #if (QT_VERSION < QT_VERSION_CHECK(5, 0, 0)) | ||
+ | this->SceneViewTableWidget->horizontalHeader()->setResizeMode(QHeaderView::Stretch); | ||
+ | this->SceneViewTableWidget->horizontalHeader()->setResizeMode(QHeaderView::ResizeToContents); | ||
+ | this->SceneViewTableWidget->horizontalHeader()->setResizeMode(SCENE_VIEW_DESCRIPTION_COLUMN, QHeaderView::Stretch); | ||
+ | #else | ||
+ | this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch); | ||
+ | this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); | ||
+ | this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(SCENE_VIEW_DESCRIPTION_COLUMN, QHeaderView::Stretch); | ||
+ | #endif | ||
+ | |||
+ | '''After:''' | ||
+ | |||
+ | this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch); | ||
+ | this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); | ||
+ | this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(SCENE_VIEW_DESCRIPTION_COLUMN, QHeaderView::Stretch); | ||
+ | |||
+ | |||
+ | '''Solution for Python''' | ||
+ | |||
+ | Before: | ||
− | + | <pre> | |
def _setSectionResizeMode(header, *args, **kwargs): | def _setSectionResizeMode(header, *args, **kwargs): | ||
if version.parse(qt.Qt.qVersion()) < version.parse("5.0.0"): | if version.parse(qt.Qt.qVersion()) < version.parse("5.0.0"): | ||
Line 69: | Line 160: | ||
header.setSectionResizeMode(*args, **kwargs) | header.setSectionResizeMode(*args, **kwargs) | ||
− | '''After:''' | + | [...] |
+ | |||
+ | _setSectionResizeMode(self.horizontalHeader(), 0, qt.QHeaderView.Stretch) | ||
+ | </pre> | ||
+ | |||
+ | After: | ||
+ | |||
+ | self.horizontalHeader().setSectionResizeMode(0, qt.QHeaderView.Stretch) | ||
+ | |||
+ | ===C++11: Update source code to use nullptr=== | ||
+ | |||
+ | C++11 introduced the keyword <tt>nullptr</tt> allowing to explicitly identify a pointer of type <tt>std::nullptr_t</tt>. It avoids ambiguous function overloads. See also https://stackoverflow.com/questions/20509734/null-vs-nullptr-why-was-it-replaced | ||
+ | |||
+ | Prior to supporting only C++11, <tt>0</tt> and [http://www.cplusplus.com/reference/cstring/NULL/ NULL] integral type were used to check for pointer value, and macro like <tt>Q_NULLPTR</tt> and <tt>ITK_NULLPTR</tt> macros were used to ensure usage of <tt>nullptr</tt> when C++11 support was enabled. | ||
+ | |||
+ | Updating the code base to use <tt>nullptr</tt> is done applying these methods: | ||
+ | * clang-tidy for updating source code being explicitly compiled in the project. | ||
+ | * regular expression for replacing occurrences of <tt>NULL</tt> in source code not being compiled (e.g conditionally compiled code, templated headers, comments) | ||
+ | * replacement of occurences of <tt>VTK_NULLPTR</tt>, <tt>ITK_NULLPTR</tt>, <tt>Q_NULLPTR</tt> with <tt>nullptr</tt> | ||
+ | |||
+ | '''Using clang-tidy''' | ||
+ | |||
+ | This will iteratively check files for updates. It takes a while. The sources are only modified at the end of the process, so you could cancel anytime before with no consequences. | ||
+ | # Compile Slicer with the CMake option -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON to generate the file compile_commands.json at configuration time. | ||
+ | # Run clang-tidy on the generated compile_commands.json (in the build folder: Slicer-build) | ||
+ | run-clang-tidy.py -header-filter='.*' -checks='-*,modernize-use-nullptr' -fix | ||
+ | [http://clang.llvm.org/extra/clang-tidy/ clang-tidy] is a tool from clang. In Archlinux: sudo pacman -S clang, in other distros might be clang-extra-tools or similar. | ||
+ | |||
+ | clang-tidy will substitute NULL and 0 for nullptr when appropriate. It is an iterative process, so it may take time. Also, it replaces all the compiled code, including external dependencies. | ||
+ | It is recommended to do a fresh build with the modified code, for testing it without modifications in any external repo. | ||
+ | |||
+ | '''Using regex for replacing NULL in comments and not-compiled code''' | ||
+ | |||
+ | After clang-tidy has modified the compiled code with the help of compile_commands.json, there will still be ignored instances of NULL and 0 in some headers, and comments. | ||
+ | |||
+ | Manually replace with the following regex to change those. | ||
+ | |||
+ | 1) Change NULL to nullptr except when there is a quote or an underscore in | ||
+ | front of it. And there is not a quote after. | ||
+ | |||
+ | ack --type-add=cpp:ext:hxx,hpp,txx,tpp,cxx,cpp,c,cc --cpp "NULL" -l --print0 | xargs -0 -n 1 sed -i 's/\([^\"\_]\)NULL\([^\"\_]\)/\1nullptr\2/g' | ||
+ | |||
+ | 2) The same than before but match end of line NULL | ||
+ | ack --type-add=cpp:ext:hxx,hpp,txx,tpp,cxx,cpp,c,cc --cpp "NULL" -l --print0 | xargs -0 -n 1 sed -i 's/\([^\"\_]\)NULL$/\1nullptr/g' | ||
+ | |||
+ | |||
+ | |||
+ | '''Replace ITK_NULLPTR, VTK_NULLPTR, Q_NULLPTR''' | ||
+ | |||
+ | <!-- For an example of commit message, see [http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=28022 r28022] --> | ||
+ | |||
+ | It was adapted from the ITK script in [https://github.com/InsightSoftwareConsortium/ITK/blob/v5.0rc01/Utilities/ITKv5Preparation/ReplaceITK_NULLPTRMacroNames.sh ReplaceITK_NULLPTRMacroNames.sh] | ||
+ | |||
+ | * <tt>ITK_NULLPTR</tt>: | ||
+ | |||
+ | <pre>git grep -l "ITK_NULLPTR" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/ITK_NULLPTR/nullptr/g"</pre> | ||
+ | |||
+ | * <tt>VTK_NULLPTR</tt>: | ||
+ | |||
+ | <pre>git grep -l "VTK_NULLPTR" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/VTK_NULLPTR/nullptr/g"</pre> | ||
+ | |||
+ | * <tt>Q_NULLPTR</tt>: | ||
+ | |||
+ | <pre>git grep -l "Q_NULLPTR" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/Q_NULLPTR/nullptr/g"</pre> | ||
+ | |||
+ | ===C++11: Update source code to use override=== | ||
+ | |||
+ | From: https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-override.html | ||
+ | |||
+ | Adds override (introduced in C++11) to overridden virtual functions and removes virtual from those functions as it is not required. | ||
+ | |||
+ | virtual on non base class implementations was used to help indicate to the user that a function was virtual. C++ compilers did not use the presence of this to signify an overriden function. | ||
+ | |||
+ | In C++ 11 override and final keywords were introduced to allow overridden functions to be marked appropriately. Their presence allows compilers to verify that an overridden function correctly overrides a base class implementation. | ||
+ | |||
+ | This can be useful as compilers can generate a compile time error when: | ||
+ | |||
+ | # The base class implementation function signature changes. | ||
+ | # The user has not created the override with the correct signature. | ||
+ | |||
+ | First we replace ITK_OVERRIDE and VTK_OVERRIDE for override: | ||
+ | |||
+ | '''Replace ITK_OVERRIDE and VTK_OVERRIDE''' | ||
+ | |||
+ | For an example of commit message, see [http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=28022 r28022] | ||
+ | |||
+ | The regexp involving ITK was extracted from the [https://github.com/InsightSoftwareConsortium/ITK/blob/master/Utilities/ITKv5Preparation/ReplaceITK_OVERRIDEMacroNames.sh ReplaceITK_OVERRIDEMacroNames.sh] script in ITK. | ||
+ | |||
+ | * <tt>ITK_OVERRIDE</tt> | ||
+ | |||
+ | <pre>git grep -l "ITK_OVERRIDE" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/ITK_OVERRIDE/override/g"</pre> | ||
+ | |||
+ | * <tt>VTK_OVERRIDE</tt> | ||
+ | |||
+ | <pre>git grep -l "VTK_OVERRIDE" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/VTK_OVERRIDE/override/g"</pre> | ||
+ | |||
+ | '''Automatic update: Using clang-tidy''' | ||
+ | |||
+ | '''Note''': Running ''clang-tidy -fix'' in parallel might cause problems, like appending multiple keywords to same function (override). It can be fixed with a grep to remove duplicates. | ||
+ | |||
+ | This will iteratively check files for updates. It takes a while. The sources are only modified at the end of the process, so you could cancel anytime before with no consequences. | ||
+ | |||
+ | Compile Slicer with the CMake option -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON to generate the file compile_commands.json at configuration time. | ||
+ | Run clang-tidy on the generated compile_commands.json (in the build folder: Slicer-build) | ||
+ | |||
+ | run-clang-tidy.py -checks=-*,modernize-use-override -header-filter=.* -fix | ||
+ | |||
+ | '''Manual fix''' | ||
+ | |||
+ | Multiple override are added to the same method. For example: | ||
+ | |||
+ | void PrintSelf(ostream& os, vtkIndent indent) override override override; | ||
+ | |||
+ | We clean them with (15 here is just a high enough number): | ||
+ | |||
+ | for n in `seq 1 15`; | ||
+ | do | ||
+ | ack "override override" -l --print0 | xargs -0 -n 1 sed -i \ | ||
+ | 's/override override/override/g' | ||
+ | done | ||
+ | |||
+ | When all of them are fixed, you should see in the terminal: | ||
+ | |||
+ | sed: no input files | ||
+ | |||
+ | ===C++11: Update source code to use = delete === | ||
+ | |||
+ | Instead of: | ||
+ | vtkImageRectangularSource(const vtkImageRectangularSource&); /// Not implemented. | ||
+ | void operator=(const vtkImageRectangularSource&); /// Not implemented. | ||
+ | |||
+ | Replace for: | ||
+ | vtkImageRectangularSource(const vtkImageRectangularSource&) = delete; | ||
+ | void operator=(const vtkImageRectangularSource&) = delete; | ||
+ | |||
+ | Same approach than before, use the compile_commands.json and clang-tidy in Slicer-build. | ||
+ | |||
+ | run-clang-tidy.py -checks=-*,modernize-use-equals-delete -header-filter=.* -fix | ||
+ | |||
+ | Because we run it in parallel, multiple occurrences of = delete might have been added. Fix them with: | ||
+ | |||
+ | for n in `seq 1 15`; | ||
+ | do | ||
+ | ack "= delete = delete" -l --print0 | xargs -0 -n 1 sed -i \ | ||
+ | 's/= delete = delete/= delete/g' | ||
+ | done | ||
+ | |||
+ | And also delete comment /// Not implemented, as now it is clear with = delete. | ||
+ | |||
+ | ack "delete" -l --print0 | xargs -0 -n 1 sed -E -i 's@delete;\s*\/*\s*[nN]ot [iI]mplemented\.?@delete;@g' | ||
+ | ack "delete" -l --print0 | xargs -0 -n 1 sed -E -i 's@delete;\s*\/*\s*purposely [nN]ot [iI]mplemented\.?@delete;@g' | ||
+ | ack "delete" -l --print0 | xargs -0 -n 1 sed -E -i 's@delete ITK\_DELETED\_FUNCTION;@delete;@g' | ||
+ | |||
+ | Next step in this area would be to move these declarations to the public interface (currently they are private). | ||
+ | |||
+ | <b>References:</b> | ||
+ | |||
+ | https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-equals-delete.html | ||
+ | |||
+ | ===C++11: Update source code to use = default === | ||
+ | |||
+ | Instead of: | ||
+ | virtual ~DataRequest(){} | ||
+ | |||
+ | Replace for: | ||
+ | virtual ~DataRequest() = default; | ||
+ | |||
+ | Same approach than before, use the compile_commands.json and clang-tidy in Slicer-build. | ||
+ | |||
+ | run-clang-tidy.py -checks=-*,modernize-use-equals-default -header-filter=.* -fix | ||
+ | |||
+ | Because we run it in parallel, multiple colons ;; might have been added. Fix them with: | ||
+ | |||
+ | for n in `seq 1 15`; | ||
+ | do | ||
+ | ack "default" -l --print0 | xargs -0 -n 1 sed -i 's/= default;;/= default;/g' | ||
+ | done | ||
+ | |||
+ | Add an empty space before = default if there is none: | ||
+ | ack "default" -l --print0 | xargs -0 -n 1 sed -E -i 's/([^\s])= default;/\1 = default;/g' | ||
+ | |||
+ | <b>References:</b> | ||
+ | |||
+ | https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-equals-default.html | ||
+ | |||
+ | ===C++11: Use default member initialization === | ||
+ | |||
+ | Instead of: | ||
+ | |||
+ | <pre> | ||
+ | // vtkMRMLFooNode.h | ||
+ | class VTK_MRML_EXPORT vtkMRMLFooNode : public vtkMRMLAbstractBarNode | ||
+ | { | ||
+ | [...] | ||
+ | |||
+ | bool DoSomething; | ||
+ | int InteractionMode; | ||
+ | } | ||
+ | |||
+ | // vtkMRMLFooNode.cxx | ||
+ | vtkMRMLFooNode::vtkMRMLFooNode() | ||
+ | : DoSomething(true) | ||
+ | , FooMode(FooModeAwesome) | ||
+ | { | ||
+ | [...] | ||
+ | } | ||
+ | </pre> | ||
+ | |||
+ | Replace with: | ||
+ | |||
+ | <pre> | ||
+ | // vtkMRMLFooNode.h | ||
+ | class VTK_MRML_EXPORT vtkMRMLFooNode : public vtkMRMLAbstractBarNode | ||
+ | { | ||
+ | [...] | ||
+ | |||
+ | bool DoSomething{true}; | ||
+ | int FooMode{FooModeAwesome}; | ||
+ | }; | ||
+ | |||
+ | // vtkMRMLFooNode.cxx | ||
+ | vtkMRMLFooNode::vtkMRMLFooNode() | ||
+ | { | ||
+ | [...] | ||
+ | } | ||
+ | |||
+ | // or this if constructor is empty after removing old-style default initialization | ||
+ | |||
+ | // vtkMRMLFooNode.cxx | ||
+ | vtkMRMLFooNode::vtkMRMLFooNode() = default; | ||
+ | </pre> | ||
+ | |||
+ | |||
+ | Same approach than before, use the compile_commands.json and clang-tidy in Slicer-build. | ||
+ | |||
+ | run-clang-tidy.py -checks=-*,modernize-use-default-member-init -header-filter=.* -fix | ||
+ | |||
+ | |||
+ | <b>References:</b> | ||
− | + | https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-default-member-init.html |
Latest revision as of 21:52, 15 April 2020
Home < Documentation < Nightly < Developers < Tutorials < MigrationGuide < ObsoleteCodeRemovalContents
- 1 Obsolete Code Removal
- 1.1 Qt>=5.0.0: Remove obsolete code supporting Qt4 plugin infrastructure (C++)
- 1.2 Qt>=5.0.0: Simpler use of QHeaderView::setSectionResizeMode
- 1.3 C++11: Update source code to use nullptr
- 1.4 C++11: Update source code to use override
- 1.5 C++11: Update source code to use = delete
- 1.6 C++11: Update source code to use = default
- 1.7 C++11: Use default member initialization
Obsolete Code Removal
This section documents suggested code changes after removing support for a particular features. Each category has a short description, code snippets, a suggested upgrade path, and references to relevant commits.
Qt>=5.0.0: Remove obsolete code supporting Qt4 plugin infrastructure (C++)
These changes apply to Qt loadable modules and designer plugins
qSlicerNAMEModuleWidgetsPlugin.h
Before:
#include "vtkSlicerConfigure.h" // For Slicer_HAVE_QT5 // Qt includes #ifdef Slicer_HAVE_QT5 #include <QtUiPlugin/QDesignerCustomWidgetCollectionInterface> #else #include <QDesignerCustomWidgetCollectionInterface> #endif [...] class Q_SLICER_MODULE_SUBJECTHIERARCHY_WIDGETS_PLUGINS_EXPORT qSlicerSubjectHierarchyModuleWidgetsPlugin : public QObject , public QDesignerCustomWidgetCollectionInterface { Q_OBJECT #ifdef Slicer_HAVE_QT5 Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QDesignerCustomWidgetCollectionInterface") #endif Q_INTERFACES(QDesignerCustomWidgetCollectionInterface);
After:
// Qt includes #include <QtUiPlugin/QDesignerCustomWidgetCollectionInterface> [...] class Q_SLICER_MODULE_SUBJECTHIERARCHY_WIDGETS_PLUGINS_EXPORT qSlicerSubjectHierarchyModuleWidgetsPlugin : public QObject , public QDesignerCustomWidgetCollectionInterface { Q_OBJECT Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QDesignerCustomWidgetCollectionInterface") Q_INTERFACES(QDesignerCustomWidgetCollectionInterface);
qSlicerNAMEModule.h
Before:
class Q_SLICER_QTMODULES_SUBJECTHIERARCHY_EXPORT qSlicerSubjectHierarchyModule : public qSlicerLoadableModule { Q_OBJECT #ifdef Slicer_HAVE_QT5 Q_PLUGIN_METADATA(IID "org.slicer.modules.loadable.qSlicerLoadableModule/1.0"); #endif
After:
class Q_SLICER_QTMODULES_SUBJECTHIERARCHY_EXPORT qSlicerSubjectHierarchyModule : public qSlicerLoadableModule { Q_OBJECT Q_PLUGIN_METADATA(IID "org.slicer.modules.loadable.qSlicerLoadableModule/1.0");
qSlicerNAMEModuleWidgetsAbstractPlugin.h
Before:
#include <QtGlobal> #if(QT_VERSION < QT_VERSION_CHECKS(5, 0, 0)) #include <QDesignerCustomWidgetInterface> #else #include <QtUiPlugin/QDesignerCustomWidgetInterface> #endif [...] class Q_SLICER_MODULE_SEGMENTATIONS_WIDGETS_PLUGINS_EXPORT qSlicerSegmentationsModuleWidgetsAbstractPlugin : public QDesignerCustomWidgetInterface { #if (QT_VERSION >= QT_VERSION_CHECK(5, 0, 0)) Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QDesignerCustomWidgetInterface") #endif Q_INTERFACES(QDesignerCustomWidgetInterface); }
After:
#include <QtUiPlugin/QDesignerCustomWidgetInterface> [...] class Q_SLICER_MODULE_SEGMENTATIONS_WIDGETS_PLUGINS_EXPORT qSlicerSegmentationsModuleWidgetsAbstractPlugin : public QDesignerCustomWidgetInterface { Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QDesignerCustomWidgetInterface") Q_INTERFACES(QDesignerCustomWidgetInterface); }
qSlicerNAMEModule.cxx and qSlicerNAMEModuleWidgetsAbstractPlugin.cxx
Remove:
#if (QT_VERSION < QT_VERSION_CHECK(5, 0, 0)) #include <QtPlugin> Q_EXPORT_PLUGIN2(customwidgetplugin, qSlicerSegmentationsModuleWidgetsPlugin); #endif
Qt>=5.0.0: Simpler use of QHeaderView::setSectionResizeMode
This migration guide entry is obsolete: Documentation/Nightly/Developers/Tutorials/MigrationGuide#Qt5:_Fix_error:_.E2.80.98class_QHeaderView.E2.80.99_has_no_member_named_.E2.80.98setResizeMode.E2.80.99
Solution for Cpp
Before:
#if (QT_VERSION < QT_VERSION_CHECK(5, 0, 0)) this->SceneViewTableWidget->horizontalHeader()->setResizeMode(QHeaderView::Stretch); this->SceneViewTableWidget->horizontalHeader()->setResizeMode(QHeaderView::ResizeToContents); this->SceneViewTableWidget->horizontalHeader()->setResizeMode(SCENE_VIEW_DESCRIPTION_COLUMN, QHeaderView::Stretch); #else this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch); this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(SCENE_VIEW_DESCRIPTION_COLUMN, QHeaderView::Stretch); #endif
After:
this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch); this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); this->SceneViewTableWidget->horizontalHeader()->setSectionResizeMode(SCENE_VIEW_DESCRIPTION_COLUMN, QHeaderView::Stretch);
Solution for Python
Before:
def _setSectionResizeMode(header, *args, **kwargs): if version.parse(qt.Qt.qVersion()) < version.parse("5.0.0"): header.setResizeMode(*args, **kwargs) else: header.setSectionResizeMode(*args, **kwargs) [...] _setSectionResizeMode(self.horizontalHeader(), 0, qt.QHeaderView.Stretch)
After:
self.horizontalHeader().setSectionResizeMode(0, qt.QHeaderView.Stretch)
C++11: Update source code to use nullptr
C++11 introduced the keyword nullptr allowing to explicitly identify a pointer of type std::nullptr_t. It avoids ambiguous function overloads. See also https://stackoverflow.com/questions/20509734/null-vs-nullptr-why-was-it-replaced
Prior to supporting only C++11, 0 and NULL integral type were used to check for pointer value, and macro like Q_NULLPTR and ITK_NULLPTR macros were used to ensure usage of nullptr when C++11 support was enabled.
Updating the code base to use nullptr is done applying these methods:
- clang-tidy for updating source code being explicitly compiled in the project.
- regular expression for replacing occurrences of NULL in source code not being compiled (e.g conditionally compiled code, templated headers, comments)
- replacement of occurences of VTK_NULLPTR, ITK_NULLPTR, Q_NULLPTR with nullptr
Using clang-tidy
This will iteratively check files for updates. It takes a while. The sources are only modified at the end of the process, so you could cancel anytime before with no consequences.
- Compile Slicer with the CMake option -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON to generate the file compile_commands.json at configuration time.
- Run clang-tidy on the generated compile_commands.json (in the build folder: Slicer-build)
run-clang-tidy.py -header-filter='.*' -checks='-*,modernize-use-nullptr' -fix
clang-tidy is a tool from clang. In Archlinux: sudo pacman -S clang, in other distros might be clang-extra-tools or similar.
clang-tidy will substitute NULL and 0 for nullptr when appropriate. It is an iterative process, so it may take time. Also, it replaces all the compiled code, including external dependencies. It is recommended to do a fresh build with the modified code, for testing it without modifications in any external repo.
Using regex for replacing NULL in comments and not-compiled code
After clang-tidy has modified the compiled code with the help of compile_commands.json, there will still be ignored instances of NULL and 0 in some headers, and comments.
Manually replace with the following regex to change those.
1) Change NULL to nullptr except when there is a quote or an underscore in front of it. And there is not a quote after.
ack --type-add=cpp:ext:hxx,hpp,txx,tpp,cxx,cpp,c,cc --cpp "NULL" -l --print0 | xargs -0 -n 1 sed -i 's/\([^\"\_]\)NULL\([^\"\_]\)/\1nullptr\2/g'
2) The same than before but match end of line NULL
ack --type-add=cpp:ext:hxx,hpp,txx,tpp,cxx,cpp,c,cc --cpp "NULL" -l --print0 | xargs -0 -n 1 sed -i 's/\([^\"\_]\)NULL$/\1nullptr/g'
Replace ITK_NULLPTR, VTK_NULLPTR, Q_NULLPTR
It was adapted from the ITK script in ReplaceITK_NULLPTRMacroNames.sh
- ITK_NULLPTR:
git grep -l "ITK_NULLPTR" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/ITK_NULLPTR/nullptr/g"
- VTK_NULLPTR:
git grep -l "VTK_NULLPTR" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/VTK_NULLPTR/nullptr/g"
- Q_NULLPTR:
git grep -l "Q_NULLPTR" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/Q_NULLPTR/nullptr/g"
C++11: Update source code to use override
From: https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-override.html
Adds override (introduced in C++11) to overridden virtual functions and removes virtual from those functions as it is not required.
virtual on non base class implementations was used to help indicate to the user that a function was virtual. C++ compilers did not use the presence of this to signify an overriden function.
In C++ 11 override and final keywords were introduced to allow overridden functions to be marked appropriately. Their presence allows compilers to verify that an overridden function correctly overrides a base class implementation.
This can be useful as compilers can generate a compile time error when:
- The base class implementation function signature changes.
- The user has not created the override with the correct signature.
First we replace ITK_OVERRIDE and VTK_OVERRIDE for override:
Replace ITK_OVERRIDE and VTK_OVERRIDE
For an example of commit message, see r28022
The regexp involving ITK was extracted from the ReplaceITK_OVERRIDEMacroNames.sh script in ITK.
- ITK_OVERRIDE
git grep -l "ITK_OVERRIDE" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/ITK_OVERRIDE/override/g"
- VTK_OVERRIDE
git grep -l "VTK_OVERRIDE" | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/VTK_OVERRIDE/override/g"
Automatic update: Using clang-tidy
Note: Running clang-tidy -fix in parallel might cause problems, like appending multiple keywords to same function (override). It can be fixed with a grep to remove duplicates.
This will iteratively check files for updates. It takes a while. The sources are only modified at the end of the process, so you could cancel anytime before with no consequences.
Compile Slicer with the CMake option -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON to generate the file compile_commands.json at configuration time. Run clang-tidy on the generated compile_commands.json (in the build folder: Slicer-build)
run-clang-tidy.py -checks=-*,modernize-use-override -header-filter=.* -fix
Manual fix
Multiple override are added to the same method. For example:
void PrintSelf(ostream& os, vtkIndent indent) override override override;
We clean them with (15 here is just a high enough number):
for n in `seq 1 15`; do ack "override override" -l --print0 | xargs -0 -n 1 sed -i \ 's/override override/override/g' done
When all of them are fixed, you should see in the terminal:
sed: no input files
C++11: Update source code to use = delete
Instead of:
vtkImageRectangularSource(const vtkImageRectangularSource&); /// Not implemented. void operator=(const vtkImageRectangularSource&); /// Not implemented.
Replace for:
vtkImageRectangularSource(const vtkImageRectangularSource&) = delete; void operator=(const vtkImageRectangularSource&) = delete;
Same approach than before, use the compile_commands.json and clang-tidy in Slicer-build.
run-clang-tidy.py -checks=-*,modernize-use-equals-delete -header-filter=.* -fix
Because we run it in parallel, multiple occurrences of = delete might have been added. Fix them with:
for n in `seq 1 15`; do ack "= delete = delete" -l --print0 | xargs -0 -n 1 sed -i \ 's/= delete = delete/= delete/g' done
And also delete comment /// Not implemented, as now it is clear with = delete.
ack "delete" -l --print0 | xargs -0 -n 1 sed -E -i 's@delete;\s*\/*\s*[nN]ot [iI]mplemented\.?@delete;@g' ack "delete" -l --print0 | xargs -0 -n 1 sed -E -i 's@delete;\s*\/*\s*purposely [nN]ot [iI]mplemented\.?@delete;@g' ack "delete" -l --print0 | xargs -0 -n 1 sed -E -i 's@delete ITK\_DELETED\_FUNCTION;@delete;@g'
Next step in this area would be to move these declarations to the public interface (currently they are private).
References:
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-equals-delete.html
C++11: Update source code to use = default
Instead of:
virtual ~DataRequest(){}
Replace for:
virtual ~DataRequest() = default;
Same approach than before, use the compile_commands.json and clang-tidy in Slicer-build.
run-clang-tidy.py -checks=-*,modernize-use-equals-default -header-filter=.* -fix
Because we run it in parallel, multiple colons ;; might have been added. Fix them with:
for n in `seq 1 15`; do ack "default" -l --print0 | xargs -0 -n 1 sed -i 's/= default;;/= default;/g' done
Add an empty space before = default if there is none:
ack "default" -l --print0 | xargs -0 -n 1 sed -E -i 's/([^\s])= default;/\1 = default;/g'
References:
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-equals-default.html
C++11: Use default member initialization
Instead of:
// vtkMRMLFooNode.h class VTK_MRML_EXPORT vtkMRMLFooNode : public vtkMRMLAbstractBarNode { [...] bool DoSomething; int InteractionMode; } // vtkMRMLFooNode.cxx vtkMRMLFooNode::vtkMRMLFooNode() : DoSomething(true) , FooMode(FooModeAwesome) { [...] }
Replace with:
// vtkMRMLFooNode.h class VTK_MRML_EXPORT vtkMRMLFooNode : public vtkMRMLAbstractBarNode { [...] bool DoSomething{true}; int FooMode{FooModeAwesome}; }; // vtkMRMLFooNode.cxx vtkMRMLFooNode::vtkMRMLFooNode() { [...] } // or this if constructor is empty after removing old-style default initialization // vtkMRMLFooNode.cxx vtkMRMLFooNode::vtkMRMLFooNode() = default;
Same approach than before, use the compile_commands.json and clang-tidy in Slicer-build.
run-clang-tidy.py -checks=-*,modernize-use-default-member-init -header-filter=.* -fix
References:
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-default-member-init.html