vtkMutexLock is useless since C++11 support and std::mutex.
Should we deprecate it for further removal ?
vtkMutexLock is useless since C++11 support and std::mutex.
Should we deprecate it for further removal ?
The same could be said about vtkAtomic and std::atomic. The work to remove it may be bigger though.
If we can do so without much upgrade pain for applications that use VTK then I say heck yeah.
Always remember that standard is one thing, implementation is another.
See https://en.cppreference.com/w/cpp/compiler_support#cpp11
However, regarding mutex and atomic, I think they are now quite well supported.
vtkAtomic has different implementations depending on the selected SMP backend. Could there a benefit to use TBB atomics instead of C++11 ones?
@will.schroeder : do you have an advice here ?
I may give it a go during the Hackaton.
I expect that std::atomic
will be an easy replacement for vtkAtomic
when T
is a primitive type. The open question is how well specified std::atomic<void*>
is, as vtkAtomic
also supports that.
one information : std::mutex does not support forward declaration
It is undefined behavior to forward declare anything from that exists in the std::
namespace.
indeed.
here is a WIP MR : https://gitlab.kitware.com/vtk/vtk/merge_requests/5424
It replace vtkMutexLock by std::mutex. Seems to work fine so far.
The porting is quite simple : #include <mutex>
, replace vtkMutexLock
by std::mutex
, replace Lock()
by lock()
and Unlock()
by unlock()
and you’re done.
However there are other classes that would need to be considered as well :
vtkCriticalSection
vtkSimpleCriticalSection
vtkConditionVariable
What needs to happen for the other classes can they be replaced similar to what you proposed?
will this work?
I found this as well which I think gets pretty close to making it make sense. https://gitlab.kitware.com/vtk/vtk/-/merge_requests/7224
and this: documentation from gitlab
Making more sense now.
Yes, this is the same idea, more work to be done though.
Thank you @mwestphal. I think I was able to resolve my issue following the current updates.
What is missing?
Neverming, looking at @seanm MR, looks like all have been adressed !