Repository navigation
Conversation
Under nvcc, nanovdb/GridHandle.h includes cuda/GridHandle.cuh at its end, and that header depends on cuda/HandleStorage.h and tools/cuda/GridChecksum.cuh, which both include GridHandle.h before their own definitions. Including either of them first therefore compiled cuda/GridHandle.cuh against their half-parsed contents. Since the single-space merge/split landed in cuda/GridHandle.cuh this broke HandleStorage.h, TopologyBuilder.cuh and VoxelBlockManager.cuh as a first include; GridChecksum.cuh and GridValidator.cuh were already broken the same way. While either header is being parsed it now defers cuda/GridHandle.cuh to its own end, and GridHandle.h skips it. Also give GridStats.cuh and util/cuda/MorphologyHelpers.cuh the includes they use, and add a compile-only target that builds each public CUDA header (except the deprecated forwarding headers) as the first include of its own translation unit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>
swahtz
reviewed
Oct 21, 2026
swahtz
left a comment
swahtz
left a comment
Contributor
There was a problem hiding this comment.
Thanks for finding this and fixing it. I think there's just two small changes for consistency, but otherwise looks pretty good to me.
The header check now also compiles every installed C++ host header (GridHandle.h, io/IO.h, tools/CreateNanoGrid.h, ...) as the first include of a CUDA translation unit, since those are the headers whose behaviour under nvcc changes with the deferred cuda/GridHandle.cuh include. The C headers are skipped, and tools/NanoToOpenVDB.h is checked only with OpenVDB. Also gate GridChecksum.cuh's deferred include on __CUDACC__, like the other two. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>
swahtz
approved these changes
Oct 21, 2026
swahtz
left a comment
swahtz
left a comment
Contributor
There was a problem hiding this comment.
Looks good, thanks for fixing this.
sifakis
added a commit
to sifakis/openvdb
that referenced
this pull request
Oct 22, 2026
DeviceGridTraits.cuh used cudaCheck without including util/cuda/Util.h, where it is defined. This went unnoticed because every cudaCheck argument depended on the template parameter, so the name was only looked up at instantiation. The cudaStreamSynchronize(stream) calls added with the stream parameter are not dependent, so the header no longer compiled on its own, which the header self-containment check from AcademySoftwareFoundation#2377 enforces. Signed-off-by: Efty Sifakis <esifakis@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Under nvcc,
#include <nanovdb/cuda/HandleStorage.h>fails to compile when it comes before any header that includesnanovdb/GridHandle.h. That's the header users include forcuda::copyTo, and the deprecation messages and docs in progress point to it.tools/cuda/TopologyBuilder.cuhandtools/cuda/VoxelBlockManager.cuhfail the same way. These three regressed with #2370; v13.1.0 and the commit before #2370 compile them fine.The cause is an include cycle:
GridHandle.hincludescuda/GridHandle.cuhat its end.cuda/GridHandle.cuhdepends oncuda/HandleStorage.handtools/cuda/GridChecksum.cuh.GridHandle.hbefore their own definitions.So including either of them first compiles
cuda/GridHandle.cuhagainst their half-parsed contents. #2370's single-space merge/split code namesHandleStorage.hhelpers outside any template-dependent context, which exposed the cycle forHandleStorage.h.GridChecksum.cuhandGridValidator.cuhwere already broken the same way in v13.1.0.The fix:
GridHandle.hskips its trailing include while a marker is set, and the outermost of the two headers includescuda/GridHandle.cuhat its own end, once its definitions exist. A translation unit that gotcuda/GridHandle.cuhbefore still gets it, andHandleStorage.hstill compiles in a host-only translation unit.tools/cuda/GridStats.cuh(cuda/NodeManager.cuh, also broken in v13.1.0) andutil/cuda/MorphologyHelpers.cuh(NanoVDB.h,<stdint.h>) gain the includes they use, and a few standard headers are now included directly.nanovdb_cuda_header_check, compiles each installed C++ header (host and CUDA, 63 in all) as the first include of its own CUDA translation unit. The deprecated forwarding headers are skipped because they warn by design, the C headers are skipped, andtools/NanoToOpenVDB.his checked only with OpenVDB. It's built whenever the CUDA unit tests are configured, before the GTest check.Validation on one RTX PRO 6000 Blackwell GPU with nvcc 12.6:
GridHandle.handHandleStorage.hput back, the check fails on exactly the five headers listed above.-Werror.🤖 Generated with Claude Code