refactor: 💥 from now unit_symbol and dimension_symbol always retu…

…rns `std::string_view`
mpusz · Nov 16, 2024 · 7b64b4b · 7b64b4b · JohelEGP · Dec 29, 2024
1 parent 2c24c61
commit 7b64b4b
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 113 deletions.
diff --git 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
@@ -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<CharT, N>` 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
@@ -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<CharT, N>` 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<CharT, N>` 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
@@ -62,21 +62,6 @@ inline constexpr auto JPY = japanese_jen;
 
 static_assert(!std::equality_comparable_with<quantity<euro, int>, quantity<us_dollar, int>>);
 
-
-#if 0 || !MP_UNITS_API_STRING_VIEW_RET  // NOLINT(readability-avoid-unconditional-preprocessor-if)
-
-// if you have only a few currencies to handle
-template<Unit auto From, Unit auto To>
-[[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<Unit auto From, Unit auto To>
 [[nodiscard]] double exchange_rate(std::chrono::sys_seconds timestamp)
 {
@@ -89,8 +74,6 @@ template<Unit auto From, Unit auto To>
   return rates.at(std::make_pair(unit_symbol(From), unit_symbol(To)));
 }
 
-#endif
-
 template<UnitOf<currency> auto To, QuantityOf<currency> From>
 QuantityOf<currency> auto exchange_to(From q, std::chrono::sys_seconds timestamp)
 {

diff --git 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,17 +72,13 @@ 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}
 )
 option(${projectPrefix}API_FREESTANDING "Builds only freestanding part of the library" OFF)
 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
@@ -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
@@ -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
@@ -286,29 +286,33 @@ constexpr Out dimension_symbol_to(Out out, D d, const dimension_symbol_formattin
   return detail::dimension_symbol_impl<CharT>(out, d, fmt, false);
 }
 
-// TODO Refactor to `dimension_symbol(D, fmt)` when P1045: constexpr Function Parameters is available
-MP_UNITS_EXPORT template<dimension_symbol_formatting fmt = dimension_symbol_formatting{}, typename CharT = char,
-                         Dimension D>
-#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<dimension_symbol_formatting fmt, typename CharT, Dimension D>
+[[nodiscard]] consteval auto dimension_symbol_impl(D)
 {
   constexpr auto oversized_symbol_text = []() consteval {
     // std::basic_string<CharT> text;  // TODO uncomment when https://wg21.link/P3032 is supported
     detail::inplace_vector<CharT, 128> text;
     dimension_symbol_to<CharT>(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<CharT, oversized_symbol_text.size()> storage(std::from_range,
-                                                                                   oversized_symbol_text);
-  return storage.view();
-#else
   return basic_fixed_string<CharT, oversized_symbol_text.size()>(std::from_range, oversized_symbol_text);
-#endif
+}
+
+template<dimension_symbol_formatting fmt, typename CharT, Dimension D>
+struct dimension_symbol_result {
+  static constexpr auto value = dimension_symbol_impl<fmt, CharT>(D{});
+};
+
+}  // namespace detail
+
+// TODO Refactor to `dimension_symbol(D, fmt)` when P1045: constexpr Function Parameters is available
+MP_UNITS_EXPORT template<dimension_symbol_formatting fmt = dimension_symbol_formatting{}, typename CharT = char,
+                         Dimension D>
+[[nodiscard]] consteval std::string_view dimension_symbol(D)
+{
+  return detail::dimension_symbol_result<fmt, CharT, D>::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
@@ -889,28 +889,32 @@ constexpr Out unit_symbol_to(Out out, U u, const unit_symbol_formatting& fmt = u
   return detail::unit_symbol_impl<CharT>(out, u, fmt, false);
 }
 
-// TODO Refactor to `unit_symbol(U, fmt)` when P1045: constexpr Function Parameters is available
-MP_UNITS_EXPORT template<unit_symbol_formatting fmt = unit_symbol_formatting{}, typename CharT = char, Unit U>
-#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<unit_symbol_formatting fmt, typename CharT, Unit U>
+[[nodiscard]] consteval auto unit_symbol_impl(U)
 {
   constexpr auto oversized_symbol_text = []() consteval {
     // std::basic_string<CharT> text;  // TODO uncomment when https://wg21.link/P3032 is supported
     detail::inplace_vector<CharT, 128> text;
     unit_symbol_to<CharT>(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<CharT, oversized_symbol_text.size()> storage(std::from_range,
-                                                                                   oversized_symbol_text);
-  return storage.view();
-#else
   return basic_fixed_string<CharT, oversized_symbol_text.size()>(std::from_range, oversized_symbol_text);
-#endif
+}
+
+template<unit_symbol_formatting fmt, typename CharT, Unit U>
+struct unit_symbol_result {
+  static constexpr auto value = unit_symbol_impl<fmt, CharT>(U{});
+};
+
+}  // namespace detail
+
+// TODO Refactor to `unit_symbol(U, fmt)` when P1045: constexpr Function Parameters is available
+MP_UNITS_EXPORT template<unit_symbol_formatting fmt = unit_symbol_formatting{}, typename CharT = char, Unit U>
+[[nodiscard]] consteval std::string_view unit_symbol(U)
+{
+  return detail::unit_symbol_result<fmt, CharT, U>::value.view();
 }
 
 }  // namespace mp_units