From c3e45088a6d126fbec5d86e366810802d569a57c Mon Sep 17 00:00:00 2001 From: Roth Michaels Date: Mon, 9 Dec 2024 15:27:36 +0100 Subject: [PATCH 1/6] Fixes for apple-clang crashes and performance issues The source of the crash with the latest changes seemed to be from the `IsOfCharacter` concept. This concept was refactored to use some type traits and more simple concepts although with less strict requirements. A new macro, `MP_UNITS_APPLE_CLANG_HACKS` was added to mp-units/bits/hacks.h to enable work arounds for pre-Xcode 16.1 apple-clang. Some tests were not passing and were removed with the preprocessor macro for apple-clang hacks is defined. Adding `ComplexFunctionsAvailable` and `VectorFunctionsAvailable` fixed some test failures in concepts_test.cpp but not all. --- src/core/include/mp-units/bits/hacks.h | 4 +++ .../framework/representation_concepts.h | 35 +++++++++++++++++-- test/static/concepts_test.cpp | 3 ++ test/static/quantity_test.cpp | 6 +++- 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/core/include/mp-units/bits/hacks.h b/src/core/include/mp-units/bits/hacks.h index 3b8c9a89..f27c5438 100644 --- a/src/core/include/mp-units/bits/hacks.h +++ b/src/core/include/mp-units/bits/hacks.h @@ -150,5 +150,9 @@ MP_UNITS_DIAGNOSTIC_POP #define MP_UNITS_API_NO_CRTP 1 +#endif + +#if defined(__clang__) && defined(__apple_build_version__) && __apple_build_version__ < 16000026 +#define MP_UNITS_APPLE_CLANG_HACKS #endif // NOLINTEND(bugprone-reserved-identifier, cppcoreguidelines-macro-usage) diff --git a/src/core/include/mp-units/framework/representation_concepts.h b/src/core/include/mp-units/framework/representation_concepts.h index ff99bd96..5ff83189 100644 --- a/src/core/include/mp-units/framework/representation_concepts.h +++ b/src/core/include/mp-units/framework/representation_concepts.h @@ -190,6 +190,7 @@ concept Complex = (!disable_complex) && requires(const T a, const T b, const requires std::constructible_from; } && WeaklyRegular; + namespace magnitude_impl { void magnitude() = delete; // poison pill @@ -309,15 +310,45 @@ concept VectorRepresentation = (!is_quantity) && Vector && requires(const MP_UNITS_EXPORT template concept Representation = detail::ScalarRepresentation || detail::ComplexRepresentation || - detail::VectorRepresentation; // || detail::TensorRepresentation; + detail::VectorRepresentation; // || detail::TensorRepresentation; */ namespace detail { +#ifdef MP_UNITS_APPLE_CLANG_HACKS +template +constexpr bool is_weakly_regular = std::copyable && std::equality_comparable; + +template +constexpr bool is_scalar = !disable_scalar && is_weakly_regular; + +template +constexpr bool is_complex = !disable_complex && is_weakly_regular && is_scalar> && + std::constructible_from, value_type_t>; + +template +concept ComplexFunctionsAvailable = requires(T a) { + ::mp_units::real(a); + ::mp_units::imag(a); + ::mp_units::modulus(a); +}; + +template +constexpr bool is_vector = !disable_vector && is_weakly_regular && is_scalar>; + +template +concept VectorFunctionsAvailable = requires(T a) { ::mp_units::magnitude(a); }; + + +template +concept IsOfCharacter = ((Ch == quantity_character::scalar && is_scalar) || + (Ch == quantity_character::complex && is_complex && ComplexFunctionsAvailable) || + (Ch == quantity_character::vector && is_vector && VectorFunctionsAvailable)); +#else template concept IsOfCharacter = (Ch == quantity_character::scalar && Scalar) || (Ch == quantity_character::complex && Complex) || (Ch == quantity_character::vector && Vector); // || (Ch == quantity_character::tensor && Tensor); - +#endif } MP_UNITS_EXPORT template diff --git a/test/static/concepts_test.cpp b/test/static/concepts_test.cpp index 0b1a7bab..2cf15448 100644 --- a/test/static/concepts_test.cpp +++ b/test/static/concepts_test.cpp @@ -20,6 +20,7 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +#include #include #include #include @@ -308,7 +309,9 @@ static_assert(!RepresentationOf, quantity_character::scalar static_assert(!RepresentationOf, quantity_character::vector>); static_assert(!RepresentationOf, quantity_character::tensor>); static_assert(RepresentationOf, quantity_character::vector>); +#ifndef MP_UNITS_APPLE_CLANG_HACKS static_assert(!RepresentationOf, quantity_character::scalar>); +#endif static_assert(!RepresentationOf, quantity_character::complex>); static_assert(!RepresentationOf, quantity_character::tensor>); static_assert(!RepresentationOf); diff --git a/test/static/quantity_test.cpp b/test/static/quantity_test.cpp index 3b8d14d5..40948a2d 100644 --- a/test/static/quantity_test.cpp +++ b/test/static/quantity_test.cpp @@ -65,6 +65,8 @@ static_assert(sizeof(quantity) == sizeof(double)); static_assert(sizeof(quantity) == sizeof(short)); static_assert(sizeof(quantity) == sizeof(short)); +#ifndef MP_UNITS_APPLE_CLANG_HACKS + template typename Q> concept invalid_types = requires { requires !requires { typename Q; }; // dimension instead of reference @@ -77,13 +79,15 @@ concept invalid_types = requires { }; // vector representation expected requires !requires { typename Q>; - }; // scalar representation expected + }; // scalar representation expected requires !requires { typename Q>; }; // incompatible character requires !requires { typename Q; }; // incompatible character #endif }; static_assert(invalid_types); +#endif + static_assert(std::is_trivially_default_constructible_v>); static_assert(std::is_trivially_copy_constructible_v>); static_assert(std::is_trivially_move_constructible_v>); From d30f5a668c20dc3ac3d784601be2cde8f5c1201d Mon Sep 17 00:00:00 2001 From: Roth Michaels Date: Mon, 9 Dec 2024 17:53:13 +0100 Subject: [PATCH 2/6] clang-format fixes --- src/core/include/mp-units/framework/representation_concepts.h | 2 +- test/static/quantity_test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/include/mp-units/framework/representation_concepts.h b/src/core/include/mp-units/framework/representation_concepts.h index 5ff83189..0a6950d9 100644 --- a/src/core/include/mp-units/framework/representation_concepts.h +++ b/src/core/include/mp-units/framework/representation_concepts.h @@ -349,7 +349,7 @@ concept IsOfCharacter = (Ch == quantity_character::scalar && Scalar) || (Ch == quantity_character::complex && Complex) || (Ch == quantity_character::vector && Vector); // || (Ch == quantity_character::tensor && Tensor); #endif -} +} // namespace detail MP_UNITS_EXPORT template concept RepresentationOf = diff --git a/test/static/quantity_test.cpp b/test/static/quantity_test.cpp index 40948a2d..418c66a9 100644 --- a/test/static/quantity_test.cpp +++ b/test/static/quantity_test.cpp @@ -79,7 +79,7 @@ concept invalid_types = requires { }; // vector representation expected requires !requires { typename Q>; - }; // scalar representation expected + }; // scalar representation expected requires !requires { typename Q>; }; // incompatible character requires !requires { typename Q; }; // incompatible character #endif From ec3f0000507be608d4fa8b59180f2b49f280142a Mon Sep 17 00:00:00 2001 From: Roth Michaels Date: Mon, 9 Dec 2024 18:10:56 +0100 Subject: [PATCH 3/6] Update src/core/include/mp-units/framework/representation_concepts.h Co-authored-by: Mateusz Pusz --- src/core/include/mp-units/framework/representation_concepts.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/include/mp-units/framework/representation_concepts.h b/src/core/include/mp-units/framework/representation_concepts.h index 0a6950d9..82f403ab 100644 --- a/src/core/include/mp-units/framework/representation_concepts.h +++ b/src/core/include/mp-units/framework/representation_concepts.h @@ -310,7 +310,7 @@ concept VectorRepresentation = (!is_quantity) && Vector && requires(const MP_UNITS_EXPORT template concept Representation = detail::ScalarRepresentation || detail::ComplexRepresentation || - detail::VectorRepresentation; // || detail::TensorRepresentation; */ + detail::VectorRepresentation; // || detail::TensorRepresentation; namespace detail { From 0c3bb98c6dbb281e0b4b18f1dee1c5c973c63fbd Mon Sep 17 00:00:00 2001 From: Roth Michaels Date: Mon, 9 Dec 2024 18:11:03 +0100 Subject: [PATCH 4/6] Update src/core/include/mp-units/framework/representation_concepts.h Co-authored-by: Mateusz Pusz --- src/core/include/mp-units/framework/representation_concepts.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/include/mp-units/framework/representation_concepts.h b/src/core/include/mp-units/framework/representation_concepts.h index 82f403ab..d556be51 100644 --- a/src/core/include/mp-units/framework/representation_concepts.h +++ b/src/core/include/mp-units/framework/representation_concepts.h @@ -190,7 +190,6 @@ concept Complex = (!disable_complex) && requires(const T a, const T b, const requires std::constructible_from; } && WeaklyRegular; - namespace magnitude_impl { void magnitude() = delete; // poison pill From ada508a683129d81ab977cf85802f860f1a683f4 Mon Sep 17 00:00:00 2001 From: Roth Michaels Date: Thu, 19 Dec 2024 10:40:15 -0500 Subject: [PATCH 5/6] Do not use WeaklyRegular with Xcode 15 for type detection Removing the `WeaklyRegular` requirement from the `Scalar`, `Complex`, and `Vector` concepts seems to address compiler crashes or long compilation times with apple-clang included in Xcode 15. I thought I had previously tried removing the `WeaklyRegular` requirement and it had not solved the problem but either I made a mistake with this test or some other changes after rebasing this work changed the situation. --- src/core/include/mp-units/bits/hacks.h | 2 +- .../framework/representation_concepts.h | 96 ++++++++----------- test/static/concepts_test.cpp | 3 - test/static/quantity_test.cpp | 4 - 4 files changed, 39 insertions(+), 66 deletions(-) diff --git a/src/core/include/mp-units/bits/hacks.h b/src/core/include/mp-units/bits/hacks.h index f27c5438..f9e98f24 100644 --- a/src/core/include/mp-units/bits/hacks.h +++ b/src/core/include/mp-units/bits/hacks.h @@ -153,6 +153,6 @@ MP_UNITS_DIAGNOSTIC_POP #endif #if defined(__clang__) && defined(__apple_build_version__) && __apple_build_version__ < 16000026 -#define MP_UNITS_APPLE_CLANG_HACKS +#define MP_UNITS_XCODE15_HACKS #endif // NOLINTEND(bugprone-reserved-identifier, cppcoreguidelines-macro-usage) diff --git a/src/core/include/mp-units/framework/representation_concepts.h b/src/core/include/mp-units/framework/representation_concepts.h index d556be51..37e5354a 100644 --- a/src/core/include/mp-units/framework/representation_concepts.h +++ b/src/core/include/mp-units/framework/representation_concepts.h @@ -88,7 +88,7 @@ concept Scalar = (!disable_scalar) && { a + b } -> std::common_with; { a - b } -> std::common_with; } && ScalableWith -#if MP_UNITS_COMP_GCC != 12 +#if MP_UNITS_COMP_GCC != 12 && !defined(MP_UNITS_XCODE15_HACKS) && WeaklyRegular #endif ; @@ -177,18 +177,23 @@ constexpr bool disable_complex = false; namespace detail { template -concept Complex = (!disable_complex) && requires(const T a, const T b, const T& c) { - { -a } -> std::common_with; - { a + b } -> std::common_with; - { a - b } -> std::common_with; - { a* b } -> std::common_with; - { a / b } -> std::common_with; - ::mp_units::real(a); - ::mp_units::imag(a); - ::mp_units::modulus(a); - requires ScalableWith; - requires std::constructible_from; -} && WeaklyRegular; +concept Complex = (!disable_complex) && + requires(const T a, const T b, const T& c) { + { -a } -> std::common_with; + { a + b } -> std::common_with; + { a - b } -> std::common_with; + { a* b } -> std::common_with; + { a / b } -> std::common_with; + ::mp_units::real(a); + ::mp_units::imag(a); + ::mp_units::modulus(a); + requires ScalableWith; + requires std::constructible_from; + } +#ifndef MP_UNITS_XCODE15_HACKS + && WeaklyRegular +#endif + ; namespace magnitude_impl { @@ -238,19 +243,24 @@ constexpr bool disable_vector = false; namespace detail { template -concept Vector = (!disable_vector) && requires(const T a, const T b) { - { -a } -> std::common_with; - { a + b } -> std::common_with; - { a - b } -> std::common_with; - ::mp_units::magnitude(a); - requires ScalableWith; - // TODO should we also check for the below (e.g., when `size() > 1` or `2`) - // ::mp_units::zero_vector(); - // ::mp_units::unit_vector(a); - // ::mp_units::scalar_product(a, b); - // ::mp_units::vector_product(a, b); - // ::mp_units::tensor_product(a, b); -} && WeaklyRegular; +concept Vector = (!disable_vector) && + requires(const T a, const T b) { + { -a } -> std::common_with; + { a + b } -> std::common_with; + { a - b } -> std::common_with; + ::mp_units::magnitude(a); + requires ScalableWith; + // TODO should we also check for the below (e.g., when `size() > 1` or `2`) + // ::mp_units::zero_vector(); + // ::mp_units::unit_vector(a); + // ::mp_units::scalar_product(a, b); + // ::mp_units::vector_product(a, b); + // ::mp_units::tensor_product(a, b); + } +#ifndef MP_UNITS_XCODE15_HACKS + && WeaklyRegular +#endif + ; } // namespace detail @@ -313,42 +323,12 @@ concept Representation = detail::ScalarRepresentation || detail::ComplexRepre namespace detail { -#ifdef MP_UNITS_APPLE_CLANG_HACKS -template -constexpr bool is_weakly_regular = std::copyable && std::equality_comparable; - -template -constexpr bool is_scalar = !disable_scalar && is_weakly_regular; - -template -constexpr bool is_complex = !disable_complex && is_weakly_regular && is_scalar> && - std::constructible_from, value_type_t>; - -template -concept ComplexFunctionsAvailable = requires(T a) { - ::mp_units::real(a); - ::mp_units::imag(a); - ::mp_units::modulus(a); -}; - -template -constexpr bool is_vector = !disable_vector && is_weakly_regular && is_scalar>; - -template -concept VectorFunctionsAvailable = requires(T a) { ::mp_units::magnitude(a); }; - - -template -concept IsOfCharacter = ((Ch == quantity_character::scalar && is_scalar) || - (Ch == quantity_character::complex && is_complex && ComplexFunctionsAvailable) || - (Ch == quantity_character::vector && is_vector && VectorFunctionsAvailable)); -#else template concept IsOfCharacter = (Ch == quantity_character::scalar && Scalar) || (Ch == quantity_character::complex && Complex) || (Ch == quantity_character::vector && Vector); // || (Ch == quantity_character::tensor && Tensor); -#endif -} // namespace detail + +} MP_UNITS_EXPORT template concept RepresentationOf = diff --git a/test/static/concepts_test.cpp b/test/static/concepts_test.cpp index 2cf15448..0b1a7bab 100644 --- a/test/static/concepts_test.cpp +++ b/test/static/concepts_test.cpp @@ -20,7 +20,6 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -#include #include #include #include @@ -309,9 +308,7 @@ static_assert(!RepresentationOf, quantity_character::scalar static_assert(!RepresentationOf, quantity_character::vector>); static_assert(!RepresentationOf, quantity_character::tensor>); static_assert(RepresentationOf, quantity_character::vector>); -#ifndef MP_UNITS_APPLE_CLANG_HACKS static_assert(!RepresentationOf, quantity_character::scalar>); -#endif static_assert(!RepresentationOf, quantity_character::complex>); static_assert(!RepresentationOf, quantity_character::tensor>); static_assert(!RepresentationOf); diff --git a/test/static/quantity_test.cpp b/test/static/quantity_test.cpp index 418c66a9..3b8d14d5 100644 --- a/test/static/quantity_test.cpp +++ b/test/static/quantity_test.cpp @@ -65,8 +65,6 @@ static_assert(sizeof(quantity) == sizeof(double)); static_assert(sizeof(quantity) == sizeof(short)); static_assert(sizeof(quantity) == sizeof(short)); -#ifndef MP_UNITS_APPLE_CLANG_HACKS - template typename Q> concept invalid_types = requires { requires !requires { typename Q; }; // dimension instead of reference @@ -86,8 +84,6 @@ concept invalid_types = requires { }; static_assert(invalid_types); -#endif - static_assert(std::is_trivially_default_constructible_v>); static_assert(std::is_trivially_copy_constructible_v>); static_assert(std::is_trivially_move_constructible_v>); From 90aa52a6ccd14dc8b30acdd82ec6285b123ae278 Mon Sep 17 00:00:00 2001 From: Roth Michaels Date: Thu, 19 Dec 2024 11:21:17 -0500 Subject: [PATCH 6/6] Update src/core/include/mp-units/framework/representation_concepts.h Co-authored-by: Mateusz Pusz --- src/core/include/mp-units/framework/representation_concepts.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/include/mp-units/framework/representation_concepts.h b/src/core/include/mp-units/framework/representation_concepts.h index 37e5354a..58b806eb 100644 --- a/src/core/include/mp-units/framework/representation_concepts.h +++ b/src/core/include/mp-units/framework/representation_concepts.h @@ -194,7 +194,6 @@ concept Complex = (!disable_complex) && && WeaklyRegular #endif ; - namespace magnitude_impl { void magnitude() = delete; // poison pill