Skip to content

Commit

Permalink
enh: Add MakeUnique
Browse files Browse the repository at this point in the history
  • Loading branch information
@schuhschuh
schuhschuh committed Nov 6, 2019
1 parent e2a2276 commit d693ef9
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions Modules/Common/include/mirtk/Memory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ using SharedPtr = std::shared_ptr<T>;
template <class T>
using WeakPtr = std::weak_ptr<T>;

template <class T>
UniquePtr<T> MakeUnique()
{
return std::make_unique<T>();
}

template <class T, class... Args>
UniquePtr<T> MakeUnique(Args&&... args)
{
return std::make_unique<T>(args...);
}

template <class T>
SharedPtr<T> NewShared()
{
Expand Down

7 comments on commit d693ef9

@jcupitt
Copy link
Contributor

@jcupitt jcupitt commented on d693ef9 Nov 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Andreas, this will break compiles on ubuntu 16.04 with the default compiler. Is MIRTK switching to C++14?

@schuhschuh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I probably should have tested. I only checked which GCC version comes with Ubuntu 16.04 (GCC 5) and whether it would support C++14 standard features. According to https://gcc.gnu.org/projects/cxx-status.html#cxx14, the default GCC compiler on Ubuntu 16.04 should have full support of C++14.

Is that not the case?

@schuhschuh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To answer the question, yes, I wanted to switch to C++14. Is there a good reason to stick to C++11 still?

@schuhschuh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also: #707

@jcupitt
Copy link
Contributor

@jcupitt jcupitt commented on d693ef9 Nov 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my fault, I should have checked more carefully.

MIRTK builds, but SphericalMesh fails when it tries to include MIRTK:

[ 45%] Buklding CXX object ThirdParty/FastMarching/src/CMakeFiles/fastMarchingGeodesic.dir/GW_GeodesicVertex.cpp.o
...
/usr/src/structural-pipeline/build/MIRTK/Modules/Common/include/mirtk/Memory.h: In function 'mirtk::UniquePtr<T> mirtk::MakeUnique()':
/usr/src/structural-pipeline/build/MIRTK/Modules/Common/include/mirtk/Memory.h:47:10: error: 'make_unique' is not a member of 'std'
   return std::make_unique<T>();
          ^

Perhaps SphericalMesh needs a patch to enable c++14, I'll have a look.

@jcupitt
Copy link
Contributor

@jcupitt jcupitt commented on d693ef9 Nov 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding reasons to stay on c++11, FSL 6.0.2 is still built with gcc 4.8 (I know, sad trombone), so if you switch MIRTK to gcc5 minimum, you will no longer be able to build programs which link to both FSL and MIRTK.

Perhaps this is not very common.

@schuhschuh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point. Though I would rather expect folks to use FSL (and MIRTK) to construct executable-based pipelines, so the code wouldn't have to be linked together. As it's only for std::make_unique for now, probably best to just fall back to C++11 compatibility.

Please sign in to comment.