Refactor a bit the TrackSlimmingTool to separate the logic needing the const_cast
The original documentation of the tool claims that
/**This method 'skims' interesting information from the passed track, and creates
a new one with cloned copies of this information
@param track A reference to the track to be skimmed. It will not be modified in any way.
@return A 'slimmed' version of 'track', where exactly what information is copied depends on
how the tool is configured*/
virtual Trk::Track* slim(const Trk::Track& track) const = 0
In reality the existence of m_setPersistificationHints
means there is another path where the passed track is modified (involving const_cast of the attached TrackStateOnSurfaces
) and a nullptr
is returned.
In this MR:
-
Add
std::unique_ptr<Trk::Track> slimCopy(const Trk::Track& track);
which implements the original intention.m_setPersistificationHints
does not enter when this interface is used. -
The
virtual Trk::Track* slim(const Trk::Track& track);
stays. Ifm_setPersistificationHints
is not set it just calls the new method above. Whenm_setPersistificationHints
is set it falls through to the old behaviour of setting Hints on the TrackStateOnSurfaces of the Track ,still doing the const_cast. -
Update the comments in the headers to reflect the issue.
-
Mark things with the
thread safety
checker. The old interface since it includes const_cast is marked as non-const safe and clients should be careful when setting them_setPersistificationHints
and/or the context it is called. -
Alternatively, for the originally intended case one can just use the new interface directly which is thread_safe
A couple of helpers methods were added to reduce a bit code duplication where it was obvious it can be avoided. As we now have 2 methods rather than 1.