ENH: Add ITK_NO_DISCARD(message) macro to Image "Transform" members#4007
Conversation
github-actions
Bot
added
type:Enhancement
| ITK_NO_DISCARD(Call the overload which has the point as the only parameter and returns the index) | ||
| bool TransformPhysicalPointToIndex(const Point<TCoordRep, VImageDimension> & point, IndexType & index) const |
There was a problem hiding this comment.
Please consider to leave the unconditional [[nodiscard]] (even for legacy-only) here. For TransformPhysicalPointToIndex, it's really a no-brainer to replace it with the other overload. (It does not have the template difficulty that TransformPhysicalPointToContinuousIndex has.)
Look at commit 49ece7f it is really straightforward. No extra template argument needed. The faster TransformPhysicalPointToIndex(point) overload has been there already with ITK 5.0.0
There was a problem hiding this comment.
Even if replacement is easy, it requires code changes. The legacy flag's purpose is to allow this to be made gradually.
There was a problem hiding this comment.
I agree with @dzenanz Even these easy changes can be quite disruptive for end-users.
The new macro is applied only to the overloads which take two arguments and return the bool. This is the only scenario where it made sense to ignore the return value.
N-Dekker
left a comment
N-Dekker
left a comment
There was a problem hiding this comment.
Thanks for including most of my suggestions 😃
5 participants
The new macro is applied only to the overloads which take two arguments and return the bool. This is the only scenario where it made sense to ignore the return value.
PR Checklist