refactor: include what these two files use, and reverse by range - #827
Merged
Conversation
`random.cpp` calls `std::back_inserter` and `odf_table_test.cpp` calls `std::ranges::find`, and neither includes the header that declares it. Both compile today only because libc++ hands them out through `<algorithm>` and `<random>` — which it stops doing under C++23, so this is the whole source cost of that bump, extracted. `type1_charstring`'s `std::reverse(args.begin(), args.end())` becomes `std::ranges::reverse(args)` per the ranges convention. C++20, unrelated to the above; it is here because it is the same size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoTh7BEUSL2z9BBThgsEi7
andiwand
added a commit
that referenced
this pull request
Sep 6, 2026
The whole matrix already supports it: clang 18, gcc 14, apple-clang, MSVC 19.40, NDK 28.1 and emsdk 3.1.73. It costs no source change at all — the two transitive includes libc++'s C++23 headers stopped handing out went in with #827, which is the whole of it. Three ceilings sit under the standard, and AGENTS.md records all three because none is discoverable from a local build. The library half is capped by emsdk 3.1.73's libc++ 18.1, the oldest here and the newest emsdk conan-center packages. `std::format` is unusable on any slice — the apple profiles deploy to macOS 12 / iOS 15, and libc++ marks the floating-point `to_chars` that `<format>` instantiates as macOS 13.3 / iOS 16.3, which is why #828 formats through `fmt`. And NDK 28.1's clang 19 segfaults on a capturing recursive lambda taking `this auto self`. The public headers stay C++20 — no `target_compile_features(odr PUBLIC …)` and no `cppstd` in `package_info`, so a consumer picks its own standard, and `check_min_cppstd` sits in `validate_build` where it constrains building `odr` rather than using it. On MSVC there is no `/std:c++23`; CMake maps `CXX_STANDARD 23` to `/std:c++latest`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoTh7BEUSL2z9BBThgsEi7
andiwand
added a commit
that referenced
this pull request
Sep 6, 2026
The whole matrix already supports it: clang 18, gcc 14, apple-clang, MSVC 19.40, NDK 28.1 and emsdk 3.1.73. It costs no source change at all — the two transitive includes libc++'s C++23 headers stopped handing out went in with #827, which is the whole of it. Three ceilings sit under the standard, and AGENTS.md records all three because none is discoverable from a local build. The library half is capped by emsdk 3.1.73's libc++ 18.1, the oldest here and the newest emsdk conan-center packages. `std::format` is unusable on any slice — the apple profiles deploy to macOS 12 / iOS 15, and libc++ marks the floating-point `to_chars` that `<format>` instantiates as macOS 13.3 / iOS 16.3, which is why #828 formats through `fmt`. And NDK 28.1's clang 19 segfaults on a capturing recursive lambda taking `this auto self`. The public headers stay C++20 — no `target_compile_features(odr PUBLIC …)` and no `cppstd` in `package_info`, so a consumer picks its own standard, and `check_min_cppstd` sits in `validate_build` where it constrains building `odr` rather than using it. On MSVC there is no `/std:c++23`; CMake maps `CXX_STANDARD 23` to `/std:c++latest`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoTh7BEUSL2z9BBThgsEi7
andiwand
added a commit
that referenced
this pull request
Sep 6, 2026
* build!: raise the standard to C++23 The whole matrix already supports it: clang 18, gcc 14, apple-clang, MSVC 19.40, NDK 28.1 and emsdk 3.1.73. It costs no source change at all — the two transitive includes libc++'s C++23 headers stopped handing out went in with #827, which is the whole of it. Three ceilings sit under the standard, and AGENTS.md records all three because none is discoverable from a local build. The library half is capped by emsdk 3.1.73's libc++ 18.1, the oldest here and the newest emsdk conan-center packages. `std::format` is unusable on any slice — the apple profiles deploy to macOS 12 / iOS 15, and libc++ marks the floating-point `to_chars` that `<format>` instantiates as macOS 13.3 / iOS 16.3, which is why #828 formats through `fmt`. And NDK 28.1's clang 19 segfaults on a capturing recursive lambda taking `this auto self`. The public headers stay C++20 — no `target_compile_features(odr PUBLIC …)` and no `cppstd` in `package_info`, so a consumer picks its own standard, and `check_min_cppstd` sits in `validate_build` where it constrains building `odr` rather than using it. On MSVC there is no `/std:c++23`; CMake maps `CXX_STANDARD 23` to `/std:c++latest`. * refactor: let deducing `this` write the const overload Forty-two accessors were the same body twice, once for each constness, with only the spelled-out return type telling the two apart. An explicit object parameter deduces that, so each pair is one function returning `T &` or `const T &` from a single body. The shared registry (#823) is where it pays most: `SideTable` and `SortedSideTable` had written the workaround out by hand — two public overloads delegating to a `static` helper templated on the object, under a comment naming the trick — and every engine then repeated the pair for each of its payload accessors. Both go, and a registry's accessor is [[nodiscard]] auto &text_element_at(this auto &self, const ElementIdentifier id) { return self.m_texts.at(id); } `pdf`'s `Array` and `Dictionary` lose eleven more. `ooxml_text_list`'s numbering walk would have dropped its Y-combinator with them, but NDK 28.1's clang 19 segfaults on a capturing lambda that recurses through an explicit object parameter, so it keeps passing itself along and says why. * perf: stop zeroing the buffer the next read overwrites anyway `resize` fills the new tail with zeros; every one of these then writes over all of it. `resize_and_overwrite` hands the chunk over unwritten instead — 12.3 GB/s to 16.7 GB/s on `read_u8s` against an in-memory stream, which is what a zip entry is. The callback must not throw, so a short read shrinks the string back to the offset it started from and the throw happens at the call site. `ppt`'s `read_raw_text_bytes` loses its second `resize` with it: the callback returns `gcount()` and the string is already the right length. `xls_io`'s string body is left alone — its `read_bytes` throws from inside, and a non-throwing path just for this is not worth the buffer it saves. * refactor: build the two derived vectors with `ranges::to` Both loops did nothing but map a range onto a vector, and both fed it straight into one call. `views::transform | ranges::to<std::vector<std::string>>()` says that in the expression that uses it, so neither needs a named variable any more. `type1_charstring` takes the iterator-pair `std::reverse` next to it with them, per the ranges convention. * test(build): hold the public headers to C++20 The bump made `odr` C++23 and deliberately did not pass that on — no `target_compile_features(odr PUBLIC …)`, no `cppstd` in the conan `package_info` — so a consumer keeps whatever standard it picked, as long as `src/odr/*.hpp` stays C++20. Nothing checked that, and a `this auto &self` in a public header would have broken someone else's build rather than ours. `odr_public_headers_cpp20` includes all seventeen of them and compiles at C++20. One object file, no test data, no link, no gtest — it either compiles or it does not. It lives in `test/CMakeLists.txt` and so builds under `ODR_TEST`, which is also what keeps it out of the conan package: `exports_sources` ships no `test/`, and an unconditional target naming a file that is not there fails the package build at generate time. * docs: cut the prose from the comments this branch adds The C++23-does-not-propagate rationale now lives in AGENTS.md alone; the test file and its cmake target state what they are and stop there. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Broken out of #826 — three changes that are valid C++20 and need nothing from
the bump, so the C++23 PR is left holding only what actually requires C++23.
common/random.cppcallsstd::back_inserterwithout<iterator>.odf_table_test.cppcallsstd::ranges::findwithout<algorithm>.Both compile today only because libc++ hands those out transitively through
<algorithm>and<random>. It stops doing so under C++23, which is why thesetwo lines were the entire source cost of that bump — they are correct on their
own terms either way.
type1_charstring.cpp'sstd::reverse(args.begin(), args.end())becomesstd::ranges::reverse(args), per the ranges convention in AGENTS.md. C++20,unrelated to the includes; here because it is the same size.
Builds clean at C++20 (
-Wall -Wextra), no behaviour change.