COMP: Suppress deprecated VariableLengthVector::AllocateElements warnings in MSVC wrapping builds

[thewtex]

Summary

• Add /wd4996 to MSVC compile flags for SWIG-generated Python wrapper targets to suppress C4996 (deprecated declaration) warnings

Background

After VariableLengthVector::AllocateElements was deprecated with [[deprecated("Please consider calling std::make_unique<TValue[]>(size) instead.")]], the SWIG-generated Python wrapper code (itkVariableLengthVectorPython.cpp) triggers MSVC warning C4996 for each wrapped template instantiation:

itkVariableLengthVectorPython.cpp(5150): warning C4996: 'itk::VariableLengthVector<std::complex<float>>::AllocateElements'
itkVariableLengthVectorPython.cpp(7765): warning C4996: 'itk::VariableLengthVector<double>::AllocateElements'
itkVariableLengthVectorPython.cpp(10633): warning C4996: 'itk::VariableLengthVector<float>::AllocateElements'
itkVariableLengthVectorPython.cpp(13501): warning C4996: 'itk::VariableLengthVector<short>::AllocateElements'
itkVariableLengthVectorPython.cpp(16369): warning C4996: 'itk::VariableLengthVector<unsigned char>::AllocateElements'

This happens because SWIG auto-generates wrapper functions that directly call the now-deprecated AllocateElements method. The generated code cannot be modified to avoid these calls.

Fix

Add /wd4996 alongside the existing /wd4244 in the MSVC compile flags for wrapping targets in Wrapping/macro_files/itk_end_wrap_module.cmake. This is consistent with GCC/Clang wrapping builds, which already suppress all warnings via the -w flag on the same targets. Deprecation warnings remain active for hand-written ITK C++ code; this only affects auto-generated SWIG wrapper compilation.

[blowekamp]

LGTM

[N-Dekker]

Thanks, but why is AllocateElements wrapped? I don't think it should be part of the Python interface. Here it is:

ITK/Modules/Core/Common/include/itkVariableLengthVector.hxx

Lines 198 to 210 in 093598d

	VariableLengthVector<TValue>::AllocateElements(ElementIdentifier size) const
	{
	try
	{
	return new TValue[size];
	}
	catch (...)
	{
	// Intercept std::bad_alloc and any exception thrown from TValue
	// default constructor.
	itkGenericExceptionMacro("Failed to allocate memory of length " << size << " for VariableLengthVector.");
	}
	}

So it basically just returns a raw pointer created by new TValue[size].

[N-Dekker]

If it's only AllocateElements that is causing the problem, I think it's preferable to just fix it locally for that specific function, instead of globally disabling the warning. The warning was introduced by the [[deprecated]] attribute here:

ITK/Modules/Core/Common/include/itkVariableLengthVector.h

Lines 767 to 774 in 093598d

	#ifndef ITK_FUTURE_LEGACY_REMOVE
	/** Allocate memory of certain size and return it.
	* \return a non-null pointer to an array of \c size elements (0 is a valid
	* parameter).
	* \deprecated Please consider calling `std::make_unique<TValue[]>(size)` instead.
	*/
	[[deprecated("Please consider calling `std::make_unique<TValue[]>(size)` instead.")]] [[nodiscard]] TValue *
	AllocateElements(ElementIdentifier size) const;

Instead of explicitly using [[deprecated]], shouldn't we use ITK_FUTURE_DEPRECATED(message)? As defined here:

ITK/Modules/Core/Common/include/itkMacro.h

Lines 1473 to 1477 in 093598d

	#if defined(ITK_LEGACY_REMOVE) && !defined(ITK_LEGACY_SILENT)
	# define ITK_FUTURE_DEPRECATED(message) [[deprecated(message)]]
	#else
	# define ITK_FUTURE_DEPRECATED(message)
	#endif

You see ITK_FUTURE_DEPRECATED(message) respects the ITK_LEGACY_REMOVE and the ITK_LEGACY_SILENT macro.

[blowekamp]

The wrapping should be compiled with ITK_LEGACY_SILENT enabled. But this function is not respecting that, there should be some macros to support this functionality of respecting deprecation to be silent.

[N-Dekker]

But this function is not respecting that, there should be some macros to support this functionality of respecting deprecation to be silent.

Then we should fix this function, right?

[blowekamp]

That would make a fine follow up pull request, if you'd like.

[N-Dekker]

• Now that PR COMP: Make VariableLengthVector::AllocateElements ITK_FUTURE_DEPRECATED #5869 is merged (thanks Hans), the VariableLengthVector::AllocateElements warnings addressed by this PR (5863) should disappear from the CI.
• I think we can avoid warnings like these in the future, by following the strategy to consistently use ITK_FUTURE_DEPRECATED(msg) for functions that are marked "ITK_FUTURE_LEGACY_REMOVE". Right?
• You know, compiler warnings are there to help us to prevent mistakes, so I'd rather not have them globally disabled 🤷

[hjmjohnson]

@thewtex I think that @N-Dekker alternate fix in #5869 removes the need for this PR. Please close if you agree.

[thewtex]

@N-Dekker 👍 thanks