-
Notifications
You must be signed in to change notification settings - Fork 707
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
Play with C++20 concepts #14836
Play with C++20 concepts #14836
Changes from all commits
7893c94
a12b99d
8a1de1b
395d489
1f035e1
b209c02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,6 +150,18 @@ | |
#cmakedefine DEAL_II_HAVE_CXX17 | ||
#cmakedefine DEAL_II_HAVE_CXX20 | ||
|
||
/** | ||
* If we have C++20 available, we can have concepts and requires | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably check that we define DEAL_II_HAVE_CXX20 only if the compiler supports concepts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean to exclude compilers that have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, adding something using concepts in cmake/checks/check_01_cxx_features.cmake. |
||
* clauses. We want to avoid using too many `#ifdef` statements, so | ||
* define a convenience macro that allows us to write a 'requires' | ||
* clause that is simply removed when not using C++20. | ||
*/ | ||
#ifdef DEAL_II_HAVE_CXX20 | ||
# define DEAL_II_CXX20_REQUIRES(condition) requires(condition) | ||
#else | ||
# define DEAL_II_CXX20_REQUIRES(condition) | ||
#endif | ||
|
||
#cmakedefine DEAL_II_HAVE_FP_EXCEPTIONS | ||
#cmakedefine DEAL_II_HAVE_COMPLEX_OPERATOR_OVERLOADS | ||
#cmakedefine DEAL_II_HAVE_CXX17_BESSEL_FUNCTIONS | ||
|
@@ -174,9 +186,10 @@ | |
#cmakedefine DEAL_II_FALLTHROUGH @DEAL_II_FALLTHROUGH@ | ||
#cmakedefine DEAL_II_CONSTEXPR @DEAL_II_CONSTEXPR@ | ||
|
||
// defined for backwards compatibility with older deal.II versions | ||
// The following two are defined for backwards compatibility with older deal.II versions: | ||
#define DEAL_II_WITH_CXX11 | ||
#define DEAL_II_WITH_CXX14 | ||
|
||
#ifdef DEAL_II_HAVE_CXX17 | ||
# define DEAL_II_WITH_CXX17 | ||
#endif | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for conformity (might also be slightly faster).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I actually would like to keep it as is. If we don't trust that
-std=c++20
works, then we also shouldn't trust that__cpp_concepts
is set correctly. Let's test for the actual feature we want to use.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of these
CMake
features check really is that we decide if C++20 is supported based on the capabilities of the compilers with the given flags and not because the user requested C++20 support in any other way (at the moment). Thus, I wouldn't say that we don't trust that-std=c++20
works (since compilers allow passing this flag without supporting all of C++20 anyway). On the other hand, I think that the__cpp_concepts
feature check is reliable, though. Anyway, I rather wanted to suggest using that than requesting changes.