-
Notifications
You must be signed in to change notification settings - Fork 188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check for missing inline
on functions in public headers.
#2570
Conversation
We can test for missing `inline` markup by linking the header objects into the same target twice. By default CMake will eliminate duplicate link targets, but linking both the target and the $<TARGET_OBJECTS:target> bypasses the duplicate checks.
@robertmaynard I'm curious what your thoughts on this would be. Too hacky? Am I relying on a bug in the duplicate link target detection, or is this behavior intentional? |
🟩 CI finished in 55m 09s: Pass: 100%/372 | Total: 1d 13h | Avg: 6m 01s | Max: 43m 16s | Hits: 99%/27949
|
Project | |
---|---|
+/- | CCCL Infrastructure |
libcu++ | |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda | |
CCCL C Parallel Library |
Modifications in project or dependencies?
Project | |
---|---|
+/- | CCCL Infrastructure |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CCCL C Parallel Library |
🏃 Runner counts (total jobs: 372)
# | Runner |
---|---|
298 | linux-amd64-cpu16 |
31 | linux-amd64-gpu-v100-latest-1 |
28 | linux-arm64-cpu16 |
15 | windows-amd64-cpu16 |
I would be more comfortable if we would just create a second .so and link that in |
The issue with that is we'd need to compile all the headers twice, and I'd like to avoid that if possible. This approach allows us to only compile the header units once and then just link the same TUs twice, so long as we can rely on this behavior from |
We are using sccache with the header tests arent we? I would guess there is almost no cost do doing it like that |
You can try creating a STATIC library that combines all the objects and also consuming that along side the original OBJECT library. That will give you two sets of the object files with only a single compilation |
I did some more experiments, and using a static lib doesn't do what we want here, cmake still removes the duplicate objects unless AFAICT the current version of this patch is the only way to link the objects twice without doubling compilation overhead. |
Sccache may help, though the duplicate header build rules may run in parallel and not benefit from caching. I'd still prefer to avoid redundant work if possible. |
* Check for missing `inline` on functions in public headers. We can test for missing `inline` markup by linking the header objects into the same target twice. By default CMake will eliminate duplicate link targets, but linking both the target and the $<TARGET_OBJECTS:target> bypasses the duplicate checks. * Add -fPIC, formatting. * Switch link_check to executable to avoid -fPIC issues
* Check for missing `inline` on functions in public headers. We can test for missing `inline` markup by linking the header objects into the same target twice. By default CMake will eliminate duplicate link targets, but linking both the target and the $<TARGET_OBJECTS:target> bypasses the duplicate checks. * Add -fPIC, formatting. * Switch link_check to executable to avoid -fPIC issues
We can test for missing
inline
markup by linking the header objects into the same target twice. By default CMake will eliminate duplicate link targets, but linking both the target and the $<TARGET_OBJECTS:target> bypasses the duplicate checks.