Difference between revisions of "Documentation/Nightly/Developers/Tutorials/MigrationGuide/ObsoleteCodeRemoval"

From Slicer Wiki
Jump to: navigation, search
Tag: 2017 source edit
Tag: 2017 source edit
Line 315: Line 315:
 
Next step in this area would be to move these declarations to the public interface (currently they are private).
 
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 ===
 
===C++11: Update source code to use = default ===

Revision as of 21:52, 15 April 2020

Home < Documentation < Nightly < Developers < Tutorials < MigrationGuide < ObsoleteCodeRemoval

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.

  1. Compile Slicer with the CMake option -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON to generate the file compile_commands.json at configuration time.
  2. 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:

  1. The base class implementation function signature changes.
  2. 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'

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