ITK icon indicating copy to clipboard operation
ITK copied to clipboard

COMP: Add type aliases of ptrdiff_t and size_t to the `itk` namespace

Open N-Dekker opened this issue 1 year ago • 3 comments

Officially, we may not assume that ptrdiff_t and size_t are defined in the global namespace.


Motivation: Both ptrdiff_t and size_t are already commonly being used inside the itk namespace, without specifying their std namespace:

https://github.com/InsightSoftwareConsortium/ITK/blob/487da6d086b341808147c71e8dd1eb1f57371ea6/Modules/IO/ImageBase/include/itkConvertPixelBuffer.h#L64

https://github.com/InsightSoftwareConsortium/ITK/blob/487da6d086b341808147c71e8dd1eb1f57371ea6/Modules/Video/IO/src/itkFileListVideoIO.cxx#L64

It just compiles, so far! However, compilers may become more picky, as those types are not guaranteed to remain available in the global namespace.

N-Dekker avatar Apr 29 '24 15:04 N-Dekker

@thewtex FYI, I think this pull request can be merged safely for ITK 5.4. On the other hand, it isn't very urgent to me, personally. It might become urgent, once compilers start to complain about using size_t without specifying the namespace std.

N-Dekker avatar Apr 30 '24 15:04 N-Dekker

This looks like a low-risk addition, and merging it sooner rather than later is better in my opinion.

dzenanz avatar Apr 30 '24 15:04 dzenanz

This does seem to be low risk; however, it is also possible to cause issues since there may be naming conflicts. Since this is not addressing a known issue, it should be merged after v5.4.0.

@thewtex OK, no problem. Marked "draft"to avoid accidentally being merged before the release of v5.4.0. 👍

N-Dekker avatar May 01 '24 17:05 N-Dekker

I think this pull request may also be merged now, as v5.4.0 has been tagged: https://github.com/InsightSoftwareConsortium/ITK/pull/4603#issuecomment-2120498131

N-Dekker avatar May 21 '24 08:05 N-Dekker