Skip to content
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

Properly test internal headers #1258

Merged
merged 29 commits into from
Jan 18, 2024

Conversation

miscco
Copy link
Collaborator

@miscco miscco commented Jan 9, 2024

Currently we werent testing all potential fallbacks we are using because some of the intrinsics are just available everywhere. This obviously led to some bugs slip in that would break everthing that includes type_traits.

To prevent this from happening again, add a script that crawls all internal headers and ensures both that they are building on their own and that all fallbacks path are at least exercised

This addresses nvbug4433362

@miscco miscco added libcu++ For all items related to libcu++ backport branch/2.3.x For backporting to the 2.3.x release branch bug: functional labels Jan 9, 2024
@miscco miscco requested review from a team as code owners January 9, 2024 18:26
@miscco miscco force-pushed the use_proper_includes_is_base_of branch from 76c19c2 to fe7e184 Compare January 9, 2024 18:41
@miscco miscco force-pushed the use_proper_includes_is_base_of branch 3 times, most recently from 9d61cfe to 87093a2 Compare January 16, 2024 11:07
@miscco miscco requested a review from a team as a code owner January 16, 2024 11:07
@miscco miscco force-pushed the use_proper_includes_is_base_of branch from 87093a2 to db2ae15 Compare January 16, 2024 11:12
@miscco
Copy link
Collaborator Author

miscco commented Jan 16, 2024

@gevtushenko @robertmaynard I believe I have implemented the internal header test functionality in CMAKE.

That said, while I see that the file is actually used and the target generated, it seems to never be compiled. thoughts?

@miscco miscco force-pushed the use_proper_includes_is_base_of branch 7 times, most recently from f2a6cd9 to 7b7b9b2 Compare January 16, 2024 14:09
@miscco
Copy link
Collaborator Author

miscco commented Jan 16, 2024

@wmaxey I believe this supersedes #1099

@miscco miscco force-pushed the use_proper_includes_is_base_of branch from 7b7b9b2 to efe6a84 Compare January 16, 2024 14:13
@robertmaynard
Copy link
Contributor

@gevtushenko @robertmaynard I believe I have implemented the internal header test functionality in CMAKE.

That said, while I see that the file is actually used and the target generated, it seems to never be compiled. thoughts?

Looking at https://github.com/NVIDIA/cccl/actions/runs/7542844478/job/20532768330?pr=1258 I see header tests are being compiled. So I think you resolved this issue?

@miscco miscco force-pushed the use_proper_includes_is_base_of branch from efe6a84 to 7634629 Compare January 16, 2024 14:39
@miscco miscco force-pushed the use_proper_includes_is_base_of branch from e201a1e to 751f86d Compare January 17, 2024 10:31
@miscco miscco force-pushed the use_proper_includes_is_base_of branch from 27e89be to dba5031 Compare January 17, 2024 14:25
@miscco miscco force-pushed the use_proper_includes_is_base_of branch from 50892fc to b5248a6 Compare January 18, 2024 07:28
@miscco miscco force-pushed the use_proper_includes_is_base_of branch 3 times, most recently from 5bfbb0d to ddf246b Compare January 18, 2024 10:51
@miscco miscco force-pushed the use_proper_includes_is_base_of branch from ddf246b to dd5a517 Compare January 18, 2024 10:52
@miscco miscco merged commit f0f8808 into NVIDIA:main Jan 18, 2024
538 checks passed
@miscco miscco deleted the use_proper_includes_is_base_of branch January 18, 2024 19:47
Copy link
Contributor

Backport failed for branch/2.3.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin branch/2.3.x
git worktree add -d .worktree/backport-1258-to-branch/2.3.x origin/branch/2.3.x
cd .worktree/backport-1258-to-branch/2.3.x
git checkout -b backport-1258-to-branch/2.3.x
ancref=$(git merge-base 6034056fa7c37b7244920b1eb653b35b5a945a80 345b08e3fbdc1c1500992638af43e056d65ffd8f)
git cherry-pick -x $ancref..345b08e3fbdc1c1500992638af43e056d65ffd8f

miscco added a commit to miscco/cccl that referenced this pull request Jan 19, 2024
* Add tests for internal headers, so that we know that they do not rely on transitive includes
* Add tests for fallback definitions of traits, to ensure hat all fallbacks are functional
* Add tests that all public headers can be included
* Add tests that all public headers can be included by just a host compiler

Co-authored-by: Wesley Maxey <wesley.maxey@gmail.com>
miscco added a commit to miscco/cccl that referenced this pull request Jan 19, 2024
* Add tests for internal headers, so that we know that they do not rely on transitive includes
* Add tests for fallback definitions of traits, to ensure hat all fallbacks are functional
* Add tests that all public headers can be included
* Add tests that all public headers can be included by just a host compiler

Co-authored-by: Wesley Maxey <wesley.maxey@gmail.com>
jrhemstad pushed a commit that referenced this pull request Jan 22, 2024
* Add tests for internal headers, so that we know that they do not rely on transitive includes
* Add tests for fallback definitions of traits, to ensure hat all fallbacks are functional
* Add tests that all public headers can be included
* Add tests that all public headers can be included by just a host compiler

Co-authored-by: Wesley Maxey <wesley.maxey@gmail.com>
@miscco miscco added backport done This PR has been backported to the relevant branch and removed backport branch/2.3.x For backporting to the 2.3.x release branch labels Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport done This PR has been backported to the relevant branch bug: functional libcu++ For all items related to libcu++
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants