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

ENH: Update required standard to C++20 #1004

Merged
merged 26 commits into from
Jul 9, 2024
Merged

ENH: Update required standard to C++20 #1004

merged 26 commits into from
Jul 9, 2024

Conversation

JDuffeyBQ
Copy link
Collaborator

No description provided.

@JDuffeyBQ JDuffeyBQ force-pushed the feature/cpp20 branch 4 times, most recently from 2b13c86 to fd16894 Compare July 1, 2024 17:24
@imikejackson imikejackson changed the title Update required standard to C++20 ENH: Update required standard to C++20 Jul 2, 2024
@JDuffeyBQ JDuffeyBQ force-pushed the feature/cpp20 branch 2 times, most recently from e8a1f9c to f01390a Compare July 9, 2024 17:29
@JDuffeyBQ JDuffeyBQ marked this pull request as ready for review July 9, 2024 18:10
@nyoungbq
Copy link
Contributor

nyoungbq commented Jul 9, 2024

Some notes:
Since this PR is mainly about getting compatibility up, I don't think these are necessary here I just wanted to mention them, since they are related to the topic of the PR.

  • Once this goes in we should be able to change the nonstd::span to std::span since it is included in c++20
  • Some of the enable_if statements in the template declarations should be reviewed to see if they can be changed to concept declarations for readability and more centralized type gating (such as in the types.hpp file)
  • Other template functions should be evaluated to see if we can use concept to reduce the compilation scope of the template heavy files
  • Unless I am misunderstanding what BasicStringLiteral does in the current code base, the StringLiteral alias can be switched to a std::string_view

@JDuffeyBQ
Copy link
Collaborator Author

Yeah I was waiting until this gets merged to do any improvements to keep this PR minimal.

For nonstd::span it actually detects if std::span is available and typedefs it if it is. So it will be using it for the compile but to minimize our dependencies we can remove it in the future.

The issue with std::string_view is that it is not guaranteed to be null terminated since it can represent an arbitrary slice of a string. Our StringLiteral is a null terminated string basically a std::string_view-like wrapper around a true string literal. This is useful for third party APIs that take char* where they assume it is null terminated.

JDuffeyBQ and others added 17 commits July 9, 2024 16:12
Signed-off-by: Jared Duffey <[email protected]>
* Ensures the __cplusplus macro has the correct value
* The default behavior has it set to 199711L for legacy reasons

Signed-off-by: Jared Duffey <[email protected]>
Signed-off-by: Jared Duffey <[email protected]>
Signed-off-by: Jared Duffey <[email protected]>
* cbegin()/cend() for std::span won't be added until C++23

Signed-off-by: Jared Duffey <[email protected]>
Signed-off-by: Jared Duffey <[email protected]>
* Functors passed to tbb::parallel_for must satisfy the parallel_for_body
concept which requires that they are copy constructible

Signed-off-by: Jared Duffey <[email protected]>
Signed-off-by: Jared Duffey <[email protected]>
Signed-off-by: Jared Duffey <[email protected]>
* Causes issues on macOS and already covered by uint64 array

Signed-off-by: Jared Duffey <[email protected]>
* Code outside of sections runs again for each section which was not
necessary for this test case

Signed-off-by: Jared Duffey <[email protected]>
@JDuffeyBQ JDuffeyBQ enabled auto-merge (squash) July 9, 2024 20:40
@JDuffeyBQ JDuffeyBQ merged commit 31406a4 into develop Jul 9, 2024
7 checks passed
@JDuffeyBQ JDuffeyBQ deleted the feature/cpp20 branch July 9, 2024 22:04
imikejackson added a commit to imikejackson/simplnx that referenced this pull request Oct 20, 2024
* Updated to Eigen 3.4.0
* Updated to expected-lite 0.8.0
* Updated to nod 0.5.4
* Removed gcc9 CI
* Added /Zc:__cplusplus compile flag for msvc
* Fixed fmtlib formatting
* Fixed tbb::parallel_for functors
* Functors passed to tbb::parallel_for must satisfy the parallel_for_body
concept which requires that they are copy constructible
* Fixed various compile errors on C++20
* Fixed FeatureFaceCurvatureTest where the principal directions
are unique up to a sign
* Fixed ComputeFeatureReferenceCAxisMisorientationsTest and
ComputeFeatureNeighborCAxisMisalignmentsTest by using 
double precision to get the values to agree on all platforms
regardless of vectorization

Signed-off-by: Jared Duffey <[email protected]>
Signed-off-by: Michael Jackson <[email protected]>
Co-authored-by: Michael Jackson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants