Skip to content

Use C++20 standard#749

Merged
rapids-bot[bot] merged 16 commits intorapidsai:branch-25.10from
kingcrimsontianyu:cpp-20
Jul 25, 2025
Merged

Use C++20 standard#749
rapids-bot[bot] merged 16 commits intorapidsai:branch-25.10from
kingcrimsontianyu:cpp-20

Conversation

@kingcrimsontianyu
Copy link
Contributor

@kingcrimsontianyu kingcrimsontianyu commented Jun 6, 2025

This PR changes KvikIO C++ standard from 17 to 20.

Depends on #751

@kingcrimsontianyu kingcrimsontianyu added improvement Improves an existing functionality non-breaking Introduces a non-breaking change c++ Affects the C++ API of KvikIO labels Jun 6, 2025
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jun 6, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@kingcrimsontianyu kingcrimsontianyu marked this pull request as ready for review June 6, 2025 01:19
@kingcrimsontianyu kingcrimsontianyu requested a review from a team as a code owner June 6, 2025 01:19
@kingcrimsontianyu kingcrimsontianyu requested a review from a team as a code owner June 9, 2025 05:39
PROPERTIES INSTALL_RPATH "\$ORIGIN/../../../lib"
CXX_STANDARD 20
CXX_STANDARD_REQUIRED ON
# For std:: support of __int128_t. Can be removed once using cuda::std
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What actions do we need to take here? Where does kvikio use __int128_t? I think we want to move to cuda::std or remove the CXX_EXTENSIONS ON if not needed.

Copy link
Contributor

@bdice bdice Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also remove this from cpp/tests/CMakeLists.txt, it seems like a copy-paste from cudf that is unnecessary in kvikio.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Done.

PROPERTIES INSTALL_RPATH "\$ORIGIN/../../../lib"
CXX_STANDARD 20
CXX_STANDARD_REQUIRED ON
CXX_EXTENSIONS ON
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this. This was the line indicated by the comment about int128 support.

Suggested change
CXX_EXTENSIONS ON

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Done.

CXX_STANDARD 20
CXX_STANDARD_REQUIRED ON
# For std:: support of __int128_t. Can be removed once using cuda::std
CXX_EXTENSIONS ON
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CXX_EXTENSIONS ON

@kingcrimsontianyu kingcrimsontianyu requested a review from a team as a code owner July 18, 2025 14:58
@vyasr vyasr changed the base branch from branch-25.08 to branch-25.10 July 25, 2025 18:44
@kingcrimsontianyu
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 7bfaf34 into rapidsai:branch-25.10 Jul 25, 2025
49 checks passed
rapids-bot bot pushed a commit to rapidsai/rapidsmpf that referenced this pull request Jul 28, 2025
Building 25.10 wheels packages fail due to the use of GCC 14, rapidsai/kvikio#749 implements a fix for that case that will probably fix it here as well.

Authors:
  - Peter Andreas Entschev (https://github.com/pentschev)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #396
rapids-bot bot pushed a commit that referenced this pull request Sep 12, 2025
Previous PR #749 forgets to bring the entrée to the table: Only the C++ code in tests and benchmarks use C++20, but not the main library. This PR fixes this oversight.

Authors:
  - Tianyu Liu (https://github.com/kingcrimsontianyu)

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)

URL: #819
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Affects the C++ API of KvikIO improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants