[libcxx-commits] [libcxx] [libc++] Replace `std::optional` in `__format/format_context.h` with a re-implementation (PR #148876)
via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Jul 15 08:57:04 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: William Tran-Viet (smallp-o-p)
<details>
<summary>Changes</summary>
- Replaces the use of `std::optional` in `__format/format_context.h` with a simple re-implementation that caters to `std::locale` only.
### Why?
While working on #<!-- -->105430 I ran into an issue implementing [[optional.syn]](https://eel.is/c++draft/optional.syn) because of a circular `#include <optional>` in `__format/format_context.h`, since I need `__format/range_default_formatter.h` inside `optional`. I originally implemented this in the `<optional> changes, but decided to split it up to make it easier to review.
### Other approaches considered
The first solution was to simply drop `std::optional` altogether, but it resulted in a pretty significant performance drop (numbers to come...) when compared to the 'lazy-load' implementation when running the `format.bench.pass.cpp` benchmark.
### Performance differences
>From the benchmark runs I've done, the difference is minimal if not very slightly worse(?)
I've attached an example benchmark run of `format.bench.pass.cpp`, `fmt_baseline` is how it's currently done and `fmt_candidate` are my changes. I will have to do a few more runs to get a better picture.
[fmt_bench_diff.txt](https://github.com/user-attachments/files/21236989/fmt_bench_diff.txt)
[fmt_candidate.txt](https://github.com/user-attachments/files/21236990/fmt_candidate.txt)
[fmt_baseline.txt](https://github.com/user-attachments/files/21236991/fmt_baseline.txt)
---
Full diff: https://github.com/llvm/llvm-project/pull/148876.diff
1 Files Affected:
- (modified) libcxx/include/__format/format_context.h (+60-13)
``````````diff
diff --git a/libcxx/include/__format/format_context.h b/libcxx/include/__format/format_context.h
index e672ee7ad0581..59bd18f38bf12 100644
--- a/libcxx/include/__format/format_context.h
+++ b/libcxx/include/__format/format_context.h
@@ -21,12 +21,12 @@
#include <__iterator/back_insert_iterator.h>
#include <__iterator/concepts.h>
#include <__memory/addressof.h>
+#include <__memory/construct_at.h>
#include <__utility/move.h>
#include <__variant/monostate.h>
#if _LIBCPP_HAS_LOCALIZATION
# include <__locale>
-# include <optional>
#endif
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -45,6 +45,57 @@ template <class _OutIt, class _CharT>
class basic_format_context;
# if _LIBCPP_HAS_LOCALIZATION
+
+/*
+ * Off-brand std::optional<locale> to avoid having to #include <optional>. This is necessary because <optional> needs
+ * range_format for P3168R2, which would cause an include cycle.
+ */
+
+class __optional_locale {
+private:
+ union {
+ std::locale __loc_;
+ char __null_state_ = '\0';
+ };
+ bool __has_value_ = false;
+
+public:
+ _LIBCPP_HIDE_FROM_ABI __optional_locale() noexcept {}
+ _LIBCPP_HIDE_FROM_ABI __optional_locale(const std::locale& __loc) noexcept : __loc_(__loc), __has_value_(true) {}
+ _LIBCPP_HIDE_FROM_ABI __optional_locale(std::locale&& __loc) noexcept
+ : __loc_(std::move(__loc)), __has_value_(true) {}
+ _LIBCPP_HIDE_FROM_ABI __optional_locale(__optional_locale&& __other_loc) noexcept
+ : __has_value_(__other_loc.__has_value_) {
+ if (__other_loc.__has_value_) {
+ std::__construct_at(&__loc_, std::move(__other_loc.__loc_));
+ }
+ }
+
+ _LIBCPP_HIDE_FROM_ABI ~__optional_locale() {
+ if (__has_value_) {
+ __loc_.~locale();
+ }
+ }
+
+ _LIBCPP_HIDE_FROM_ABI __optional_locale& operator=(std::locale&& __loc) noexcept {
+ if (__has_value_) {
+ __loc_ = std::move(__loc);
+ } else {
+ std::__construct_at(&__loc_, std::move(__loc));
+ __has_value_ = true;
+ }
+ return *this;
+ }
+
+ _LIBCPP_HIDE_FROM_ABI std::locale& __value() noexcept {
+ if (!__has_value_) {
+ __has_value_ = true;
+ std::__construct_at(&__loc_);
+ }
+ return __loc_;
+ }
+};
+
/**
* Helper to create a basic_format_context.
*
@@ -54,7 +105,7 @@ template <class _OutIt, class _CharT>
_LIBCPP_HIDE_FROM_ABI basic_format_context<_OutIt, _CharT>
__format_context_create(_OutIt __out_it,
basic_format_args<basic_format_context<_OutIt, _CharT>> __args,
- optional<std::locale>&& __loc = nullopt) {
+ __optional_locale&& __loc = __optional_locale()) {
return std::basic_format_context(std::move(__out_it), __args, std::move(__loc));
}
# else
@@ -72,8 +123,8 @@ using wformat_context = basic_format_context< back_insert_iterator<__format::__o
template <class _OutIt, class _CharT>
requires output_iterator<_OutIt, const _CharT&>
-class _LIBCPP_PREFERRED_NAME(format_context)
- _LIBCPP_IF_WIDE_CHARACTERS(_LIBCPP_PREFERRED_NAME(wformat_context)) basic_format_context {
+class _LIBCPP_PREFERRED_NAME(format_context) _LIBCPP_IF_WIDE_CHARACTERS(_LIBCPP_PREFERRED_NAME(wformat_context))
+ basic_format_context {
public:
using iterator = _OutIt;
using char_type = _CharT;
@@ -84,11 +135,7 @@ class _LIBCPP_PREFERRED_NAME(format_context)
return __args_.get(__id);
}
# if _LIBCPP_HAS_LOCALIZATION
- _LIBCPP_HIDE_FROM_ABI std::locale locale() {
- if (!__loc_)
- __loc_ = std::locale{};
- return *__loc_;
- }
+ _LIBCPP_HIDE_FROM_ABI std::locale locale() { return __loc_.__value(); }
# endif
_LIBCPP_HIDE_FROM_ABI iterator out() { return std::move(__out_it_); }
_LIBCPP_HIDE_FROM_ABI void advance_to(iterator __it) { __out_it_ = std::move(__it); }
@@ -106,16 +153,16 @@ class _LIBCPP_PREFERRED_NAME(format_context)
// This is done by storing the locale of the constructor in this optional. If
// locale() is called and the optional has no value the value will be created.
// This allows the implementation to lazily create the locale.
- // TODO FMT Validate whether lazy creation is the best solution.
- optional<std::locale> __loc_;
+
+ __optional_locale __loc_;
template <class _OtherOutIt, class _OtherCharT>
friend _LIBCPP_HIDE_FROM_ABI basic_format_context<_OtherOutIt, _OtherCharT> __format_context_create(
- _OtherOutIt, basic_format_args<basic_format_context<_OtherOutIt, _OtherCharT>>, optional<std::locale>&&);
+ _OtherOutIt, basic_format_args<basic_format_context<_OtherOutIt, _OtherCharT>>, __optional_locale&&);
// Note: the Standard doesn't specify the required constructors.
_LIBCPP_HIDE_FROM_ABI explicit basic_format_context(
- _OutIt __out_it, basic_format_args<basic_format_context> __args, optional<std::locale>&& __loc)
+ _OutIt __out_it, basic_format_args<basic_format_context> __args, __optional_locale&& __loc)
: __out_it_(std::move(__out_it)), __args_(__args), __loc_(std::move(__loc)) {}
# else
template <class _OtherOutIt, class _OtherCharT>
``````````
</details>
https://github.com/llvm/llvm-project/pull/148876
More information about the libcxx-commits
mailing list