From 7b64b4b65051dc85072f72ec292248df7157d235 Mon Sep 17 00:00:00 2001 From: Mateusz Pusz Date: Sat, 16 Nov 2024 09:57:53 +0100 Subject: [PATCH] refactor: :boom: from now `unit_symbol` and `dimension_symbol` always returns `std::string_view` --- conanfile.py | 12 ------- docs/getting_started/cpp_compiler_support.md | 15 -------- .../getting_started/installation_and_usage.md | 24 ------------- example/currency.cpp | 17 ---------- src/CMakeLists.txt | 9 ----- src/core/CMakeLists.txt | 1 - src/core/include/mp-units/bits/hacks.h | 6 ---- .../include/mp-units/framework/dimension.h | 34 +++++++++++-------- src/core/include/mp-units/framework/unit.h | 32 +++++++++-------- 9 files changed, 37 insertions(+), 113 deletions(-) diff --git a/conanfile.py b/conanfile.py index c34071137..7b9c1853b 100644 --- a/conanfile.py +++ b/conanfile.py @@ -58,7 +58,6 @@ class MPUnitsConan(ConanFile): "cxx_modules": [True, False], "import_std": [True, False], "std_format": [True, False], - "string_view_ret": [True, False], "no_crtp": [True, False], "contracts": ["none", "gsl-lite", "ms-gsl"], "freestanding": [True, False], @@ -67,7 +66,6 @@ class MPUnitsConan(ConanFile): # "cxx_modules" default set in config_options() # "import_std" default set in config_options() # "std_format" default set in config_options() - # "string_view_ret" default set in config_options() # "no_crtp" default set in config_options() "contracts": "gsl-lite", "freestanding": False, @@ -113,10 +111,6 @@ def _feature_compatibility(self): "min_cppstd": "23", "compiler": {"gcc": "", "clang": "18", "apple-clang": "", "msvc": ""}, }, - "static_constexpr_vars_in_constexpr_func": { - "min_cppstd": "23", - "compiler": {"gcc": "13", "clang": "17", "apple-clang": "", "msvc": ""}, - }, "explicit_this": { "min_cppstd": "23", "compiler": { @@ -134,7 +128,6 @@ def _option_feature_map(self): "std_format": "std_format", "cxx_modules": "cxx_modules", "import_std": "import_std", - "string_view_ret": "static_constexpr_vars_in_constexpr_func", "no_crtp": "explicit_this", } @@ -280,7 +273,6 @@ def generate(self): tc.cache_variables["MP_UNITS_API_FREESTANDING"] = True else: tc.cache_variables["MP_UNITS_API_STD_FORMAT"] = opt.std_format - tc.cache_variables["MP_UNITS_API_STRING_VIEW_RET"] = opt.string_view_ret tc.cache_variables["MP_UNITS_API_NO_CRTP"] = opt.no_crtp tc.cache_variables["MP_UNITS_API_CONTRACTS"] = str(opt.contracts).upper() @@ -340,10 +332,6 @@ def package_info(self): ) # handle API options - self.cpp_info.components["core"].defines.append( - "MP_UNITS_API_STRING_VIEW_RET=" - + str(int(self.options.string_view_ret == True)) - ) self.cpp_info.components["core"].defines.append( "MP_UNITS_API_NO_CRTP=" + str(int(self.options.no_crtp == True)) ) diff --git a/docs/getting_started/cpp_compiler_support.md b/docs/getting_started/cpp_compiler_support.md index d5eb5a348..525585c30 100644 --- a/docs/getting_started/cpp_compiler_support.md +++ b/docs/getting_started/cpp_compiler_support.md @@ -19,7 +19,6 @@ C++ feature: | **`std::format`** | 20 | 13 | 17 | None | 194 | | **C++ modules** | 20 | None | 17 | None | None | | **`import std;`** | 23 | None | 18 | None | None | -| **Static `constexpr` variables in `constexpr` functions** | 23 | 13 | 17 | None | None | | **Explicit `this` parameter** | 23 | 14 | 18 | None | None | ??? note "MSVC bugs" @@ -87,20 +86,6 @@ C++ feature: - CMake: [CMAKE_CXX_MODULE_STD](https://cmake.org/cmake/help/latest/variable/CMAKE_CXX_MODULE_STD.html) -## Static `constexpr` variables in `constexpr` functions - -- Allows returning `std::string_view` from the - [`unit_symbol()`](../users_guide/framework_basics/text_output.md#unit_symbol) - and [`dimension_symbol()`](../users_guide/framework_basics/text_output.md#dimension_symbol) - functions - - `std::string_view` type has a reference semantics so it has to point to a storage with - a longer lifetime. -- If this feature is not available, the API returns `mp_units::basic_fixed_string` instead. -- Tested as `__cpp_constexpr >= 202211L` [feature test macro](https://en.cppreference.com/w/cpp/feature_test). -- Related build options: - - Conan: [string_view_ret](installation_and_usage.md#string_view_ret) - - CMake: [MP_UNITS_API_STRING_VIEW_RET](installation_and_usage.md#MP_UNITS_API_STRING_VIEW_RET) - ## Explicit `this` parameter - This feature removes the need for the usage of the CRTP idiom in the diff --git a/docs/getting_started/installation_and_usage.md b/docs/getting_started/installation_and_usage.md index 79de4a58c..8fb6115ba 100644 --- a/docs/getting_started/installation_and_usage.md +++ b/docs/getting_started/installation_and_usage.md @@ -123,18 +123,6 @@ dependencies by other means, some modifications to the library's CMake files mig [conan std::format support]: https://github.com/mpusz/mp-units/releases/tag/v2.2.0 -[`string_view_ret`](#string_view_ret){ #string_view_ret } - -: [:octicons-tag-24: 2.2.0][conan returning string_view] · :octicons-milestone-24: `True`/`False` (Default: automatically determined from settings) - - Enables returning `std::string_view` from the - [`unit_symbol()`](../users_guide/framework_basics/text_output.md#unit_symbol) - and [`dimension_symbol()`](../users_guide/framework_basics/text_output.md#dimension_symbol) - functions. If this feature is not available, those functions will return - `mp_units::basic_fixed_string` instead. - - [conan returning string_view]: https://github.com/mpusz/mp-units/releases/tag/v2.2.0 - [`no_crtp`](#no_crtp){ #no_crtp } : [:octicons-tag-24: 2.2.0][conan no crtp support] · :octicons-milestone-24: `True`/`False` (Default: automatically determined from settings) @@ -196,18 +184,6 @@ dependencies by other means, some modifications to the library's CMake files mig [cmake std::format support]: https://github.com/mpusz/mp-units/releases/tag/v2.2.0 - [`MP_UNITS_API_STRING_VIEW_RET`](#MP_UNITS_API_STRING_VIEW_RET){ #MP_UNITS_API_STRING_VIEW_RET } - - : [:octicons-tag-24: 2.2.0][cmake returning string_view] · :octicons-milestone-24: `ON`/`OFF` (Default: automatically determined) - - Enables returning `std::string_view` from the - [`unit_symbol()`](../users_guide/framework_basics/text_output.md#unit_symbol) - and [`dimension_symbol()`](../users_guide/framework_basics/text_output.md#dimension_symbol) - functions. If this feature is not available, those functions will return - `mp_units::basic_fixed_string` instead. - - [cmake returning string_view]: https://github.com/mpusz/mp-units/releases/tag/v2.2.0 - [`MP_UNITS_API_NO_CRTP`](#MP_UNITS_API_NO_CRTP){ #MP_UNITS_API_NO_CRTP } : [:octicons-tag-24: 2.2.0][cmake no crtp support] · :octicons-milestone-24: `ON`/`OFF` (Default: automatically determined) diff --git a/example/currency.cpp b/example/currency.cpp index 43aa28fd6..bedf7c05e 100644 --- a/example/currency.cpp +++ b/example/currency.cpp @@ -62,21 +62,6 @@ inline constexpr auto JPY = japanese_jen; static_assert(!std::equality_comparable_with, quantity>); - -#if 0 || !MP_UNITS_API_STRING_VIEW_RET // NOLINT(readability-avoid-unconditional-preprocessor-if) - -// if you have only a few currencies to handle -template -[[nodiscard]] double exchange_rate(std::chrono::sys_seconds timestamp) -{ - (void)timestamp; // get conversion ratios for this timestamp - if constexpr (From == us_dollar && To == euro) return 0.9215; - else if constexpr (From == euro && To == us_dollar) return 1.0848; - // ... -} - -#else - template [[nodiscard]] double exchange_rate(std::chrono::sys_seconds timestamp) { @@ -89,8 +74,6 @@ template return rates.at(std::make_pair(unit_symbol(From), unit_symbol(To))); } -#endif - template auto To, QuantityOf From> QuantityOf auto exchange_to(From q, std::chrono::sys_seconds timestamp) { diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8b19f595b..ca5b7ed1f 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -48,7 +48,6 @@ endif() # check for C++ features check_cxx_feature_supported(__cpp_lib_format ${projectPrefix}LIB_FORMAT_SUPPORTED) -check_cxx_feature_supported("__cpp_constexpr >= 202211L" ${projectPrefix}STATIC_CONSTEXPR_VARS_IN_CONSTEXPR_FUNCTIONS) check_cxx_feature_supported(__cpp_explicit_this_parameter ${projectPrefix}EXPLICIT_THIS_PARAMETER_SUPPORTED) # libc++ has a basic supports for std::format but does not set __cpp_lib_format @@ -73,9 +72,6 @@ endif() # project API settings option(${projectPrefix}API_STD_FORMAT "Enable `std::format` support" ${${projectPrefix}LIB_FORMAT_SUPPORTED}) -option(${projectPrefix}API_STRING_VIEW_RET "Enable returning `std::string_view` from `constexpr` functions" - ${${projectPrefix}STATIC_CONSTEXPR_VARS_IN_CONSTEXPR_FUNCTIONS} -) option(${projectPrefix}API_NO_CRTP "Enable class definitions without CRTP idiom" ${${projectPrefix}EXPLICIT_THIS_PARAMETER_SUPPORTED} ) @@ -83,7 +79,6 @@ option(${projectPrefix}API_FREESTANDING "Builds only freestanding part of the li set(${projectPrefix}API_CONTRACTS GSL-LITE CACHE STRING "Enable contract checking") message(STATUS "${projectPrefix}API_STD_FORMAT: ${${projectPrefix}API_STD_FORMAT}") -message(STATUS "${projectPrefix}API_STRING_VIEW_RET: ${${projectPrefix}API_STRING_VIEW_RET}") message(STATUS "${projectPrefix}API_NO_CRTP: ${${projectPrefix}API_NO_CRTP}") message(STATUS "${projectPrefix}API_FREESTANDING: ${${projectPrefix}API_FREESTANDING}") @@ -107,10 +102,6 @@ if(NOT ${projectPrefix}API_FREESTANDING AND ${projectPrefix}API_STD_FORMAT AND N message(FATAL_ERROR "`std::format` enabled but not supported") endif() -if(${projectPrefix}API_STRING_VIEW_RET AND NOT ${projectPrefix}STATIC_CONSTEXPR_VARS_IN_CONSTEXPR_FUNCTIONS) - message(FATAL_ERROR "`std::string_view` returns enabled but not supported") -endif() - if(${projectPrefix}API_NO_CRTP AND NOT ${projectPrefix}EXPLICIT_THIS_PARAMETER_SUPPORTED) message(FATAL_ERROR "`NO_CRTP` mode enabled but explicit `this` parameter is not supported") endif() diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index e6251cf22..1f3dc449d 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -101,7 +101,6 @@ if(NOT ${projectPrefix}API_FREESTANDING) endif() set_feature_flag(API_STD_FORMAT) -set_feature_flag(API_STRING_VIEW_RET) set_feature_flag(API_NO_CRTP) # Text formatting diff --git a/src/core/include/mp-units/bits/hacks.h b/src/core/include/mp-units/bits/hacks.h index c52c9abc7..70ee2c141 100644 --- a/src/core/include/mp-units/bits/hacks.h +++ b/src/core/include/mp-units/bits/hacks.h @@ -146,12 +146,6 @@ MP_UNITS_DIAGNOSTIC_POP #endif -#if !defined MP_UNITS_API_STRING_VIEW_RET && __cpp_constexpr >= 202211L - -#define MP_UNITS_API_STRING_VIEW_RET 1 - -#endif - #if !defined MP_UNITS_API_NO_CRTP && __cpp_explicit_this_parameter #define MP_UNITS_API_NO_CRTP 1 diff --git a/src/core/include/mp-units/framework/dimension.h b/src/core/include/mp-units/framework/dimension.h index 9d45a4961..73223ccbf 100644 --- a/src/core/include/mp-units/framework/dimension.h +++ b/src/core/include/mp-units/framework/dimension.h @@ -286,14 +286,10 @@ constexpr Out dimension_symbol_to(Out out, D d, const dimension_symbol_formattin return detail::dimension_symbol_impl(out, d, fmt, false); } -// TODO Refactor to `dimension_symbol(D, fmt)` when P1045: constexpr Function Parameters is available -MP_UNITS_EXPORT template -#if defined MP_UNITS_COMP_CLANG && MP_UNITS_COMP_CLANG <= 18 -[[nodiscard]] constexpr auto dimension_symbol(D) -#else -[[nodiscard]] consteval auto dimension_symbol(D) -#endif +namespace detail { + +MP_UNITS_EXPORT template +[[nodiscard]] consteval auto dimension_symbol_impl(D) { constexpr auto oversized_symbol_text = []() consteval { // std::basic_string text; // TODO uncomment when https://wg21.link/P3032 is supported @@ -301,14 +297,22 @@ MP_UNITS_EXPORT template(std::back_inserter(text), D{}, fmt); return text; }(); - -#if MP_UNITS_API_STRING_VIEW_RET // Permitting static constexpr variables in constexpr functions - static constexpr basic_fixed_string storage(std::from_range, - oversized_symbol_text); - return storage.view(); -#else return basic_fixed_string(std::from_range, oversized_symbol_text); -#endif +} + +template +struct dimension_symbol_result { + static constexpr auto value = dimension_symbol_impl(D{}); +}; + +} // namespace detail + +// TODO Refactor to `dimension_symbol(D, fmt)` when P1045: constexpr Function Parameters is available +MP_UNITS_EXPORT template +[[nodiscard]] consteval std::string_view dimension_symbol(D) +{ + return detail::dimension_symbol_result::value.view(); } } // namespace mp_units diff --git a/src/core/include/mp-units/framework/unit.h b/src/core/include/mp-units/framework/unit.h index 962f1acc3..fa5edf14a 100644 --- a/src/core/include/mp-units/framework/unit.h +++ b/src/core/include/mp-units/framework/unit.h @@ -889,13 +889,10 @@ constexpr Out unit_symbol_to(Out out, U u, const unit_symbol_formatting& fmt = u return detail::unit_symbol_impl(out, u, fmt, false); } -// TODO Refactor to `unit_symbol(U, fmt)` when P1045: constexpr Function Parameters is available -MP_UNITS_EXPORT template -#if defined MP_UNITS_COMP_CLANG && MP_UNITS_COMP_CLANG <= 18 -[[nodiscard]] constexpr auto unit_symbol(U) -#else -[[nodiscard]] consteval auto unit_symbol(U) -#endif +namespace detail { + +MP_UNITS_EXPORT template +[[nodiscard]] consteval auto unit_symbol_impl(U) { constexpr auto oversized_symbol_text = []() consteval { // std::basic_string text; // TODO uncomment when https://wg21.link/P3032 is supported @@ -903,14 +900,21 @@ MP_UNITS_EXPORT template(std::back_inserter(text), U{}, fmt); return text; }(); - -#if MP_UNITS_API_STRING_VIEW_RET // Permitting static constexpr variables in constexpr functions - static constexpr basic_fixed_string storage(std::from_range, - oversized_symbol_text); - return storage.view(); -#else return basic_fixed_string(std::from_range, oversized_symbol_text); -#endif +} + +template +struct unit_symbol_result { + static constexpr auto value = unit_symbol_impl(U{}); +}; + +} // namespace detail + +// TODO Refactor to `unit_symbol(U, fmt)` when P1045: constexpr Function Parameters is available +MP_UNITS_EXPORT template +[[nodiscard]] consteval std::string_view unit_symbol(U) +{ + return detail::unit_symbol_result::value.view(); } } // namespace mp_units