COMP: Support C17/C23 standards in VXL and modernize vcl iterator test by thewtex · Pull Request #6558 · InsightSoftwareConsortium/ITK

thewtex · 2026-07-06T18:28:55Z

Summary

Enables configuring and building ITK's vendored VXL under the newer C17 and C23 language standards, and repairs the vcl test_iterator test, which failed to compile under C++17 and later.

Motivation

Modules/ThirdParty/VNL/src/vxl/CMakeLists.txt validates CMAKE_C_STANDARD against a hard-coded VALID_C_STANDARDS list and issues a FATAL_ERROR for anything outside it. The list only allowed 90, 99, and 11, so configuring ITK with -DCMAKE_C_STANDARD=17 (or 23) aborted the build even though modern toolchains support these standards — and even though the C++ side already accepts 11/14/17/20.

Separately, vcl/tests/test_iterator.cxx contained a compile-time probe of std::iterator:

void f(std::iterator<float, int> *) { }

std::iterator has been deprecated since C++17. Under -Werror=deprecated-declarations (used in ITK CI-style strict builds) this breaks the vcl test suite on both GCC and Clang when building with C++17 or newer.

Changes

Modules/ThirdParty/VNL/src/vxl/CMakeLists.txt — add "17" and "23" to VALID_C_STANDARDS so those standards pass the existing IN_LIST validation guard.
Modules/ThirdParty/VNL/src/vxl/vcl/tests/test_iterator.cxx — remove the deprecated std::iterator<float, int> probe. Rather than leaving the test body empty, add runtime coverage of the non-deprecated <iterator> facilities:
- std::iterator_traits
- std::distance / std::advance / std::next / std::prev
- std::begin / std::end (containers and C arrays)
- std::back_inserter
- std::reverse_iterator (via rbegin/rend)
- std::ostream_iterator / std::istream_iterator
The test follows the fail-flag style of the sibling tests (e.g. test_vector.cxx) and returns non-zero on any mismatch.

Testing

The new test compiles warning-free under -Wall -Wextra -Wdeprecated -Werror and passes on both GCC and Clang across C++14, C++17, C++20, and C++23. The change is confined to a self-contained standard-library test and a CMake validation list; no public API is affected.

🤖 Generated with Claude Code

greptile-apps · 2026-07-06T18:32:53Z

Greptile Summary

This PR updates VXL standard handling and modernizes the iterator test. The main changes are:

Adds 17 and 23 to the accepted CMAKE_C_STANDARD values.
Removes the deprecated std::iterator compile probe.
Adds runtime checks for current standard iterator utilities.

Confidence Score: 4/5

The PR is close, but the CMake compatibility issue should be fixed before merging.

The iterator test change is self-contained. The CMake change creates a real compatibility failure for users on the declared minimum CMake version.

Modules/ThirdParty/VNL/src/vxl/CMakeLists.txt

T-Rex Logs

What T-Rex did

Reproduced the version gate issue where CMake 3.16.3 cannot enable the C23 dialect, and ran a minimal CMake harness that preserves the VXL C standard validation to observe the failure.
Observed that C++17 and C++23 builds and runtimes completed successfully for the iterator tests, indicating language-level compatibility along those paths.
Noted that GCC 12 rejects -std=c23 for C, while GCC/G++ 12.2.0 can run with -std=c2x, clarifying the dialect-support constraints affecting the C23 path.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
Modules/ThirdParty/VNL/src/vxl/CMakeLists.txt	Adds C17/C23 to the accepted standards list, but does not reconcile those values with the file's CMake 3.16.3 minimum.
Modules/ThirdParty/VNL/src/vxl/vcl/tests/test_iterator.cxx	Replaces the deprecated `std::iterator` probe with runtime coverage of standard iterator utilities.

_{Reviews (1): Last reviewed commit: "COMP: Support C17/C23 standards in VXL a..." | Re-trigger Greptile}

Add "17" and "23" to VALID_C_STANDARDS in the vendored VXL CMake configuration so ITK can be configured and built with the newer C17 and C23 language standards, matching the C++ standards already accepted (11/14/17/20). The "17" and "23" values of CMAKE_C_STANDARD were only added to CMake in version 3.21, but this configuration declares a lower cmake_minimum_required. To avoid a confusing downstream failure where an older CMake accepts the value here and then errors when it tries to apply a C standard it does not understand, the two new values are only appended to VALID_C_STANDARDS when the running CMake is 3.21 or newer. On older CMake, requesting C17/C23 now fails early with the existing clear validation message. The C++ standards list needs no such guard because its "17"/"20" values predate the declared CMake minimum. The vcl test_iterator test previously probed std::iterator<float, int>: void f(std::iterator<float, int> *) { } std::iterator has been deprecated since C++17 and this usage triggers -Werror=deprecated-declarations on GCC and Clang when building under C++17 or later, breaking the vcl test suite. Remove the deprecated probe and, rather than leaving an empty test body, replace it with runtime coverage of the non-deprecated <iterator> facilities: iterator_traits, distance/advance/next/prev, begin/end, back_inserter, reverse_iterator, and the ostream/istream stream iterators. The new test compiles warning-free under -Wall -Wextra -Wdeprecated -Werror and passes with GCC and Clang across C++14, C++17, C++20, and C++23. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

dzenanz

CMake change looks OK. I have not taken a good look at the test.

hjmjohnson · 2026-07-06T20:44:44Z

 int test_iterator_main(int /*argc*/,char* /*argv*/[])
 {
-  // invent some more tests.
-  return 0;


The old test did nothing, but the new tests only test the std:: behaviors on std:: objects, and there is not exercising of vnl objects. I think removing the file completely would be better than adding tests of the standard template library behavior. It is not clear to me what should be tested in this file.

hjmjohnson · 2026-07-06T20:47:13Z

-  return 0;
+  bool fail = false;
+
+  // std::iterator_traits should expose the expected typedefs.


The vcl layer was vxl's 1990s/2000s compiler-portability shim, and vcl/tests/ historically validated that the platform's standard library was usable at all — meaningful on pre-standard compilers, obsolete now. This particular file had already decayed to a single declaration, void f(std::iterator<float,int>*), plus a // invent some more tests comment.

This test is no longer relavant in the ITK vendored vxl.

hjmjohnson · 2026-07-06T20:51:37Z

+# valid when the running CMake understands them. Otherwise a user could pass
+# this validation on an older CMake and then hit a confusing failure when CMake
+# tries to apply a C standard it does not know about.
+if(NOT CMAKE_VERSION VERSION_LESS "3.21")


OK for the release-5.4, but unnecessary for the main release where cmake >3.22.1

hjmjohnson · 2026-07-06T20:53:37Z

  set(CMAKE_C_EXTENSIONS OFF)
 endif()
 set(VALID_C_STANDARDS "90" "99" "11")
+# The "17" and "23" values of CMAKE_C_STANDARD were added in CMake 3.21.


This comment is verbose for what is fairly obvious from the code itself.

hjmjohnson · 2026-07-06T20:54:23Z

+# valid when the running CMake understands them. Otherwise a user could pass
+# this validation on an older CMake and then hit a confusing failure when CMake
+# tries to apply a C standard it does not know about.
+if(NOT CMAKE_VERSION VERSION_LESS "3.21")


if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.21")

hjmjohnson

The current test does not test anything related to vcl. The tests are required to pass for any compliant compiler, so it is really testing if std:: works, not if vxl works.

github-actions Bot added type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:ThirdParty Issues affecting the ThirdParty module labels Jul 6, 2026

thewtex requested review from dzenanz, hjmjohnson and seanm July 6, 2026 18:31

greptile-apps Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread Modules/ThirdParty/VNL/src/vxl/CMakeLists.txt Outdated

thewtex force-pushed the vxl-c-version branch from a690f81 to 6fbd62b Compare July 6, 2026 18:40

dzenanz approved these changes Jul 6, 2026

View reviewed changes

thewtex added this to the ITK 5.4.7 milestone Jul 6, 2026

hjmjohnson reviewed Jul 6, 2026

View reviewed changes

hjmjohnson requested changes Jul 6, 2026

View reviewed changes

This was referenced Jul 6, 2026

COMP: Remove obsolete vcl iterator test InsightSoftwareConsortium/vxl#10

Merged

ENH: Update vendored VNL pin to for/itk-vxl-master-ae2f579 #6559

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

COMP: Support C17/C23 standards in VXL and modernize vcl iterator test#6558

COMP: Support C17/C23 standards in VXL and modernize vcl iterator test#6558
thewtex wants to merge 1 commit into
InsightSoftwareConsortium:release-5.4from
thewtex:vxl-c-version

thewtex commented Jul 6, 2026

Uh oh!

greptile-apps Bot commented Jul 6, 2026

Greptile Summary

Confidence Score: 4/5

T-Rex Logs

Important Files Changed

Uh oh!

Uh oh!

dzenanz left a comment

Uh oh!

hjmjohnson Jul 6, 2026

Uh oh!

hjmjohnson Jul 6, 2026

Uh oh!

hjmjohnson Jul 6, 2026

Uh oh!

hjmjohnson Jul 6, 2026

Uh oh!

hjmjohnson Jul 6, 2026

Uh oh!

hjmjohnson left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

thewtex commented Jul 6, 2026

Summary

Motivation

Changes

Testing

Uh oh!

greptile-apps Bot commented Jul 6, 2026

Greptile Summary

Confidence Score: 4/5

T-Rex Logs

Important Files Changed

Uh oh!

Uh oh!

dzenanz left a comment

Choose a reason for hiding this comment

Uh oh!

hjmjohnson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

hjmjohnson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

hjmjohnson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

hjmjohnson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

hjmjohnson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

hjmjohnson left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants