Add Visual Studio specific debug helpers to ITKCommon project - #6699
Add Visual Studio specific debug helpers to ITKCommon project#6699dzenanz wants to merge 2 commits into
Conversation
github-actions
Bot
added
type:Infrastructure
|
Reviewed this on two axes (repo standards + does it do what the description claims). The standards side is minor, but the spec side has one blocker: a project-embedded Two things worth fixing regardless of what you decide about the step filter:
Spec: does the change deliver what the description claims?
Standards: repo conventions
The explanatory comments on the two |
6d710a5 to
35ef325
Compare
|
This doesn't seem to work. It needs to be added to end-user executable. Maybe what we can do here is put these files in a more visible location so they are easy to reference from a project using ITK. We could also add them to tests, so we get this nicety when debugging tests. |
8da6c3b to
8592d9f
Compare
|
Propagation of both files via interface sources works. However, ITK.natjmc is ignored. Stepping into goes into |
|
Using |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
| Filename | Overview |
|---|---|
| Modules/Core/Common/src/CMakeLists.txt | Adds MSVC debugger metadata to ITKCommon and now anchors the installed Ninja Natvis path with the installation prefix. |
| Utilities/Debugger/ITK.natvis | Generalizes FixedArray visualization and adds visualizers for several related ITK data types. |
| Utilities/Debugger/ITK.natstepfilter | Adds an ITK-focused native debugger step filter. |
| Utilities/Debugger/default.natstepfilter | Removes the former machine-wide generic step-filter definition. |
Reviews (3): Last reviewed commit: "ENH: Expand Visual Studio debug visualiz..." | Re-trigger Greptile
Starting in Visual Studio 2022 version 17.6, copying these files to VS installation directory will no longer be required. Natvis has been supported for longer, including all currently supported Visual Studio versions.
|
@greptileai re-review. |
|
This works with latest VS2022, both from build and install directories. |
|
AI-assisted review of the debugger-asset wiring. The mechanism works for the generator ITK itself was configured with, but the usage requirements are baked at ITK configure time and consumed by projects that may use a different generator, so several paths either break downstream Blocking: the 1. INSTALL_INTERFACE sources make the debug assets a hard requirement for consumers (Modules/Core/Common/src/CMakeLists.txt:308)
under both Ninja and Unix Makefiles. The 2. LINK_DEPENDS is on ITKCommon, but /NATVIS: is INTERFACE-only (Modules/Core/Common/src/CMakeLists.txt:293)
Consequence on MSVC+Ninja: editing Additionally, with 3. The generator branch is decided by ITK's generator, not the consumer's (Modules/Core/Common/src/CMakeLists.txt:284)The
Either way the consumer silently gets less than advertised. 4. ITK.natstepfilter is installed but never wired up for Ninja (Modules/Core/Common/src/CMakeLists.txt:286)The Ninja branch references only So on the standard MSVC+Ninja configuration, F11 on 5. MATCHES "Ninja" does not match NMake / MinGW Makefiles (Modules/Core/Common/src/CMakeLists.txt:284)The comment on line 285 says "Ninja/Makefiles Generator support", but the regex matches only Verified that Makefile generators silently drop an unknown-extension source: 6. The std:: NoStepInto rule is shipped active while the comment says it is disabled (Utilities/Debugger/ITK.natstepfilter:7-8)<!-- We can also disable stepping into STL functions by uncommeting the next line: -->
<Function><Name>std::.+</Name><Action>NoStepInto</Action></Function>Line 8 is not commented out, so the comment asserts the opposite of the shipped state. This matters more than a stale comment because the rule is no longer opt-in: the VS-generator branch injects the filter into every consuming project. A downstream developer loses F11 into anything reached through a (Also flagged separately as a behavioral concern: force-applying a 7. Absolute ITK_INSTALL_DATA_DIR produces an impossible /NATVIS: path (Modules/Core/Common/src/CMakeLists.txt:290)
With 8. The manual-install instruction was deleted with no replacement (Utilities/Debugger/ITK.natvis:2)The removed line <!-- e.g. copy into C:\Program Files (x86)\...\Debugger\Visualizers\ITK.natvis -->was the only documented path for users who do not consume ITK through its exported CMake targets: hand-maintained 9. INTERFACE propagation reaches every target that links ITKCommon (Modules/Core/Common/src/CMakeLists.txt:286) — cleanupAttaching the assets to |
|
@dzenanz The review in the previous comment should not be viewed as a blocker; these are concerns that arose during an AI code review. I wanted to bring these concerns to your attention. |
|
I will try to address some of it. |
|
Mixing VS and Ninja generators never worked for me. I will not even attempt to support this combination. I am now building with Ninja generator. |
|
Compiling with ninja worked without any issues, both against build tree and install tree. The compiled test executable works. |
* Generalize FixedArray for template types other than float/double * Add customized member names for Point Add new visualizers for: * Vector * CovariantVector * ContinuousIndex * RGBPixel * RGBAPixel * VariableLengthVector * Matrix
|
Testing with Ninja+MSVC 2026 revealed no problems: |
|
I am thinking about point 9. Should we expose a CMake option for this? To allow someone to turn it off, if they are trying to minimize size of debugging information ( |
|
2 participants
Copying these files to the installation directory will not be required any more.
PR Checklist