Cytnx icon indicating copy to clipboard operation
Cytnx copied to clipboard

Upgrade project C++ standard to C++20

Open IvanaGyro opened this issue 9 months ago • 9 comments

C++20 includes several valuable features:

  • Concepts (clearer template constraints)
  • Ranges (improved algorithm readability)
  • Three-way comparison operator (<=>) (simplifies comparison logic)
  • std::span (easier handling of contiguous data)
  • std::format (modern string formatting)

These features will help improve code readability.

IvanaGyro avatar Mar 10 '25 08:03 IvanaGyro

Let's chat on this a bit more. Specifically I also want to explore a bit on module from C++20

kaihsin avatar Mar 10 '25 08:03 kaihsin

Would you mind focus on the refactor of storage first before bumping this up to C++20? Or you need any features from C++20?

kaihsin avatar Mar 10 '25 08:03 kaihsin

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 23.55%. Comparing base (4d3547e) to head (c81f8a2). Report is 55 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #567      +/-   ##
==========================================
- Coverage   23.57%   23.55%   -0.03%     
==========================================
  Files         211      211              
  Lines       44929    44929              
  Branches    14022    13896     -126     
==========================================
- Hits        10594    10581      -13     
- Misses      32665    32667       +2     
- Partials     1670     1681      +11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Mar 10 '25 08:03 codecov[bot]

I would suggest to use std::mdspan which will be part of C++23, and has a reference implementation

https://en.cppreference.com/w/cpp/container/mdspan

https://github.com/kokkos/mdspan

yingjerkao avatar Mar 10 '25 08:03 yingjerkao

Modules are not really usable in C++20. I think everyone is hoping C++26 fixes it....

On Mon, Mar 10, 2025 at 6:30 PM Kai-Hsin Wu, PhD @.***> wrote:

Let's chat on this a bit more. Specifically I also want to explore a bit on module from C++20

— Reply to this email directly, view it on GitHub https://github.com/Cytnx-dev/Cytnx/pull/567#issuecomment-2709783038, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBJPWXKCSB4LMQCRKYUJVD2TVLRLAVCNFSM6AAAAABYVO2YLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMBZG44DGMBTHA . You are receiving this because you are subscribed to this thread.Message ID: @.***> [image: kaihsin]kaihsin left a comment (Cytnx-dev/Cytnx#567) https://github.com/Cytnx-dev/Cytnx/pull/567#issuecomment-2709783038

Let's chat on this a bit more. Specifically I also want to explore a bit on module from C++20

— Reply to this email directly, view it on GitHub https://github.com/Cytnx-dev/Cytnx/pull/567#issuecomment-2709783038, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBJPWXKCSB4LMQCRKYUJVD2TVLRLAVCNFSM6AAAAABYVO2YLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMBZG44DGMBTHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

ianmccul avatar Mar 10 '25 08:03 ianmccul

Would you mind focus on the refactor of storage first before bumping this up to C++20? Or you need any features from C++20?

The work of refactoring storage is paused because the storage component is being moved to another project. If the code is unseen, we will not have to touch it.

Is there any concern about raising the version of the C++ standard higher than C++20? It seems that we didn't use any feature that was removed since c++20.

IvanaGyro avatar Mar 10 '25 09:03 IvanaGyro

I would suggest to use std::mdspan which will be part of C++23, and has a reference implementation

https://en.cppreference.com/w/cpp/container/mdspan

https://github.com/kokkos/mdspan

GCC somehow hasn't implemented mdspan. I learned this from the compiler support table. There are two options. One is to drop the support of GCC and use Clang only. The other is making kokkos' implementation as a dependency until GCC implements mdspan. Do you have any ideas?

IvanaGyro avatar Mar 10 '25 09:03 IvanaGyro

GCC somehow hasn't implemented mdspan. I learned this from the compiler support table. There are two options. One is to drop the support of GCC and use Clang only. The other is making kokkos' implementation as a dependency until GCC implements mdspan. Do you have any ideas?

Since the reference implementation is pretty complete and should be simply replaced to std::mdspan when the compiler is ready, we should go this route.

yingjerkao avatar Mar 10 '25 13:03 yingjerkao

cuda::std also implemented mdspan.

IvanaGyro avatar Mar 15 '25 07:03 IvanaGyro

I'm not sure it the discussion will happen. In order to move forward, I am merging this PR now. We can revert it in the future.

IvanaGyro avatar May 29 '25 03:05 IvanaGyro