Skip to content

Commit 720d4a7

Browse files
ednolanclaude
andcommitted
Specify code unit adaptor conversion rule in terms of width, not sizeof
Per SG16 feedback, compare the width of the source and target types ([basic.fundamental]: value bits plus sign bit, excluding padding) rather than sizeof, so that padding bits cannot hide a narrowing conversion. The implementation computes width from numeric_limits digits and is_signed, using unsigned char for std::byte. Update the wording to define width for integer types, character types, and byte via the underlying type. Replace the design section's single conversion table with two: one covering all integral type sizes the standard permits (assuming CHAR_BIT == 8), and one covering typical x86_64/aarch64 sizes, using "same width" wherever LP64 and LLP64 targets differ. Also size the exposition-only transcoding buffer as 4, 2, or 1 elements for char8_t, char16_t, or char32_t respectively, rather than 4 / sizeof(ToType), which under-sizes the buffer on an implementation whose char16_t has padding bits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 891c618 commit 720d4a7

4 files changed

Lines changed: 83 additions & 33 deletions

File tree

‎include/beman/utf_view/code_unit_view.hpp‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import beman.utf_view;
2222
#include <beman/transform_view/transform_view.hpp>
2323
#include <concepts>
2424
#include <cstddef>
25+
#include <limits>
2526
#include <ranges>
2627
#include <type_traits>
2728
#endif
@@ -47,18 +48,32 @@ struct exposition_only_cast_to {
4748

4849
namespace detail {
4950

51+
// The width of an integral type is the number of bits in its value
52+
// representation ([basic.fundamental]), i.e. value bits plus the sign bit
53+
// and excluding padding bits. For std::byte it is the width of its
54+
// underlying type, unsigned char.
55+
template <class T>
56+
using code_unit_width_type =
57+
std::conditional_t<std::same_as<T, std::byte>, unsigned char, T>;
58+
59+
template <class T>
60+
constexpr int code_unit_width =
61+
std::numeric_limits<code_unit_width_type<T>>::digits +
62+
std::numeric_limits<code_unit_width_type<T>>::is_signed;
63+
5064
// A source type U may be cast to the code unit type Char if it is Char
5165
// itself, or if it is std::byte or a non-bool integral type of the same
52-
// width as Char. This rules out narrowing, widening (e.g. accidentally
53-
// treating UTF-8 code units as code points), floating-point sources, bool,
54-
// enumerations other than std::byte, and class types such as proxy
55-
// references.
66+
// width as Char. Width rather than sizeof is compared so that padding bits
67+
// cannot hide a narrowing conversion. This rules out narrowing, widening
68+
// (e.g. accidentally treating UTF-8 code units as code points),
69+
// floating-point sources, bool, enumerations other than std::byte, and class
70+
// types such as proxy references.
5671
template <class U, class Char>
5772
concept code_unit_castable =
5873
std::same_as<U, Char> ||
5974
((std::same_as<U, std::byte> ||
6075
(std::is_integral_v<U> && !std::same_as<U, bool>)) &&
61-
sizeof(U) == sizeof(Char));
76+
code_unit_width<U> == code_unit_width<Char>);
6277

6378
template <typename Char>
6479
struct as_code_unit_impl

‎include/beman/utf_view/to_utf_view.hpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,10 @@ struct to_utf_view<V, E, ToType>::exposition_only_iterator
294294
[[no_unique_address]] std::ranges::sentinel_t<exposition_only_Base> exposition_only_end_{}; // @*exposition only*@
295295
/* PAPER: sentinel_t<exposition_only_Base> @*end_*@; // @*exposition only*@ */
296296

297-
detail::fake_inplace_vector<value_type, 4 / sizeof(ToType)> exposition_only_buf_{}; // @*exposition only*@
298-
/* PAPER: inplace_vector<value_type, 4 / sizeof(ToType)> @*buf_*@{}; // @*exposition only*@ */
297+
detail::fake_inplace_vector<value_type,
298+
std::same_as<ToType, char8_t> ? 4 : std::same_as<ToType, char16_t> ? 2 : 1>
299+
exposition_only_buf_{}; // @*exposition only*@
300+
/* PAPER: inplace_vector<value_type, same_as<ToType, char8_t> ? 4 : same_as<ToType, char16_t> ? 2 : 1> @*buf_*@{}; // @*exposition only*@ */
299301
/* PAPER */
300302

301303
std::int8_t exposition_only_buf_index_{}; // @*exposition only*@

‎papers/P2728.md‎

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,7 @@ private:
748748
iterator_t<@*Base*@> @*current_*@{}; // @*exposition only*@
749749
sentinel_t<@*Base*@> @*end_*@; // @*exposition only*@
750750

751-
inplace_vector<value_type, 4 / sizeof(ToType)> @*buf_*@{}; // @*exposition only*@
751+
inplace_vector<value_type, same_as<ToType, char8_t> ? 4 : same_as<ToType, char16_t> ? 2 : 1> @*buf_*@{}; // @*exposition only*@
752752

753753
int8_t @*buf_index_*@{}; // @*exposition only*@
754754
uint8_t @*to_increment_*@{}; // @*exposition only*@
@@ -1109,11 +1109,13 @@ The names `as_char`, `as_wchar_t`, `as_char8_t`, `as_char16_t`, and
11091109
`as_char16_t`, and `as_char32_t`. Let `Char` be the corresponding character
11101110
type for `as_charT`, let `E` be an expression, let `T` be
11111111
`remove_cvref_t<decltype((E))>`, and let `U` be
1112-
`remove_cvref_t<ranges::range_reference_t<T>>`. `as_charT(E)` is ill-formed if
1113-
`T` is an array, or if `U` is not `Char` and `U` is not a type `X` such that
1114-
`sizeof(X) == sizeof(Char)` and either `X` is `byte` or `X` is an integral type
1115-
other than `bool`. Otherwise, the expression `as_charT(E)` is
1116-
expression-equivalent to:
1112+
`remove_cvref_t<ranges::range_reference_t<T>>`. For a type `X`, let the width
1113+
of `X` be the width ([basic.fundamental]) of `X` if `X` is a signed or unsigned
1114+
integer type, and otherwise the width of the underlying type of `X`.
1115+
`as_charT(E)` is ill-formed if `T` is an array, or if `U` is not `Char` and `U`
1116+
is not a type `X` such that the width of `X` equals the width of `Char` and
1117+
either `X` is `byte` or `X` is an integral type other than `bool`. Otherwise,
1118+
the expression `as_charT(E)` is expression-equivalent to:
11171119

11181120
- If `T` is a specialization of `empty_view` ([range.empty.view]), then
11191121
`empty_view<Char>{}`.
@@ -1330,7 +1332,7 @@ For the `as_charT` casting views, the rule we use to determine whether the eleme
13301332
is:
13311333
13321334
- `U` is `Char`, or
1333-
- `U` is `std::byte` or a non-`bool` integral type, and `sizeof(U) == sizeof(Char)`.
1335+
- `U` is `std::byte` or a non-`bool` integral type, and `U` has the same width as `Char`.
13341336
13351337
This accomplishes the following design goals:
13361338
@@ -1352,23 +1354,32 @@ out a `transform_view` if, for example, they want to cast between `char16_t` and
13521354
merits an explicit carveout here since there's no justification based on backwards
13531355
compatibility).
13541356
1357+
### All integral type sizes permitted by the standard, assuming `CHAR_BIT == 8`
1358+
1359+
"Same width" marks pairs whose relative widths the standard does not fix.
1360+
1361+
| Source | `as_char` | `as_char8_t` | `as_char16_t` | `as_char32_t` | `as_wchar_t` |
1362+
|---|---|---|---|---|---|
1363+
| `char`, `signed char`, `unsigned char`, `char8_t`, `std::byte` | yes | yes | no | no | same width |
1364+
| `char16_t` | no | no | yes | same width | same width |
1365+
| `char32_t` | no | no | same width | yes | same width |
1366+
| `wchar_t` | same width | same width | same width | same width | yes |
1367+
| `short`, `int`, `long`, `long long`, and their `unsigned` counterparts | no | no | same width | same width | same width |
1368+
| `bool`, floating-point types, enumerations other than `std::byte`, class types | no | no | no | no | no |
1369+
1370+
### Typical x86_64/aarch64 integral type sizes
1371+
13551372
| Source | `as_char` | `as_char8_t` | `as_char16_t` | `as_char32_t` | `as_wchar_t` |
13561373
|---|---|---|---|---|---|
1357-
| `char` | yes | yes | no | no | no |
1358-
| `signed char`, `unsigned char` | yes | yes | no | no | no |
1359-
| `std::byte` | yes | yes | no | no | no |
1360-
| `char8_t` | yes | yes | no | no | no |
1374+
| `char`, `signed char`, `unsigned char`, `char8_t`, `std::byte` | yes | yes | no | no | no |
13611375
| `char16_t` | no | no | yes | no | same width |
13621376
| `char32_t` | no | no | no | yes | same width |
13631377
| `wchar_t` | no | no | same width | same width | yes |
13641378
| `short`, `unsigned short` | no | no | yes | no | same width |
1365-
| `int`, `unsigned int` | no | no | no | same width | same width |
1366-
| `long`, `unsigned long` | no | no | no | same width | same width |
1379+
| `int`, `unsigned int` | no | no | no | yes | same width |
1380+
| `long`, `unsigned long` | no | no | no | same width | no |
13671381
| `long long`, `unsigned long long` | no | no | no | no | no |
1368-
| `bool` | no | no | no | no | no |
1369-
| floating-point types | no | no | no | no | no |
1370-
| enumerations other than `std::byte` | no | no | no | no | no |
1371-
| class types | no | no | no | no | no |
1382+
| `bool`, floating-point types, enumerations other than `std::byte`, class types | no | no | no | no | no |
13721383
13731384
## Why `to_utf_view` doesn't have a deduction guide like other views do
13741385
@@ -1481,6 +1492,11 @@ adjacent_view, and adjacent_transform_view.
14811492
- Remove `to_utf_tag` types and the `to_utf_view` deduction guide; add discussion section
14821493
on dropping support for CTAD per
14831494
`elements_view`/`adjacent_view`/`adjacent_transform_view` precedent.
1495+
- Specify the code unit adaptor conversion rule in terms of width rather than `sizeof`,
1496+
and split its table into one reflecting only the standard's guarantees and one
1497+
reflecting typical platforms, per SG16 feedback.
1498+
- Size the exposition-only transcoding buffer by target type rather than by
1499+
`sizeof(ToType)`.
14841500

14851501
## Changes since R14
14861502

‎tests/beman/utf_view/code_unit_view.test.cpp‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import std;
1717
#include <cstddef>
1818
#include <cstdint>
1919
#include <ranges>
20+
#include <limits>
2021
#include <span>
2122
#include <string>
2223
#include <string_view>
@@ -108,6 +109,22 @@ static_assert(
108109

109110
// Conversion rule: a source type is accepted if it is the target type, or if it
110111
// is std::byte or a non-bool integral type of the same width as the target.
112+
// Width counts value and sign bits and excludes padding, so signed and
113+
// unsigned types of the same size have the same width.
114+
constexpr int char_bit{std::numeric_limits<unsigned char>::digits};
115+
static_assert(detail::code_unit_width<char> == char_bit);
116+
static_assert(detail::code_unit_width<signed char> == char_bit);
117+
static_assert(detail::code_unit_width<unsigned char> == char_bit);
118+
static_assert(detail::code_unit_width<std::byte> == char_bit);
119+
static_assert(detail::code_unit_width<char8_t> == char_bit);
120+
static_assert(detail::code_unit_width<short> ==
121+
detail::code_unit_width<unsigned short>);
122+
static_assert(detail::code_unit_width<int> == detail::code_unit_width<unsigned>);
123+
static_assert(detail::code_unit_width<char16_t> >= 16);
124+
static_assert(detail::code_unit_width<char32_t> >= 32);
125+
static_assert(detail::code_unit_width<wchar_t> ==
126+
std::numeric_limits<wchar_t>::digits +
127+
std::numeric_limits<wchar_t>::is_signed);
111128
template <class R, auto const& Adaptor>
112129
constexpr bool adaptable = requires { std::declval<R>() | Adaptor; };
113130

@@ -153,7 +170,7 @@ static_assert(adaptable<std::span<short const>, as_char16_t>);
153170
static_assert(adaptable<std::span<unsigned short const>, as_char16_t>);
154171
static_assert(adaptable<std::span<std::uint16_t const>, as_char16_t>);
155172
static_assert(adaptable<std::wstring_view, as_char16_t> ==
156-
(sizeof(wchar_t) == sizeof(char16_t)));
173+
(detail::code_unit_width<wchar_t> == detail::code_unit_width<char16_t>));
157174
static_assert(!adaptable<std::u8string_view, as_char16_t>);
158175
static_assert(!adaptable<std::u32string_view, as_char16_t>);
159176
static_assert(!adaptable<std::string_view, as_char16_t>);
@@ -164,13 +181,13 @@ static_assert(adaptable<std::u32string_view, as_char32_t>);
164181
static_assert(adaptable<std::span<std::uint32_t const>, as_char32_t>);
165182
static_assert(adaptable<std::span<std::int32_t const>, as_char32_t>);
166183
static_assert(adaptable<std::span<int const>, as_char32_t> ==
167-
(sizeof(int) == sizeof(char32_t)));
184+
(detail::code_unit_width<int> == detail::code_unit_width<char32_t>));
168185
static_assert(adaptable<std::span<unsigned const>, as_char32_t> ==
169-
(sizeof(unsigned) == sizeof(char32_t)));
186+
(detail::code_unit_width<unsigned> == detail::code_unit_width<char32_t>));
170187
static_assert(adaptable<std::span<long const>, as_char32_t> ==
171-
(sizeof(long) == sizeof(char32_t)));
188+
(detail::code_unit_width<long> == detail::code_unit_width<char32_t>));
172189
static_assert(adaptable<std::wstring_view, as_char32_t> ==
173-
(sizeof(wchar_t) == sizeof(char32_t)));
190+
(detail::code_unit_width<wchar_t> == detail::code_unit_width<char32_t>));
174191
static_assert(!adaptable<std::u8string_view, as_char32_t>);
175192
static_assert(!adaptable<std::u16string_view, as_char32_t>);
176193
static_assert(!adaptable<std::span<long long const>, as_char32_t>);
@@ -179,11 +196,11 @@ static_assert(!adaptable<std::span<double const>, as_char32_t>);
179196
// as_wchar_t
180197
static_assert(adaptable<std::wstring_view, as_wchar_t>);
181198
static_assert(adaptable<std::u16string_view, as_wchar_t> ==
182-
(sizeof(wchar_t) == sizeof(char16_t)));
199+
(detail::code_unit_width<wchar_t> == detail::code_unit_width<char16_t>));
183200
static_assert(adaptable<std::u32string_view, as_wchar_t> ==
184-
(sizeof(wchar_t) == sizeof(char32_t)));
201+
(detail::code_unit_width<wchar_t> == detail::code_unit_width<char32_t>));
185202
static_assert(adaptable<std::span<unsigned short const>, as_wchar_t> ==
186-
(sizeof(wchar_t) == sizeof(unsigned short)));
203+
(detail::code_unit_width<wchar_t> == detail::code_unit_width<unsigned short>));
187204
static_assert(!adaptable<std::u8string_view, as_wchar_t>);
188205
static_assert(!adaptable<std::string_view, as_wchar_t>);
189206

@@ -239,7 +256,7 @@ constexpr bool signed_source_test() {
239256
}
240257

241258
constexpr bool int_source_test() {
242-
if constexpr (sizeof(int) == sizeof(char32_t)) {
259+
if constexpr (detail::code_unit_width<int> == detail::code_unit_width<char32_t>) {
243260
std::array<int, 2> const code_points{0x1F574, 0xFFFD};
244261
auto const as_utf32{code_points | as_char32_t |
245262
std::ranges::to<std::u32string>()};

0 commit comments

Comments
 (0)