STL_Extension: document CGAL::cpp23::expected<T,E> in the package manual#9350
STL_Extension: document CGAL::cpp23::expected<T,E> in the package manual#9350RajdeepKushwaha5 wants to merge 1 commit intoCGAL:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds documentation for the CGAL::cpp23::expected<T,E> type (and related types) that was vendored in <CGAL/expected.h> from the tl/expected library (v1.3.1) in a prior PR but shipped without documentation. It fills that gap by adding a new Doxygen reference header, a user-manual section, and reference-index entries.
Changes:
- New
STL_Extension/doc/STL_Extension/CGAL/expected.h: Full Doxygen reference docs for all six public names inCGAL::cpp23 - Updated
STL_Extension/doc/STL_Extension/STL_Extension.txt: New\section stl_expectedsection with narrative and code examples, and an updated introduction paragraph - Updated
STL_Extension/doc/STL_Extension/PackageDescription.txt: New\cgalCRPSectionlisting all six public names for the reference-manual index
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
STL_Extension/doc/STL_Extension/CGAL/expected.h |
New Doxygen reference documentation for expected, unexpected, bad_expected_access, unexpect_t, unexpect, and make_unexpected |
STL_Extension/doc/STL_Extension/STL_Extension.txt |
New user-manual section explaining usage, with two code examples; intro paragraph updated |
STL_Extension/doc/STL_Extension/PackageDescription.txt |
New reference-manual section listing all six public names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| compilers that do not yet fully support C++23's `<expected>` header. | ||
| The implementation is sourced from the | ||
| <a href="https://github.com/TartanLlama/expected">TartanLlama/expected</a> library | ||
| (v1.3.1, CC0 licence) with the following changes: |
There was a problem hiding this comment.
The word "licence" is a British English spelling. The CGAL project uses American English spelling conventions throughout its codebase (e.g., the actual file uses "SPDX-License-Identifier" and the CC0 dedication text uses "license"). This should be "license".
| (v1.3.1, CC0 licence) with the following changes: | |
| (v1.3.1, CC0 license) with the following changes: |
| /// \name Monadic Operations | ||
| /// @{ | ||
| /*! | ||
| If `has_value()`, calls `f(value())` and returns its result (an `expected`); | ||
| otherwise propagates the error. | ||
| */ | ||
| template <class F> | ||
| constexpr auto and_then(F&& f) &; | ||
|
|
||
| /*! | ||
| If `has_value()`, applies `f` to the contained value and wraps the result in | ||
| an `expected`; otherwise propagates the error unchanged. | ||
| */ | ||
| template <class F> | ||
| constexpr auto map(F&& f) &; | ||
|
|
||
| /*! | ||
| If `!has_value()`, calls `f(error())` and returns its result; | ||
| otherwise propagates the value. | ||
| */ | ||
| template <class F> | ||
| constexpr auto or_else(F&& f) &; | ||
|
|
||
| /*! | ||
| If `!has_value()`, applies `f` to the error and returns an `expected` with | ||
| the transformed error; otherwise propagates the value unchanged. | ||
| */ | ||
| template <class F> | ||
| constexpr auto map_error(F&& f) &; | ||
| /// @} |
There was a problem hiding this comment.
The monadic operations section documents map and map_error (tl/expected extensions), but omits transform and transform_error which are the C++23 standard names for the same operations. Since this type is presented as a std::expected compatibility shim, users expecting the C++23 API (i.e., transform and transform_error) would be left without documentation for those methods. Both sets of names exist in the actual implementation (STL_Extension/include/CGAL/expected.h:1425 onwards). Consider documenting transform and transform_error as well, or at minimum adding a note that map/map_error are equivalent to the standard transform/transform_error.
|
|
||
| /*! | ||
| Returns a reference to the contained expected value. | ||
| \pre `has_value()` is `true`. Throws `CGAL::cpp23::bad_expected_access<E>` otherwise (when exceptions are enabled). |
There was a problem hiding this comment.
The \pre tag on line 122 conflates a precondition with exception-throwing behavior. The value() function does not have a precondition that has_value() is true; instead, it actively checks the state and throws bad_expected_access<E> (or calls std::terminate()) when has_value() is false. A \pre implies that the caller has a contractual obligation and that violating it results in undefined behavior, which is inaccurate here. The exception/terminate behavior should be documented as a runtime guarantee (e.g., with \throws or inline prose), not as a precondition.
| \pre `has_value()` is `true`. Throws `CGAL::cpp23::bad_expected_access<E>` otherwise (when exceptions are enabled). | |
| If `has_value()` is `false`, this function throws `CGAL::cpp23::bad_expected_access<E>` (when exceptions are enabled), | |
| or calls `std::terminate()` otherwise. | |
| \throws CGAL::cpp23::bad_expected_access<E> if `has_value()` is `false` and exceptions are enabled. |
| template <class E> | ||
| class bad_expected_access : public std::exception | ||
| { | ||
| public: | ||
| /*! | ||
| Returns a reference to the stored error value. | ||
| */ | ||
| constexpr const E& error() const &; | ||
| }; |
There was a problem hiding this comment.
The bad_expected_access<E> documentation is missing both a constructor and the what() override. The actual implementation (at STL_Extension/include/CGAL/expected.h:1264-1279) has an explicit bad_expected_access(E e) constructor and a virtual const char* what() const noexcept override method. The class is documented here as inheriting from std::exception, which means users will expect what() to be available. Omitting these public members leaves the API partially undocumented.
Add Doxygen documentation for the vendored tl/expected (v1.3.1) types
that live in the CGAL::cpp23 namespace and are exposed via
<CGAL/expected.h>:
- CGAL::cpp23::expected<T,E> -- C++23 std::expected analogue
- CGAL::cpp23::unexpected<E> -- error wrapper
- CGAL::cpp23::bad_expected_access<E> -- exception on bad value access
- CGAL::cpp23::unexpect_t / unexpect -- in-place error-construction tag
- CGAL::cpp23::make_unexpected() -- factory helper
Changes:
* STL_Extension/doc/STL_Extension/CGAL/expected.h (new)
Full Doxygen reference documentation with member descriptions and
a usage example.
* STL_Extension/doc/STL_Extension/STL_Extension.txt
New section \\section stl_expected with narrative, code examples, and
exception/terminate semantics note. Introduction paragraph updated.
* STL_Extension/doc/STL_Extension/PackageDescription.txt
New \\cgalCRPSection{Expected Value or Error (C++23)} with all six
public names listed for the reference-manual index.
Fixes CGAL#9280
RajdeepKushwaha5
force-pushed
the
doc/stl-extension-expected-h
branch
from
062c1c4 to
13ba3af
Compare
Fixes #9280
Add Doxygen reference documentation and user-manual text for the vendored tl/expected (v1.3.1) types shipped in
<CGAL/expected.h>under PR #9197 but not documented at the time.Changes
STL_Extension/doc/STL_Extension/CGAL/expected.h (new file) Full Doxygen docs for all six public names in
CGAL::cpp23:expected<T,E>,unexpected<E>,bad_expected_access<E>,unexpect_t,unexpect, andmake_unexpected(). Each entry has member docs, \sa cross-links, and a code example.STL_Extension/doc/STL_Extension/STL_Extension.txt
New
\section stl_expectedwith narrative, two code examples, and a note on exception vs. terminate semantics. Introduction paragraph updated.STL_Extension/doc/STL_Extension/PackageDescription.txt
New
\cgalCRPSection{Expected Value or Error (C++23)}listing all six public names for the reference-manual index.