Split part of concepts.h into range_concepts.h#5294
Conversation
c9195fc to
0bb839a
Compare
There was a problem hiding this comment.
Pull request overview
This PR reduces compile-time cost of <botan/concepts.h> by splitting range-related concepts/utilities into a dedicated <botan/range_concepts.h>, and relocates strong-type related concepts into <botan/strong_type.h>. It also removes the “opportunistic” forwarding of bounds-checked container accessors (at()) from Strong<>.
Changes:
- Introduce
src/lib/utils/range_concepts.hand moveBotan::ranges::*concepts/helpers there. - Move strong-type traits/concepts (
is_strong_type_v,concepts::*_strong_type, etc.) fromconcepts.hintostrong_type.h, and drop forwarding ofat()in the strong container adapter. - Update various includes (and a couple missing standard includes) to reflect the header split.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/unit_x509.cpp | Adds <algorithm> to support std::ranges::find usage. |
| src/tests/test_strong_type.cpp | Removes tests for bounds-checked accessor forwarding (Strong<>::at()). |
| src/lib/utils/strong_type.h | Adds strong-type traits/concepts previously in concepts.h; removes at() forwarding from container adapter. |
| src/lib/utils/stl_util.h | Includes the new <botan/range_concepts.h> (range utilities now live there). |
| src/lib/utils/range_concepts.h | New header providing Botan::ranges::* concepts and byte-length assertion helpers. |
| src/lib/utils/mem_ops.h | Switches from <botan/concepts.h> to <botan/range_concepts.h> for range helpers. |
| src/lib/utils/loadstor.h | Adjusts includes to use <botan/range_concepts.h> after the split. |
| src/lib/utils/info.txt | Exposes range_concepts.h as public (with a TODO note). |
| src/lib/utils/ct_utils.h | Switches from <botan/concepts.h> to <botan/range_concepts.h> for ranges::* usage. |
| src/lib/utils/concepts.h | Removes Botan::ranges::* and strong-type related concepts/traits. |
| src/lib/utils/assert.cpp | Switches include to <botan/range_concepts.h> (contains memory_region_size_violation decl now). |
| src/lib/utils/alignment_buffer.h | Adds <tuple> for std::tuple return type usage. |
| src/lib/pubkey/ec_group/ec_scalar.h | Adds <string_view> for std::string_view parameter usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| inline constexpr void assert_exact_byte_length(const R& r) { | ||
| const std::span s{r}; | ||
| if constexpr(statically_spanable_range<R>) { | ||
| static_assert(s.size_bytes() == expected, "memory region does not have expected byte lengths"); |
There was a problem hiding this comment.
The static_assert message says "expected byte lengths" (plural) although it checks a single expected length. Consider changing it to "expected byte length" to match the condition and to improve diagnostics.
| static_assert(s.size_bytes() == expected, "memory region does not have expected byte lengths"); | |
| static_assert(s.size_bytes() == expected, "memory region does not have expected byte length"); |
0bb839a to
5d75e32
Compare
Due to <ranges> this header is quite expensive to compile, and the ranges parts aren't needed by many users of concepts.h - and in fact isn't needed at all in the public API outside of the (deprecated for public use) mem_ops.h Move the parts of concepts.h related to strong types to that header. Remove the opportunistic exposure of checked accessors in strong type; it wasn't used outside of tests and the concept is somewhat complicated.
5d75e32 to
4086b4c
Compare
Due to this header is quite expensive to compile, and the ranges parts aren't needed by many users of concepts.h - and in fact isn't needed at all in the public API outside of the (deprecated for public use) mem_ops.h
Move the parts of concepts.h related to strong types to that header.
Remove the opportunistic exposure of checked accessors in strong type; it wasn't used outside of tests and the concept is somewhat complicated.