diff --git a/.github/dependabot.yml b/.github/dependabot.yml index dfb30a7..22a708c 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -23,11 +23,6 @@ updates: schedule: interval: "weekly" - - package-ecosystem: pip - directory: /papers/P2988 - schedule: - interval: daily - - package-ecosystem: pip directory: / schedule: diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 3d96a8b..ddd7b95 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -50,6 +50,8 @@ jobs: include: - language: actions build-mode: none + - language: c-cpp + build-mode: manual # CodeQL supports the following values keywords for 'language': 'actions', 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'swift' # Use `c-cpp` to analyze code written in C, C++ or both # Use 'java-kotlin' to analyze code written in Java, Kotlin or both @@ -95,15 +97,12 @@ jobs: # to build your code. # ℹ️ Command-line programs to run using the OS shell. # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun - - if: matrix.build-mode == 'manual' + - name: Build C++ with the GCC debug preset + if: matrix.build-mode == 'manual' shell: bash run: | - echo 'If you are using a "manual" build mode for one or more of the' \ - 'languages you are analyzing, replace this with the commands to build' \ - 'your code, for example:' - echo ' make bootstrap' - echo ' make release' - exit 1 + cmake --preset gcc-debug + cmake --build --preset gcc-debug - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@e4fba868fa4b1b91e1fdab776edc8cfbe6e9fb81 # v4.37.3 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8742438..b745b87 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,7 +37,7 @@ If the presets are not suitable for your use case, a traditional CMake invocatio provide more configurability. To configure, build and test the project manually, you can run this set of commands. Note -that this requires GoogleTest to be installed. +that this requires Catch2 to be installed. ```bash cmake \ @@ -65,7 +65,7 @@ The best way to install the project's dependencies is to use the vcpkg workflow. To do so, make sure vcpkg is installed and `VCPKG_ROOT` is defined in your environment, then specify `-DCMAKE_TOOLCHAIN_FILE="$VCPKG_ROOT/scripts/buildsystems/vcpkg.cmake"`. Vcpkg will handle -the project's dependencies, including GoogleTest. +the project's dependencies, including Catch2. Example commands: @@ -73,7 +73,7 @@ Example commands: cmake \ -B build \ -S . \ - -DCMAKE_CXX_STANDARD=17 \ + -DCMAKE_CXX_STANDARD=20 \ -DCMAKE_TOOLCHAIN_FILE="$VCPKG_ROOT/scripts/buildsystems/vcpkg.cmake" cmake --build build ctest --test-dir build @@ -89,7 +89,7 @@ configure beman.expected to fetch them automatically via CMake FetchContent. To do so, specify `-DCMAKE_PROJECT_TOP_LEVEL_INCLUDES=./infra/cmake/use-fetch-content.cmake`. This will -bring in GoogleTest automatically along with any other dependency the project may require. +bring in Catch2 automatically along with any other dependency the project may require. Example commands: diff --git a/Makefile b/Makefile index a75cb86..eb40c92 100755 --- a/Makefile +++ b/Makefile @@ -55,7 +55,7 @@ VCPKG ?= $(shell command -v vcpkg 2> /dev/null) ifeq ($(VCPKG),) _cmake_top_level?="./cmake/use-fetch-content.cmake" _toolchain:=$(_local_toolchain) - _args=-DBEMANINFRA_googletest_REPO=file:///home/sdowney/bld/googletest/googletest.git + _args= else _vcpkg_toolchain:=$(VCPKG_ROOT)/scripts/buildsystems/vcpkg.cmake _cmake_top_level?=$(_vcpkg_toolchain) @@ -117,7 +117,7 @@ compile-headers: $(_build_path)/CMakeCache.txt ## Compile the headers .PHONY: install install: $(_build_path)/CMakeCache.txt compile ## Install the project - $(CMAKE) --install $(_build_path) --config $(CONFIG) --component beman.expected --verbose + $(CMAKE) --install $(_build_path) --config $(CONFIG) --verbose .PHONY: clean-install clean-install: @@ -155,7 +155,7 @@ env: .PHONY: papers papers: - $(MAKE) -C papers/P2988 papers + $(MAKE) -C papers papers .DEFAULT: $(_build_path)/CMakeCache.txt ## Other targets passed through to cmake $(CMAKE) --build $(_build_path) --config $(CONFIG) --target $@ -- -k 0 diff --git a/README.md b/README.md index cb669f3..069b08c 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,10 @@ The fix is to make each such member's definition token-identical to its declaration; that work is not done here simply because we do not have direct MSVC access to develop and verify the workarounds. Contributions with MSVC access are welcome. + +The Windows setup below is retained for contributors who want to investigate +MSVC support. It is currently unsupported and unverified; the documented build +and test commands are not expected to pass with MSVC yet. > These development environments are verified using our CI configuration. ## Development @@ -138,15 +142,18 @@ brew install llvm
For Windows -To build Beman libraries, you will need the MSVC compiler. MSVC can be obtained -by installing Visual Studio; the free Visual Studio 2022 Community Edition can -be downloaded from +> [!WARNING] +> +> MSVC builds are currently unsupported and unverified. These instructions only +> establish a Windows development environment for work on that support. + +The MSVC compiler can be obtained by installing Visual Studio; the free Visual +Studio 2022 Community Edition can be downloaded from [Microsoft](https://visualstudio.microsoft.com/vs/community/). After Visual Studio has been installed, you can launch "Developer PowerShell for VS 2022" by typing it into Windows search bar. This shell environment will -provide CMake, Ninja, and MSVC, allowing you to build the library and run the -tests. +provide CMake, Ninja, and MSVC for investigating the current build failures.
@@ -259,6 +266,11 @@ include an appropriate `beman.expected` header from your source code. > Altering include search paths to spell the include target another way (e.g. > `#include `) is unsupported. +Textual-header consumers can detect the reference extension with the public +configuration macro `BEMAN_EXPECTED_HAS_REFERENCES`, whose value is `1`. +Importing the `beman.expected` module does not import preprocessor macros; +module support itself is unconditional when that build mode is selected. + The process for incorporating `beman.expected` into your project depends on the build system being used. Instructions for CMake are provided in following sections. @@ -296,6 +308,8 @@ This will generate the following directory structure at `/opt/beman`. │   └── beman │   └── expected │   ├── bad_expected_access.hpp +│   ├── config.hpp +│   ├── config_generated.hpp │   ├── expected.hpp │   └── unexpected.hpp └── lib @@ -303,4 +317,5 @@ This will generate the following directory structure at `/opt/beman`. └── beman.expected ├── beman.expected-config.cmake ├── beman.expected-config-version.cmake - └── beman.expected-targets.cmake``` + └── beman.expected-targets.cmake +``` diff --git a/docs/human-design-review-guide.md b/docs/human-design-review-guide.md index a2dbac0..410276f 100644 --- a/docs/human-design-review-guide.md +++ b/docs/human-design-review-guide.md @@ -6,7 +6,7 @@ A complete implementation of `std::expected` (C++26) extended to allow `T` and/or `E` to be reference types, proposed for C++29. This is a new `expected` specialization for a reference value, a relaxation of the primary and `void` templates to admit a reference error `E`, and a new `unexpected` — three `expected` class templates in all, each accepting an object or reference error. The reference semantics follow P2988 (`optional`): rebind on assignment, shallow const, dangling prevention via deleted constructors. -**File count:** 3 headers, ~4400 lines of implementation, 15 positive test files (469 tests), 54 negative compile tests. +**File count:** 3 headers, 3,413 lines in `expected.hpp`, 19 positive test files with 688 Catch2 `TEST_CASE`s, and 59 negative compile tests. **This guide is not a checklist.** It's a map of the decisions that shaped this code. Some are obviously correct. Some are defensible but debatable. Some might be wrong. Your job is to decide which is which. @@ -41,7 +41,7 @@ generates them. ### Why this matters -The total is ~4300 lines of template code in a single header. A factored implementation using a common base or policy template could plausibly cut this by 40-60%. The monadic operations alone account for 4 operations x 4 ref-qualified overloads across the three templates — dozens of method definitions that share structural similarity. +The total is 3,413 lines of template code in a single header. A factored implementation using a common base or policy template could plausibly cut this substantially. The monadic operations alone account for 4 operations x 4 ref-qualified overloads across the three templates — dozens of method definitions that share structural similarity. ### The argument for the current approach @@ -53,7 +53,7 @@ The total is ~4300 lines of template code in a single header. A factored impleme - **Maintenance burden.** A bug in `and_then` must be fixed in three templates. A new monadic operation (hypothetical `or_transform`) must be added 12 times (4 overloads x 3 templates). - **Consistency risk.** Are all three templates actually consistent, across both object and reference `E`? Small divergences creep in when hand-copying constraint clauses. The only way to verify is exhaustive side-by-side comparison. -- **Review fatigue.** A reviewer looking at 4300 lines of structurally similar code will inevitably skim. The 95th `requires` clause gets less scrutiny than the 5th. +- **Review fatigue.** A reviewer looking at 3,413 lines of structurally similar code will inevitably skim. The 95th `requires` clause gets less scrutiny than the 5th. ### What to decide @@ -79,7 +79,7 @@ P2988 settled this for `optional` after years of debate. JeanHeyd Meneide's ### What to discuss anyway -- **User expectation.** C++ programmers coming from `std::reference_wrapper` expect assign-through (that's what `reference_wrapper::operator=` does). This will surprise some users. +- **Wrapper precedent.** `std::reference_wrapper::operator=` also rebinds. The remaining wrapper drawbacks are `.get()` friction, wrapper identity in generic interfaces, and adaptation before monadic callables receive `T&` directly. - **`emplace` also rebinds.** `e.emplace(new_ref)` rebinds, which is the only sensible behavior, but it differs from `expected::emplace(args...)` which constructs in-place. The name `emplace` is arguably misleading for reference types since nothing is being "emplaced" — a pointer is being reassigned. - **Swap rebinds both sides.** Two `expected` objects swap their pointers, not the values they point to. This is consistent with rebind semantics but will surprise users who think of references as aliases. @@ -159,7 +159,7 @@ Every deleted operation carries a C++26 `= delete("message")` string explaining ### What to discuss -- **C++26 feature.** `= delete("message")` is not available in C++23 or earlier. The README claims C++17+ support. This feature silently degrades — compilers that don't support it treat it as plain `= delete` — but the claim of C++17+ compatibility deserves scrutiny against the actual minimum language version needed for the rest of the code (`requires` clauses, concepts, `constexpr` union, etc.). +- **C++26 feature.** `= delete("message")` is not available in C++23 or earlier. The project targets C++20 and uses a macro that emits a diagnostic message only when `__cpp_deleted_function` advertises support, otherwise spelling a plain deleted function. - **Message quality.** Are the messages actionable? Do they guide the user to the correct alternative? Review each message for clarity and accuracy. - **Is this the right mechanism?** An alternative is `static_assert(false, "message")` inside a constrained-away-but-still-instantiable template. That would work in C++23 but is arguably worse. The `= delete("message")` approach is cleaner and should be preferred if C++26 is truly the floor. @@ -204,16 +204,16 @@ std::move(e).and_then(f); // f still gets int&, not int&& ### What exists -- 15 positive test files with 469 Catch2 `TEST_CASE` entries -- 54 negative compile tests (`_fail.cpp`) each with a `PASS_REGULAR_EXPRESSION` regex +- 19 positive test files with 688 Catch2 `TEST_CASE` entries +- 59 negative compile tests (`_fail.test.cpp`) each with a `PASS_REGULAR_EXPRESSION` regex - Separate constraint test files for primary and `T&` specializations - Hardened precondition tests under `BEMAN_EXPECTED_HARDENED` ### Gaps to discuss -- **No constraint test files for `expected`, `expected`, or `expected`.** The primary template has `expected_constraints.test.cpp` and `expected` has `expected_ref_constraints.test.cpp`. The other three reference specializations have no dedicated constraint test file. Their SFINAE behavior is untested under `static_assert(!is_constructible_v<...>)` patterns. -- **No monadic constraint tests for reference specializations.** `expected_monadic_constraints.test.cpp` covers only the primary and void specializations with move-only error types. The same constraints apply to reference specializations but are untested. -- **No triviality tests for reference specializations.** `expected_trivial.test.cpp` covers only primary and void. Reference specializations (especially `T&, E&` and `void, E&`, which are pointer-only) should be trivially copyable, movable, and destructible unconditionally. This is not verified. +- **Reference-error constraints are distributed.** `expected`, `expected`, and `expected` assertions live in their behavioral files and `expected_review_corrections.test.cpp`, rather than dedicated constraint files. Review both locations together. +- **Monadic constraints remain concentrated.** `expected_monadic_constraints.test.cpp` is the systematic matrix; the reference behavioral files add specialization-specific checks. Confirm any new monadic constraint is represented in both relevant layers. +- **Triviality is configuration-dependent.** Pointer-only `T&`/`E&` combinations have direct trait assertions. `expected` still inherits object-`E` properties, so it is not unconditionally trivial. - **Monadic tests are embedded in general test files** rather than having dedicated `_monadic.test.cpp` files for reference specializations. This makes it harder to verify completeness. ### What to decide @@ -230,7 +230,7 @@ Everything is in `expected.hpp`. The `unexpected.hpp` and `bad_expected_access.h ### What to discuss -- **4400 lines in one file.** This is at the boundary of manageable. A reviewer can `grep` and navigate, but side-by-side comparison of specializations requires tooling. +- **3,413 lines in one file.** This is at the boundary of manageable. A reviewer can `grep` and navigate, but side-by-side comparison of class-template forms requires tooling. - **The standard doesn't mandate file structure.** But for a reference implementation intended to demonstrate proposed wording, would splitting `expected_ref.hpp` (or similar) improve reviewability? - **Compile times.** Every translation unit that includes `expected.hpp` parses all three templates. For users who only need `expected`, this is wasted work. The module build path (`expected.cppm`) mitigates this but is opt-in and not the default. diff --git a/docs/llm-code-review-guide.md b/docs/llm-code-review-guide.md index 527f394..dd1456a 100644 --- a/docs/llm-code-review-guide.md +++ b/docs/llm-code-review-guide.md @@ -1,7 +1,7 @@ # LLM Code Review Guide: `beman::expected` **Target Audience:** High-context Large Language Models acting as rigorous C++ code reviewers. -**Scope:** The entire `include/beman/expected/` header set — one 4400-line header (`expected.hpp`) plus two small companions (`unexpected.hpp`, `bad_expected_access.hpp`). +**Scope:** The entire `include/beman/expected/` header set — one 3,413-line header (`expected.hpp`) plus two small companions (`unexpected.hpp`, `bad_expected_access.hpp`). The suite has 19 positive test files with 688 Catch2 `TEST_CASE`s and 59 negative compile tests. ## Mission @@ -17,7 +17,7 @@ You are reviewing a proposed **C++29** reference implementation of `std::expecte | Standard (org-mode) | `docs/standard/expected.org` | Same content, machine-parseable | | Standard (plain text) | `docs/standard/expected.txt` | Same content, grep-friendly | | Conformance audit | `docs/conformance-audit.md` | Every clause checked with PASS/EXT/FIXED status | -| P2988 (optional\) | External: open-std.org/P2988 | Rebind semantics design source for reference specializations | +| P2988R12 (optional\) | [wg21.link/P2988R12](https://wg21.link/P2988R12) | Rebind semantics design source for reference specializations | | beman/optional reference impl | `~/src/steve-downey/optional/main/` | Pattern source for T& storage and dangling prevention | ### What You Have No Normative Source For @@ -38,8 +38,8 @@ This means: for the primary template and void specialization, you can check clau Before reading line-by-line, establish the shape: -1. **Count specializations.** There should be exactly 6: ``, ``, ``, ``, ``, ``. Each should have complete API surface (constructors, assignment, observers, swap, equality, monadic ops). -2. **Identify the storage model** for each specialization. Primary uses union of T and E. Void uses union of E only. Reference specializations store pointers. Verify no specialization accidentally uses a union member for a reference type. +1. **Count class-template forms.** There should be exactly 3: primary ``, partial specialization ``, and partial specialization ``. Allowing `E` to be an lvalue reference yields the six value/error configurations without three additional class definitions. +2. **Identify the storage model** for each form. Every error arm is `unexpected`; it owns an object for object `E` and stores a pointer for reference `E`. The `T&` form stores its value as a pointer. Verify no union directly contains a reference. 3. **Map the helper utilities.** `reinit_expected` (lines 65-84) handles destroy-and-reconstruct for value-type transitions. `reference_constructs_from_temporary_v` (lines 88-100) is the dangling-prevention concept. Verify these are used correctly and only where applicable. ### Phase 2: Clause-by-Clause for Primary and Void @@ -86,7 +86,7 @@ Each reference specialization should offer the **same user-facing operations** a | unexpect_t ctor | yes | yes | yes | yes | yes | | in_place_t ctor | yes | **deleted** | yes | **deleted** | N/A (void) | | Value assignment | yes | rebind | yes | yes | N/A | -| unexpected assignment | yes | yes | **deleted** | **deleted** | N/A | +| unexpected assignment | yes | yes | from `` only | from `` only | from `` only | | emplace | yes | rebind | yes | rebind | emplace() (void) | | operator* | T& | T& | T& | T& | void | | operator-> | T* | T* | T* | T* | N/A | @@ -99,7 +99,7 @@ Each reference specialization should offer the **same user-facing operations** a Verify each cell. Pay special attention to the **deleted** entries — each should have a `= delete("message")` with a clear diagnostic and a corresponding negative compile test. -For the `from only` cells: construction from `unexpected` is permitted for reference `E` **only when `G` is itself a reference** (`unexpected`, which holds a pointer to an external object). Construction from a value-typed `unexpected` stays `= delete`d (it would dangle), and rebinding *assignment* from `unexpected` is not offered for reference `E`. Verify: `is_constructible_v, unexpected>` is true, `is_constructible_v, unexpected>` is false, and `unexpected` → `int&` is rejected (const drop). +For the `from only` cells: construction and rebinding assignment from `unexpected` are permitted for reference `E` **only when `G` is itself a reference** (`unexpected`, which holds a pointer to an external object). The value-typed form stays deleted because it would dangle. Verify construction and assignment traits for the accepted reference case, rejected value case, and rejected const-dropping case. #### 2. Dangling Prevention @@ -112,10 +112,10 @@ For every constructor or assignment that accepts a forwarding reference where th #### 3. Storage Layout -- `expected`: pointer `T*` plus union `{ E unex_; }` plus `bool has_val_` -- `expected`: value `T` in union plus `E*` pointer plus `bool has_val_` -- `expected`: two pointers `T*`, `E*` plus `bool has_val_` -- `expected`: pointer `E*` plus `bool has_val_` +- `expected`: union of `T*` and `unexpected` plus `bool has_val_` +- `expected`: union of `T` and pointer-holding `unexpected` plus `bool has_val_` +- `expected`: union of `T*` and pointer-holding `unexpected` plus `bool has_val_` +- `expected`: pointer-holding `unexpected` plus `bool has_val_` Verify: - No union contains a reference or pointer where the active member tracking could be wrong @@ -141,7 +141,7 @@ Every function in this implementation should be `constexpr`. Verify: #### Trivial Special Member Functions -The primary and void specializations must be trivially copyable/movable/destructible when T and E are. Reference specializations (pointer-based) should be trivially everything unconditionally. Verify: +The primary and void forms must be trivially copyable/movable/destructible when their stored alternatives permit it. Pointer-only `T&`/`E&` configurations should be trivial, while `expected` still follows object-`E` properties. Verify: - `= default` paths exist for trivial cases - The conditional dispatch between trivial and non-trivial paths is correct - Reference specializations don't accidentally have non-trivial destructors diff --git a/docs/optional_references.md b/docs/optional_references.md deleted file mode 100644 index ab780e0..0000000 --- a/docs/optional_references.md +++ /dev/null @@ -1,638 +0,0 @@ -# Changes Since Last Version - -- **Changes since R10** - - - Wording changes from LWG review. - -- **Changes since R9** - - - Fix cast in wording removing base/derived UB - -# Comparison table - -## Using a raw pointer result for an element search function - -This is the convention the C++ core guidelines suggest, to use a raw pointer for representing optional non-owning references. However, there is a user-required check against , no type safety meaning no safety against mis-interpreting such a raw pointer, for example by using pointer arithmetic on it. - -## returning result of an element search function via a (smart) pointer - -The disadvantage here is that is both non-standard and not well named, therefore this example uses that would have the advantage of avoiding dangling through potential lifetime extension. However, on the downside is still the explicit checks against the on the client side, failing so risks undefined behavior. - -## returning result of an element search function via an iterator - -This might be the obvious choice, for example, for associative containers, especially since their iterator stability guarantees. However, returning such an iterator will leak the underlying container type as well necessarily requires one to know the sentinel of the container to check for the not-found case. - -## Using an optional\ as a substitute for optional\ - -This approach adds another level of indirection and requires two checks to take a definite action. - -# Motivation - -Other than the standard library’s implementation of optional, optionals holding references are common. The desire for such a feature is well understood, and many optional types in commonly used libraries provide it, with the semantics proposed here. One standard library implementation already provides an implementation of but disables its use, because the standard forbids it. - -The research in JeanHeyd Meneide’s \_References for Standard Library Vocabulary Types - an optional case study.\_ shows conclusively that rebind semantics are the only safe semantic as assign through on engaged is too bug-prone. Implementations that attempt assign-through are abandoned. The standard library should follow existing practice and supply an that rebinds on assignment. - -Additional background reading on can be found in JeanHeyd Meneide’s article \_To Bind and Loose a Reference\_ . - -In freestanding environments or for safety-critical libraries, an optional type over references is important to implement containers, that otherwise as the standard library either would cause undefined behavior when accessing an non-available element, throw an exception, or silently create the element. Returning a plain pointer for such an optional reference, as the core guidelines suggest, is a non-type-safe solution and doesn’t protect in any way from accessing an non-existing element by a de-reference. In addition, the monadic APIs of makes is especially attractive by streamlining client code receiving such an optional reference, in contrast to a pointer that requires an explicit nullptr check and de-reference. - -There is a principled reason not to provide a partial specialization over as the semantics are in some ways subtly different than the primary template. Assignment may have side-effects not present in the primary, which has pure value semantics. However, I argue this is misleading, as reference semantics often has side-effects. The proposed semantic is similar to what an provides, with much greater usability. - -There are well motivated suggestions that perhaps instead of an there should be an that is an independent primary template. This proposal rejects that, because we need a policy over all sum types as to how reference semantics should work, as optional is a variant over T and monostate. That the library sum type can not express the same range of types as the product type, tuple, is an increasing problem as we add more types logically equivalent to a variant. The template types and should behave as extensions of and , or we lose the ability to reason about generic types. - -That we can’t guarantee from (product type) that (sum type) is valid, is a problem, and one that reflection can’t solve. A language sum type could, but we need agreement on the semantics. - -The semantics of a variant with a reference are as if it holds the address of the referent when referring to that referent. All other semantics are worse. Not being able to express a variant\ is inconsistent, hostile, and strictly worse than disallowing it. - -Thus, we expect future papers to propose and with the ability to hold references. The latter can be used as an iteration type over elements. - -# Design - -The design is straightforward. The holds a pointer to the underlying object of type , or if the optional is disengaged. The implementation is simple, especially with C++20 and up techniques, using concept constraints. As the held pointer is a primitive regular type with reference semantics, many operations can be defaulted and are by nature. See and . The implementation is less than 200 lines of code, much of it the monadic functions with identical textual implementations with different signatures and different overloads being called. - -In place construction is not supported as it would just be a way of providing immediate life-time issues. - -## Relational Operations - -The definitions of the relational operators are the same as for the base template. Interoperable comparisons between T and optional\ work as expected. This is not true for the boost optional\. - -## make_optional - -With further research, the existing uses of make_optional\ seem to be primarily test cases, and deliberate use seems to be exceedingly rare in the wild. Reflector review was much more positive about removing the misleading ability to create an via . In addition, the multiple argument forms can be used to attempt to construct a optional that contains a reference, but this becomes ill formed because of existing mandates at the type level. In order to preserve existing behavior, where make_optional is not well formed if it constructs a reference, changes to should be made. - -Adding a non-type template parameter as the first template parameter to the single argument and mandating that the multi-argument version not request a reference type as the parameter, will diagnose mistaken use of and preserve the existing behavior. - -Since construction of an object in order to make a reference to it to construct an optional containing a reference would always dangle, there do not seem to be any use cases for the multi-argument or initializer list forms of make_optional for reference types, and the constructor form seems to satisfy all cases for single argument construction of a optional containing a reference, there does not seem to be a need for a factory function for optional over reference. - -There was also discussion of using to indicate reference use, in analogy with std::tuple. Unfortunately there are existing uses of optional over reference_wrapper as a workaround for lack of reference specialization, and it would be a breaking change for such code. - -## Trivial construction - -Construction of should be trivial, because it is straightforward to implement, and is trivial. Boost is not. - -## Value Category Affects value() - -For several implementations there are distinct overloads for functions depending on value category, with the same implementation. However, this makes it very easy to accidentally steal from the underlying referred to object. Value category should be shallow. Thanks to many people for pointing this out. If “Deducing ” had been used, the problem would have been much more subtle in code review. - -## Shallow vs Deep const - -There is some implementation divergence in optionals about deep const for . That is, can the referred to be modified through a . Does return an or a , and does return an or a . I believe it is overall more defensible if the is shallow as it would be for a where the constness of the struct ref does not affect if the p pointer can be written through. This is consistent with the rebinding behavior being proposed. - -Where deeper constness is desired, would prevent non const access to the underlying object. - -## Conditional Explicit - -As in the base template, is made conditional on the type used to construct the optional. . This is not present in boost::optional, leading to differences in construction between braced initialization and = that can be surprising. - -## value_or - -After extensive discussion, it seems there is no particularly wonderful solution for that does not involve a time machine. Implementations of optionals that support reference semantics diverge over the return type, and the current one is arguably wrong, and should use something based on , which of course did not exist when was standardized. - -The weak consensus is to return a from as this is least likely to cause issues. There was at least one strong objection to this choice, but all other choices had more objections. The author intends to propose free functions , , , and over all types modeling optional-like, , in the next revision of . This would cover , , and pointer types. - -Having return by value also allows the common case of using a literal as the alternative to be expressed concisely. - -## in_place_t construction - -The reference specialization allows a limited form of in_place construction where the argument can be converted to the appropriate reference without creation of a temporary. As the reference specialization is non-owning, there is no “place” for a temporary to be constructed that will not dangle. For cases where the lifetime of the constructed object would match the lifetime of the optional, the temporary can be constructed explicitly, instead. - -## Converting assignment - -A similarly limited converting assignment operator is provided for cases where an optional\ has a value or refers to a value which can be converted to a T& without construction of a temporary. In particular, converting an optional\ to an optional\ is supported. - -## Compiler Explorer Playground - -See for an updated playground with relevant Google Test functions and various optional implementations made available for cross reference including a flattened in-place version of the reference implementation. - -# Principles for Reification of Design - -Optional must never construct a temporary, or knowingly take the address of an temporary or part of an temporary. - -It is always presumed safe to copy the pointer value from an optional, since by induction, it is not dangling. - -Optional has no storage, so should never construct a T, it may convert a U to a T, so long as that conversion does not create a temporary. - -Constructors that would convert from temporary are marked deleted. They should be sufficiently constrained that it was the correct choice and there is no more general, less constrained constructor that would not have created a dangling pointer. - -Failure to compile either by ambiguity or no eligible constructors in the overload set is preferable to optional being responsible for use after free or dangling. - -Assignment is always from an optional, which may have been an implicit construction. The assignment cannot throw, the construction/conversion may. The assignment may therefore need annotation converting the rhs if that constructor was explicit. This must not be necessary in the default case of creating an optional reference to an lvalue of the same type. - -The model for the constraints and mandates for is taken from over reference types. The type takes the most care of types in the standard library in dealing with creation of temporaries. - -As is designed to be converting, to create instances from arguments that can be used to create the underlying type, constructors should be explicit only where the operations used to create the pointer or the notional reference would be or are explicit. - -## Construction from temporary - -We disallow construction of from any type U in which: - -- the constructor body will create a temporary and bind it to a reference. - -- a const lvalue reference would be bound to rvalue. - -An example of the first case would be construction from . These cases always dangle. - -An example of the second case would be a construction from temporary . - -Prohibiting the second case does prevent some safe uses of the optional as the function parameter. - -Given: - -This will make a invocation ill-formed, despite the arg being safe to use from within the function body. - -This deviates from the design of the “view” parameters type, like or . However, we believe that this is the right choice due to the following: - -- Only a subset of cases would be working. As an illustration the very similar invocation is ill-formed, due to always being dangling. - -- Such design leads to the detection of reference to temporaries or local variables when is used as the return type. - - | | | - |:----|------------------------------------------------------------------------------------------------------------------------------------------------------------------------:| - | | \> getValue() std::string localString; return localString; // Ill-formed. std::optional\ localOptionalString; return localOptionalString; // ill-formed | - - One of the main motivational examples of is return from a lookup function, and eliminating dangling in such cases outweighs parameter cases. - - We are very grateful to Arthur O’Dwyer for his work on P2266R3 Simpler implicit move accepted in C++23, which makes it possible to implement this correctly. - -- We provide behavior consistent with , that disallows binding to xvalues. We believe that is closer in spirit to than any view type. It certainly shares some of the features. - -## Deleting dangling overloads - -To achieve the dangling safety expressed before, the constructor is marked deleted if it would lead to binding of the reference to temporary or the xvalue. However, deleted constructors are still considered to be candidates during overload resolution, leading to ambiguity in the following examples: - -During the reflector discussion, an option of an alternate design was presented, where the dangling overload would be constrained, and eliminated from the overload set. - -We strongly oppose changing this behavior, as: - -- We think that it is impossible to detect temporary binding to xvalue in such a design. - -- The behavior we propose is consistent with the behavior for optional for object types - - [TABLE] - -As language in general treats functions accepting by value and by const reference in the same manner during overload resolution, we believe achieving this consistency is a feature. - -The design that was introduced by , and , for references, is followed, where the detection of dangling does not affect the results of overload resolution and instead makes a call that would dangle be ill-formed and diagnosed. - -## Assignment of optional\ - -In the case of , any assignment operation is equivalent to assigning a pointer, and there is no observable difference between: using converting assignment from or constructing temporary , and then assigning it to it. - -This observation allows us to provide only copy-assignment for , instead of a set of converting assignments, that would need to replicate the signatures of constructors and their constraints. Assignment from any other value is handled by first implicitly constructing and then using copy-assignment. Move-assignment is the same as copy-assignment, since only pointer copy is involved. - -## Copy and Assignment of optional\&& to optional\ - -Care must be take to prevent the assignment of a movable optional to disallow the copy or assignment of the underlying referred to value to be stolen. The assignment or copy constructor should be used instead, which also needs to check slightly different constraints for and for testing . We thank Jan Kokemüller for uncovering this bug. The bug seems to be present in many optional implementations that support references. - -# Proposal - -Add an lvalue reference specialization for the std::optional template. - -# Wording - -The wording here cross references and adopts the wording in . The proposed changes are relative to the current working draft . - -# Impact on the standard - -A pure library extension, affecting no other parts of the library or language. - -# Acknowledgments - -Many thanks to all of the reviewers and authors of beman.optional, , in particular A. Jiang, Darius Neațu, David Sankel, Eddie Nolan, Jan Kokemüller, Jeff Garland, and River (Xueqing) Wu. Tomasz Kamiński provided extensive support for the library wording of optional\. - -# Document history - -- **Changes since R8** - - - Fix move/assign optional\ allowing stealing of referenced U - -- **Changes since R7** - - - Wording mandates/constraint fixes - - - Hash on T& pulled out - - - Notes on wording rendering - - - “Fix” make_optional\ - -- **Changes since R6** - - - strike refref specialization - - - add converting assignment operator - - - add converting in place constructor - -- **Changes since R5** - - - refref specialization - - - fix monadic constraints on base template - -- **Changes since R4** - - - feature test macro - - - value_or updates from P3091 - -- **Changes since R3** - - - make_optional discussion - always value - - - value_or discussion - always value - -- **Changes since R1** - - - Design points called out - -- **Changes since R0** - - - Wording Updates - -# Implementation - -``` c++ -// ---------------------- -// BASE AND DETAILS ELIDED -// ---------------------- - -/****************/ -/* optional */ -/****************/ - -template -class optional { - public: - using value_type = T; - using iterator = - std::contiguous_iterator; // see [optionalref.iterators] - public: - // \ref{optionalref.ctor}, constructors - - constexpr optional() noexcept = default; - constexpr optional(nullopt_t) noexcept : optional() {} - constexpr optional(const optional& rhs) noexcept = default; - - template - requires(std::is_constructible_v && - !std::reference_constructs_from_temporary_v) - constexpr explicit optional(in_place_t, Arg&& arg); - - template - requires(std::is_constructible_v && - !(std::is_same_v, in_place_t>) && - !(std::is_same_v, optional>) && - !std::reference_constructs_from_temporary_v) - constexpr explicit(!std::is_convertible_v) - optional(U&& u) noexcept(std::is_nothrow_constructible_v) { - convert_ref_init_val(u); - } - - template - requires(std::is_constructible_v && - !(std::is_same_v, in_place_t>) && - !(std::is_same_v, optional>) && - std::reference_constructs_from_temporary_v) - constexpr optional(U&& u) = delete; - - // The full set of 4 overloads on optional by value category, doubled to - // 8 by deleting if reference_constructs_from_temporary_v is true. This - // allows correct constraints by propagating the value category from the - // optional to the value within the rhs. - template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - !std::reference_constructs_from_temporary_v) - constexpr explicit(!std::is_convertible_v) optional( - optional& rhs) noexcept(std::is_nothrow_constructible_v); - - template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - !std::reference_constructs_from_temporary_v) - constexpr explicit(!std::is_convertible_v) - optional(const optional& rhs) noexcept( - std::is_nothrow_constructible_v); - - template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - !std::reference_constructs_from_temporary_v) - constexpr explicit(!std::is_convertible_v) - optional(optional&& rhs) noexcept( - noexcept(std::is_nothrow_constructible_v)); - - template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - !std::reference_constructs_from_temporary_v) - constexpr explicit(!std::is_convertible_v) - optional(const optional&& rhs) noexcept( - noexcept(std::is_nothrow_constructible_v)); - - template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - std::reference_constructs_from_temporary_v) - constexpr optional(optional& rhs) = delete; - - template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - std::reference_constructs_from_temporary_v) - constexpr optional(const optional& rhs) = delete; - - template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - std::reference_constructs_from_temporary_v) - constexpr optional(optional&& rhs) = delete; - - template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - std::reference_constructs_from_temporary_v) - constexpr optional(const optional&& rhs) = delete; - - // \ref{optionalref.dtor}, destructor - constexpr ~optional() = default; - - // \ref{optionalref.assign}, assignment - constexpr optional& operator=(nullopt_t) noexcept; - - constexpr optional& operator=(const optional& rhs) noexcept = default; - - template - requires(std::is_constructible_v && - !std::reference_constructs_from_temporary_v) - constexpr T& - emplace(U&& u) noexcept(std::is_nothrow_constructible_v); - - // \ref{optionalref.swap}, swap - constexpr void swap(optional& rhs) noexcept; - - // \ref{optional.iterators}, iterator support - constexpr iterator begin() const noexcept; - constexpr iterator end() const noexcept; - - // \ref{optionalref.observe}, observers - constexpr T* operator->() const noexcept; - constexpr T& operator*() const noexcept; - constexpr explicit operator bool() const noexcept; - constexpr bool has_value() const noexcept; - constexpr T& value() const; - template > - constexpr std::remove_cv_t value_or(U&& u) const; - - // \ref{optionalref.monadic}, monadic operations - template - constexpr auto and_then(F&& f) const; - template - constexpr optional> transform(F&& f) const; - template - constexpr optional or_else(F&& f) const; - - // \ref{optional.mod}, modifiers - constexpr void reset() noexcept; - - private: - T* value_ = nullptr; // exposition only - - // \ref{optionalref.expos}, exposition only helper functions - template - constexpr void convert_ref_init_val(U&& u) { - // Creates a variable, \tcode{r}, - // as if by \tcode{T\& r(std::forward(u));} - // and then initializes \exposid{val} with \tcode{addressof(r)} - T& r(std::forward(u)); - value_ = std::addressof(r); - } -}; - -// \rSec3[optionalref.ctor]{Constructors} -template -template - requires(std::is_constructible_v && - !std::reference_constructs_from_temporary_v) -constexpr optional::optional(in_place_t, Arg&& arg) { - convert_ref_init_val(std::forward(arg)); -} - -// Clang is unhappy with the out-of-line definition -// -// template -// template -// requires(std::is_constructible_v && -// !(is_same_v, in_place_t>) && -// !(is_same_v, optional>) && -// !std::reference_constructs_from_temporary_v) -// constexpr optional::optional(U&& u) -// noexcept(is_nothrow_constructible_v) -// : value_(std::addressof(static_cast(std::forward(u)))) {} - -template -template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - !std::reference_constructs_from_temporary_v) -constexpr optional::optional(optional& rhs) noexcept( - std::is_nothrow_constructible_v) { - if (rhs.has_value()) { - convert_ref_init_val(*rhs); - } -} - -template -template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - !std::reference_constructs_from_temporary_v) -constexpr optional::optional(const optional& rhs) noexcept( - std::is_nothrow_constructible_v) { - if (rhs.has_value()) { - convert_ref_init_val(*rhs); - } -} - -template -template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - !std::reference_constructs_from_temporary_v) -constexpr optional::optional(optional&& rhs) noexcept( - noexcept(std::is_nothrow_constructible_v)) { - if (rhs.has_value()) { - convert_ref_init_val(*std::move(rhs)); - } -} - -template -template - requires(std::is_constructible_v && - !std::is_same_v, optional> && - !std::is_same_v && - !std::reference_constructs_from_temporary_v) -constexpr optional::optional(const optional&& rhs) noexcept( - noexcept(std::is_nothrow_constructible_v)) { - if (rhs.has_value()) { - convert_ref_init_val(*std::move(rhs)); - } -} - -// \rSec3[optionalref.assign]{Assignment} -template -constexpr optional& optional::operator=(nullopt_t) noexcept { - value_ = nullptr; - return *this; -} - -template -template - requires(std::is_constructible_v && - !std::reference_constructs_from_temporary_v) -constexpr T& -optional::emplace(U&& u) noexcept(std::is_nothrow_constructible_v) { - convert_ref_init_val(std::forward(u)); - return *value_; -} - -// \rSec3[optionalref.swap]{Swap} - -template -constexpr void optional::swap(optional& rhs) noexcept { - std::swap(value_, rhs.value_); -} - -// \rSec3[optionalref.iterators]{Iterator Support} - -template -constexpr optional::iterator optional::begin() const noexcept { - return iterator(has_value() ? value_ : nullptr); -}; - -template -constexpr optional::iterator optional::end() const noexcept { - return begin() + has_value(); -} - -// \rSec3[optionalref.observe]{Observers} -template -constexpr T* optional::operator->() const noexcept { - return value_; -} - -template -constexpr T& optional::operator*() const noexcept { - return *value_; -} - -template -constexpr optional::operator bool() const noexcept { - return value_ != nullptr; -} -template -constexpr bool optional::has_value() const noexcept { - return value_ != nullptr; -} - -template -constexpr T& optional::value() const { - return has_value() ? *value_ : throw bad_optional_access(); -} - -template -template -constexpr std::remove_cv_t optional::value_or(U&& u) const { - static_assert(std::is_constructible_v, T&>, - "T must be constructible from a T&"); - static_assert(std::is_convertible_v>, - "Must be able to convert u to T"); - return has_value() ? *value_ - : static_cast>(std::forward(u)); -} - -// \rSec3[optionalref.monadic]{Monadic operations} -template -template -constexpr auto optional::and_then(F&& f) const { - using U = std::invoke_result_t; - static_assert(detail::is_optional, "F must return an optional"); - if (has_value()) { - return std::invoke(std::forward(f), *value_); - } else { - return std::remove_cvref_t(); - } -} - -template -template -constexpr optional> -optional::transform(F&& f) const { - using U = std::invoke_result_t; - static_assert(!std::is_same_v, in_place_t>, - "Result must not be in_place_t"); - static_assert(!std::is_same_v, nullopt_t>, - "Result must not be nullopt_t"); - static_assert((std::is_object_v && !std::is_array_v) || - std::is_lvalue_reference_v, - "Result must be an non-array object or an lvalue reference"); - if (has_value()) { - return optional{std::invoke(std::forward(f), *value_)}; - } else { - return optional{}; - } -} - -template -template -constexpr optional optional::or_else(F&& f) const { - using U = std::invoke_result_t; - static_assert(std::is_same_v, optional>, - "Result must be an optional"); - if (has_value()) { - return *value_; - } else { - return std::forward(f)(); - } -} - -// \rSec3[optional.mod]{modifiers} -template -constexpr void optional::reset() noexcept { - value_ = nullptr; -} -} // namespace beman::optional - -namespace std { -template - requires requires(T a) { - { - std::hash>{}(a) - } -> std::convertible_to; - } -struct hash> { - static_assert(!is_reference_v, - "hash is not enabled for reference types"); - size_t operator()(const beman::optional::optional& o) const - noexcept(noexcept(hash>{}(*o))) { - if (o) { - return std::hash>{}(*o); - } else { - return 0; - } - } -}; -``` diff --git a/docs/plan/review-remediation-2026-09-02.md b/docs/plan/review-remediation-2026-09-02.md new file mode 100644 index 0000000..98e845b --- /dev/null +++ b/docs/plan/review-remediation-2026-09-02.md @@ -0,0 +1,93 @@ +# Review Remediation Plan + +## Objective + +Resolve every confirmed code, proposal, documentation, citation, packaging, and +CI issue found in the 2026-09-02 repository review. Preserve +`docs/review-findings-2026-08-06.md` unchanged. + +## Implementation + +1. Reject initializer-list error construction whenever `E` is a reference in + all three `expected` class-template forms. Apply matching constraints to + declarations and definitions, provide a clear deleted-overload diagnostic, + and update proposal wording so the unsafe form is explicitly unavailable. +2. Replace the reserved placeholder `__cpp_lib_expected_ref` with the public + Beman-owned macro `BEMAN_EXPECTED_HAS_REFERENCES`, defined as `1` through + shared public configuration. Document that macro detection is available to + header consumers; module support itself is unconditional. +3. Add a global module fragment that includes `` before + `export module beman.expected`, ensuring feature-test-dependent declarations + such as constexpr exception support match textual-header mode. +4. Preserve the current type-based rejection of xvalues and the current + conservative exception specifications for reference-error assignment. Add + rationale and tests rather than changing those semantics. + +## Proposal and Citations + +1. Correct every statement claiming `std::reference_wrapper::operator=` assigns + through. Describe its actual rebinding semantics and retain only the valid + objections: `.get()` friction, wrapper type identity, and monadic adaptation. +2. Correct the tuple/variant contradiction: tuple and pair support references; + variant does not. +3. Change illustrative error storage from raw `E` to `unexpected`. +4. Specify both `error_or` overloads with the non-reference error value type, + matching the implementation. +5. Replace the claimed working-draft `static_assert` with the actual + ill-formed-program requirement and remove the statement that included edits + arrive in a later revision. +6. Explain that type-based dangling detection intentionally rejects xvalues + because expression provenance cannot be recovered from the constructor's + deduced types. +7. Set the paper date to `2026-09-02`. +8. Label D4270R0 as an unpublished draft and pin its citation to commit + `5060f1e5c22db1e36db0bdb270e4e0efdcbd575d`. +9. Reflow wording and code blocks until significant overfull-box warnings are + removed without changing normative meaning. + +## Documentation and Infrastructure + +1. Reconcile README platform guidance by retaining Windows setup information + while clearly marking MSVC builds unsupported and unverified. Repair the + malformed installed-tree code fence. +2. Rewrite install-test instructions for `beman.expected`, install all + components by omitting `--component`, and set the consumer project to C++20. +3. Replace GoogleTest references with Catch2 and remove the invalid C++17 + contributing example. +4. Remove the developer-local GoogleTest override from the root Makefile and + point its paper target at the current `papers` directory. +5. Remove the nonexistent `/papers/P2988` Dependabot entry. +6. Add CodeQL `c-cpp` analysis using a manual existing CMake preset build while + retaining GitHub Actions analysis. +7. Refresh both review guides to describe three `expected` class-template + forms, `unexpected` storage, current assignment behavior, 3,405 + implementation lines, 19 positive test files, 688 Catch2 `TEST_CASE`s, and + 56 negative compile tests. +8. Delete `docs/optional_references.md` and replace any in-repository links to + it with stable P2988 references. + +## Tests and Acceptance Criteria + +1. Add negative compile tests for initializer-list reference errors in + `expected`, `expected`, and `expected`. +2. Add positive/constraint assertions proving initializer-list construction for + object error types remains available. +3. Add conditional header/module parity coverage for constexpr-exception + declarations where supported. +4. Add tests that lock down prvalue and xvalue rejection and the existing + conservative `noexcept` results. +5. Require GCC and Clang debug builds to pass all existing and new tests. +6. Require a clean install and external consumer test from a temporary prefix. +7. Build the paper with no unresolved citations or significant overfull boxes. +8. Run `clang-format --dry-run`, `gersemi --check`, `codespell`, Markdown/link + checks, and workflow syntax validation. +9. Confirm final repository status contains only intended changes plus the + untouched pre-existing untracked review document. + +## Public Interface Effects + +- Remove `__cpp_lib_expected_ref`. +- Add `BEMAN_EXPECTED_HAS_REFERENCES` with value `1`. +- Make initializer-list error constructors ill-formed for reference error + types; these calls were previously accepted but always unsafe. +- Make no other ownership, layout, ABI, rebinding, or monadic semantic changes. diff --git a/include/beman/expected/config.hpp b/include/beman/expected/config.hpp index bfaf5ac..ab9fc01 100644 --- a/include/beman/expected/config.hpp +++ b/include/beman/expected/config.hpp @@ -3,6 +3,8 @@ #ifndef BEMAN_EXPECTED_CONFIG_HPP #define BEMAN_EXPECTED_CONFIG_HPP +#define BEMAN_EXPECTED_HAS_REFERENCES 1 + #if !defined(__has_include) || __has_include() #include #else diff --git a/include/beman/expected/expected.cppm b/include/beman/expected/expected.cppm index e4b0820..8f0e8d0 100644 --- a/include/beman/expected/expected.cppm +++ b/include/beman/expected/expected.cppm @@ -1,3 +1,7 @@ +module; + +#include + export module beman.expected; import std; diff --git a/include/beman/expected/expected.hpp b/include/beman/expected/expected.hpp index 6119005..f4c0ad1 100644 --- a/include/beman/expected/expected.hpp +++ b/include/beman/expected/expected.hpp @@ -23,13 +23,6 @@ #define BEMAN_EXPECTED_TRAP() std::abort() #endif -// Feature-test macro for the reference-E / reference-T extensions (expected, -// expected, expected) implemented by this header. Not yet a WG21-assigned -// macro; the value below is a placeholder pending standardization. -#ifndef __cpp_lib_expected_ref - #define __cpp_lib_expected_ref 202608L // placeholder value pending WG21 assignment -#endif - /*** 22.8.2 Header synopsis[expected.syn] @@ -290,9 +283,14 @@ class expected { // In-place constructor for error with initializer_list template - requires std::is_constructible_v&, Args...> + requires(!std::is_reference_v && std::is_constructible_v&, Args...>) constexpr explicit expected(unexpect_t, std::initializer_list il, Args&&... args); + template + requires std::is_reference_v + constexpr expected(unexpect_t, std::initializer_list, Args&&...) = BEMAN_EXPECTED_DELETE_MSG( + "expected: initializer-list error construction cannot bind a reference; pass an lvalue reference"); + // ------------------------------------------------------------------------- // [expected.object.dtor] Destructor // ------------------------------------------------------------------------- @@ -701,7 +699,7 @@ constexpr expected::expected(unexpect_t, Args&&... args) : has_val_(false) template template - requires std::is_constructible_v&, Args...> + requires(!std::is_reference_v && std::is_constructible_v&, Args...>) constexpr expected::expected(unexpect_t, std::initializer_list il, Args&&... args) : has_val_(false) { std::construct_at(std::addressof(unex_), std::in_place, il, std::forward(args)...); } @@ -1528,9 +1526,14 @@ class expected { // In-place constructor for error with initializer_list template - requires std::is_constructible_v&, Args...> + requires(!std::is_reference_v && std::is_constructible_v&, Args...>) constexpr explicit expected(unexpect_t, std::initializer_list il, Args&&... args); + template + requires std::is_reference_v + constexpr expected(unexpect_t, std::initializer_list, Args&&...) = BEMAN_EXPECTED_DELETE_MSG( + "expected: initializer-list error construction cannot bind a reference; pass an lvalue reference"); + // Converting constructor from expected — reference-E path only. G is itself a // reference to an external object, so binding E& to it cannot dangle regardless of the // source's value category, provided the reference conversion itself does not materialize a @@ -1833,7 +1836,7 @@ constexpr expected::expected(unexpect_t, Args&&... args) : has_val_(fal template template - requires std::is_constructible_v&, Args...> + requires(!std::is_reference_v && std::is_constructible_v&, Args...>) constexpr expected::expected(unexpect_t, std::initializer_list il, Args&&... args) : has_val_(false) { std::construct_at(std::addressof(unex_), std::in_place, il, std::forward(args)...); } @@ -2510,9 +2513,14 @@ class expected { // In-place constructor for error with initializer_list template - requires std::is_constructible_v&, Args...> + requires(!std::is_reference_v && std::is_constructible_v&, Args...>) constexpr explicit expected(unexpect_t, std::initializer_list il, Args&&... args); + template + requires std::is_reference_v + constexpr expected(unexpect_t, std::initializer_list, Args&&...) = BEMAN_EXPECTED_DELETE_MSG( + "expected: initializer-list error construction cannot bind a reference; pass an lvalue reference"); + // ------------------------------------------------------------------------- // Destructor // ------------------------------------------------------------------------- @@ -2856,7 +2864,7 @@ constexpr expected::expected(unexpect_t, Args&&... args) : has_val_(false template template - requires std::is_constructible_v&, Args...> + requires(!std::is_reference_v && std::is_constructible_v&, Args...>) constexpr expected::expected(unexpect_t, std::initializer_list il, Args&&... args) : has_val_(false) { std::construct_at(std::addressof(unex_), std::in_place, il, std::forward(args)...); } diff --git a/include/beman/expected/unexpected.hpp b/include/beman/expected/unexpected.hpp index ab95efe..dbfc428 100644 --- a/include/beman/expected/unexpected.hpp +++ b/include/beman/expected/unexpected.hpp @@ -3,6 +3,8 @@ #ifndef BEMAN_EXPECTED_UNEXPECTED_HPP #define BEMAN_EXPECTED_UNEXPECTED_HPP +#include + #ifndef BEMAN_EXPECTED_INCLUDED_FROM_INTERFACE_UNIT #include #include diff --git a/installtest/CMakeLists.txt b/installtest/CMakeLists.txt index d7738b4..7d3c62a 100644 --- a/installtest/CMakeLists.txt +++ b/installtest/CMakeLists.txt @@ -2,21 +2,17 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception cmake_minimum_required(VERSION 3.27) -project(TestInstalledExpected) +project(TestInstalledExpected LANGUAGES CXX) -set(CMAKE_CXX_STANDARD 26) #current minimum C++ version +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_EXTENSIONS OFF) # Enable testing in this separate project enable_testing() # Find the installed package -set(EXPECTED_INSTALL_DIR "../.install/lib/cmake/beman.expected") -find_package( - beman.expected - REQUIRED - PATHS ${EXPECTED_INSTALL_DIR} - NO_DEFAULT_PATH -) +find_package(beman.expected REQUIRED) # Add the test executable add_executable(TestInstalledExpected test.cpp) diff --git a/installtest/README.md b/installtest/README.md index 72ffa94..b9e7fec 100644 --- a/installtest/README.md +++ b/installtest/README.md @@ -1,9 +1,10 @@ -# Test Project against installed `beman.optional` +# Test Project against installed `beman.expected` To test from the root of the source tree + ```sh cmake --workflow --preset gcc-release -cmake --install build/gcc-release --prefix .install --component beman.optional -cmake -S installtest -B installtest/build -cmake --build installtest/build --target test +cmake --install build/gcc-release --prefix .install +cmake -S installtest -B installtest/build -DCMAKE_PREFIX_PATH="$PWD/.install" +cmake --build installtest/build --target test ``` diff --git a/papers/D4280R0.tex b/papers/D4280R0.tex index 088f36e..e760ad5 100644 --- a/papers/D4280R0.tex +++ b/papers/D4280R0.tex @@ -19,7 +19,7 @@ \begin{flushright} \begin{tabular}{ll} Document \#: & D4280R0 \\ - Date: & \today \\ + Date: & 2026-09-02 \\ Project: & Programming Language C++ \\ Audience: & LEWG \end{tabular} @@ -119,11 +119,9 @@ \section{\tcode{expected} as a substitute} \section{\tcode{expected, E>}} -\tcode{std::reference_wrapper} closes the null hole but is verbose, wants -\tcode{.get()} everywhere the reference is used, and --- the part that -matters here --- assigns through on assignment, which is the behavior -\cite{P1683R0} found to be a bug source. It reintroduces the problem the -reference specialization is designed to remove. +\tcode{std::reference_wrapper} closes the null hole but is verbose, exposes the +wrapper rather than the reference as the value type, wants \tcode{.get()} where +the reference is used, and requires adapters when composing monadic operations. \begin{tabular}{ lr } \begin{minipage}[t]{0.45\columnwidth} @@ -216,14 +214,9 @@ \chapter{The proposal} \tcode{std::expected} \cite{P0323R12}, added in \CppXXIII{}, holds either a value of type \tcode{T} or an error of type \tcode{E}. Like \tcode{std::optional} before it, the primary template forbids reference types; -the working draft carries the static assertion - -\begin{minted}[fontsize=\small]{c++} -static_assert(!std::is_reference_v, - "T must not be a reference (use expected specialization)"); -\end{minted} - -whose comment already points at the answer. Reference values need a +the working draft instead requires that a program instantiating +\tcode{expected} use \tcode{void} or a complete non-array object type for +\tcode{T}; otherwise the program is ill-formed. Reference values need a specialization; reference errors do not. In full, the proposal is: \begin{itemize} @@ -313,8 +306,10 @@ \chapter{The proposal} constexpr E&& error() && noexcept; template> constexpr remove_cv_t value_or(U&&) const; - template - constexpr E error_or(G&&) const&; + template>> + constexpr remove_cv_t> error_or(G&&) const&; + template>> + constexpr remove_cv_t> error_or(G&&) &&; // monadic operations template @@ -362,15 +357,14 @@ \chapter{The proposal} bool @\exposid{has_val}@; // \expos union { T* @\exposid{val}@; // \expos - E @\exposid{unex}@; // \expos + unexpected @\exposid{unex}@; // \expos }; }; } \end{codeblock} -The reference implementation lives in the Beman Project's -\tcode{beman.expected} library \cite{Downey_beman_expected}, and is described -in \nameref{chap:impl}. +The Beman reference implementation is available online +\cite{Downey_beman_expected}. It is described in \nameref{chap:impl}. \chapter{Motivation} @@ -384,8 +378,8 @@ \chapter{Motivation} happy path, and nothing stops a null \tcode{T*} from being returned as a success. \item \tcode{expected, E>} works, but requires - \tcode{.get()} at every use, does not thread through the monadic API as a - reference would, and assigns through on assignment. + \tcode{.get()} at every use, exposes the wrapper type in the interface, and + does not thread through the monadic API as a reference would. \end{itemize} \tcode{expected} fills the gap. It participates in \tcode{and_then}, @@ -418,9 +412,10 @@ \chapter{Motivation} fallible sum type is not a special case that has earned an exception. \tcode{optional} is \tcode{variant}; \tcode{expected} is -\tcode{variant}. Neither \tcode{variant} nor \tcode{tuple} can hold a -reference today. That is a separate and larger defect, and it should not block -a feature that can be specified precisely now by following \tcode{optional}. +\tcode{variant}. \tcode{tuple} and \tcode{pair} can hold references; +\tcode{variant} cannot. The missing sum-type support is a separate and larger +defect, and it should not block a feature that can be specified precisely now +by following \tcode{optional}. \chapter{Prior art and precedent} @@ -458,14 +453,12 @@ \section{Rebind on assignment, never assign-through (D1)} e = std::expected(b); // e now refers to b; a is still 1 \end{minted} -There is a real objection: a programmer arriving from -\tcode{std::reference_wrapper} expects assign-through, because that is exactly -what \tcode{reference_wrapper::operator=} does. Granted. And the objection is -answered by the record: \cite{P2988R12} settled this for \tcode{optional}, -and the survey behind it \cite{P1683R0} is a list of assign-through -implementations that were abandoned. Rebind is the only semantics that is not a -latent bug. \tcode{emplace} rebinds too --- it is the only thing it can do --- -and \tcode{swap} exchanges the two pointers, not the two referents. +This matches \tcode{std::reference_wrapper::operator=}, which rebinds the +wrapper rather than assigning through to its referent. \cite{P2988R12} settled +the same question for \tcode{optional}, and the survey behind it +\cite{P1683R0} records assign-through implementations that were abandoned. +\tcode{emplace} rebinds too --- it is the only thing it can do --- and +\tcode{swap} exchanges the two pointers, not the two referents. \section{Shallow \tcode{const} (D2)} @@ -496,6 +489,12 @@ \section{Dangling constructors are deleted, with a diagnostic (D3)} message is seen. Both are correct rejections. They do not read identically, and a reviewer should expect both forms. +The detection is deliberately type-based. For a forwarding constructor, a +prvalue and an xvalue deduce the same source type; the source expression's +provenance is no longer available to the constraint. Accepting xvalues while +rejecting prvalues would therefore require information this interface cannot +recover, so both are conservatively rejected. + \section{References in the error position, by relaxation not specialization (D4)} The error type is a template parameter, and errors are routinely owned @@ -551,8 +550,8 @@ \section{\tcode{value\_or} returns by value; \tcode{error\_or} mirrors it (D6)} force the alternative to be a \tcode{T\&} or something convertible to one, which is too restrictive to be useful. \tcode{error_or} returns the error by value on the same reasoning. Callers who genuinely want a reference-returning fallback -have the free-function forms, \tcode{reference_or} \cite{D4270R0}, which work -over all the nullable types at once. +have the free-function forms, \tcode{reference_or}, from the unpublished draft +\cite{D4270R0}, which work over all the nullable types at once. For \tcode{expected} there is no value, so \tcode{value_or} is \tcode{= delete}d with a message rather than merely absent, so the error names @@ -593,31 +592,31 @@ \section{Shallow conversions must not steal (D9)} std::expected v{std::move(r)}; // copies; s == "bar" afterwards \end{minted} -The internal rule that secures this is move-the-wrapper-then-access --- -\tcode{*std::move(r)}, \tcode{std::move(r).error()} --- never -access-then-move. Deref-then-move, \tcode{std::move(*r)}, would force an rvalue -onto a reference the wrapper does not own, and steal from the caller's object. -Because it is a guarantee and not merely an implementation habit, it belongs in -the design. +The internal rule that secures this is move-the-wrapper-then-access. For +example, use \tcode{*std::move(r)} or \tcode{std::move(r).error()}. Never use +access-then-move: \tcode{std::move(*r)} would force an rvalue onto a reference +the wrapper does not own, and steal from the caller's object. Because it is a +guarantee and not merely an implementation habit, it belongs in the design. \section{Deleted functions carry messages, where the compiler allows it (D10)} -Every deleted operation is written to say what is wrong and what to do -instead --- for instance, ``\tcode{expected: no default constructor; -T\& cannot be null}.'' Deleted-function messages \cite{P2573R2} are -\CppXXVI{}; this proposal does not raise the library's floor to get them. A -macro emits \tcode{= delete("...")} when the compiler advertises support --- -tested via \tcode{__cpp_deleted_function} --- and falls back to a plain +Every deleted operation says what is wrong and what to do instead. For example, +the deleted default constructor explains that \tcode{T\&} cannot be null. +Deleted-function messages \cite{P2573R2} are \CppXXVI{}; this proposal does not +raise the library's floor to get them. A macro emits \tcode{= delete("...")} +when \tcode{__cpp_deleted_function} advertises support, and falls back to plain \tcode{= delete} otherwise. The reference implementation, and this proposal, -stay at \CppXX{}. For a feature whose whole difficulty is explaining why a -given construction is refused, a good diagnostic is worth having when it is -free, but not worth a floor. +stay at \CppXX{}. A good diagnostic is useful when free, but not worth a higher +language-version floor. -\section{Feature test macro (D11)} +\section{Reference-implementation detection macro (D11)} -A feature test macro \tcode{__cpp_lib_expected_ref} is proposed, so code can -detect the reference specializations. The reference implementation defines it -with a placeholder value, pending assignment by LWG on adoption. +The reference implementation defines the Beman-owned macro +\tcode{BEMAN_EXPECTED_HAS_REFERENCES} as \tcode{1} in its shared public +configuration. It is available to textual-header consumers; importing the +\tcode{beman.expected} module does not import macros, and module support is +unconditional. No reserved standard-library feature-test macro is claimed +before standardization assigns one. \section{The held \tcode{unexpected} is exposition-only; the observable behavior is keyed on \tcode{E} (D12)} @@ -653,6 +652,13 @@ \section{The held \tcode{unexpected} is exposition-only; the observable \tcode{T}-and-\tcode{E} rendering gives. The wrapper is how the error is described, never what the program is allowed to observe. +The reference implementation also preserves the existing conservative +exception specifications for assignment when \tcode{E} is a reference. Those +specifications remain expressed in terms of \tcode{E}, and can therefore report +that assignment is potentially throwing even though the implementation merely +rebinds a pointer. This avoids broadening the proposal with a second observable +semantic change; tests lock in that conservative result. + One implementation note, since it is not obvious and cost real debugging time. Declaring a copy or move constructor as two candidates, each guarded by its own \tcode{requires}-clause covering only the trivial and only the non-trivial case, @@ -690,9 +696,9 @@ \chapter{Anticipated objections} \section{``Use \tcode{reference\_wrapper} instead''} It works, and it is what people write today. But \tcode{.get()} litters every -use, it does not thread through the monadic API as a reference does, and -\tcode{reference_wrapper::operator=} assigns through --- reintroducing exactly -the bug D1 removes. The wrapper is the workaround the reference specialization +use, the wrapper remains visible in the type, and monadic composition needs +adaptation before a callable receives \tcode{T\&}. Its assignment correctly +rebinds; the other interface friction is what the reference specialization retires. \section{``References in sum types were banned for a reason''} @@ -712,10 +718,11 @@ \section{``\tcode{expected} is good enough''} \section{``Wait for \tcode{variant} and do it all at once''} -\tcode{variant} and \tcode{tuple} cannot hold references today, and fixing that -is a separate, larger job. \tcode{expected} can be specified precisely -right now by following \tcode{optional} line for line. A clean, ready -feature should not wait on an unrelated one. +\tcode{tuple} and \tcode{pair} can hold references today; \tcode{variant} +cannot. Extending \tcode{variant} is a separate, larger job. +\tcode{expected} can be specified precisely right now by following +\tcode{optional} line for line. A clean, ready feature should not wait on +an unrelated one. \chapter{Impact on the standard} @@ -744,8 +751,8 @@ \chapter{Wording} shallow-\tcode{const} observers. Marked edits in place widen the error type of the primary \tcode{expected} and of \tcode{expected} to admit an lvalue reference, and add \tcode{unexpected} with the reference-only -construction rule of D5. This revision establishes the baseline; the marked -edits land in the next. +construction rule of D5. The marked edits included here are the proposed +changes. \include{expected-new} diff --git a/papers/expected-new.tex b/papers/expected-new.tex index 20f0290..dd91327 100644 --- a/papers/expected-new.tex +++ b/papers/expected-new.tex @@ -165,6 +165,7 @@ \pnum \throws Any exception thrown by the initialization of \exposid{unex}. + \end{itemdescr} \rSec3[expected.un.obs]{Observers} @@ -579,8 +580,10 @@ constexpr E&& error() && noexcept; template> constexpr T value_or(U&&) const &; template> constexpr T value_or(U&&) &&; - template constexpr E error_or(G&&) const &; - template constexpr E error_or(G&&) &&; + template>> + constexpr remove_cv_t> error_or(G&&) const &; + template>> + constexpr remove_cv_t> error_or(G&&) &&; // \ref{expected.object.monadic}, monadic operations template constexpr auto and_then(F&& f) &; @@ -985,6 +988,7 @@ \begin{itemdescr} \pnum \constraints +\tcode{is_reference_v} is \tcode{false}, and \tcode{is_constructible_v\&, Args...>} is \tcode{true}. \pnum @@ -999,6 +1003,12 @@ \pnum \throws Any exception thrown by the initialization of \exposid{unex}. + +\pnum +\remarks +An overload with the same parameter types is defined as deleted when \tcode{E} +is an lvalue reference type. An initializer list cannot provide the required +long-lived error referent. \end{itemdescr} \rSec3[expected.object.dtor]{Destructor} @@ -1595,14 +1605,17 @@ \indexlibrarymember{error_or}{expected}% \begin{itemdecl} -template constexpr E error_or(G&& e) const &; +template>> + constexpr remove_cv_t> error_or(G&& e) const &; \end{itemdecl} \begin{itemdescr} \pnum \mandates -\tcode{is_copy_constructible_v} is \tcode{true} and -\tcode{is_convertible_v} is \tcode{true}. +\tcode{is_copy_constructible_v>>} is +\tcode{true} and +\tcode{is_convertible_v>>} is +\tcode{true}. \pnum \returns @@ -1612,14 +1625,17 @@ \indexlibrarymember{error_or}{expected}% \begin{itemdecl} -template constexpr E error_or(G&& e) &&; +template>> + constexpr remove_cv_t> error_or(G&& e) &&; \end{itemdecl} \begin{itemdescr} \pnum \mandates -\tcode{is_move_constructible_v} is \tcode{true} and -\tcode{is_convertible_v} is \tcode{true}. +\tcode{is_move_constructible_v>>} is +\tcode{true} and +\tcode{is_convertible_v>>} is +\tcode{true}. \pnum \returns @@ -2020,8 +2036,10 @@ constexpr E& error() & noexcept; constexpr const E&& error() const && noexcept; constexpr E&& error() && noexcept; - template constexpr E error_or(G&&) const &; - template constexpr E error_or(G&&) &&; + template>> + constexpr remove_cv_t> error_or(G&&) const &; + template>> + constexpr remove_cv_t> error_or(G&&) &&; // \ref{expected.void.monadic}, monadic operations template constexpr auto and_then(F&& f) &; @@ -2280,6 +2298,7 @@ \begin{itemdescr} \pnum \constraints +\tcode{is_reference_v} is \tcode{false}, and \tcode{is_constructible_v\&, Args...>} is \tcode{true}. \pnum @@ -2294,6 +2313,12 @@ \pnum \throws Any exception thrown by the initialization of \exposid{unex}. + +\pnum +\remarks +An overload with the same parameter types is defined as deleted when \tcode{E} +is an lvalue reference type. An initializer list cannot provide the required +long-lived error referent. \end{itemdescr} \rSec3[expected.void.dtor]{Destructor} @@ -2625,14 +2650,17 @@ \indexlibrarymember{error_or}{expected}% \begin{itemdecl} -template constexpr E error_or(G&& e) const &; +template>> + constexpr remove_cv_t> error_or(G&& e) const &; \end{itemdecl} \begin{itemdescr} \pnum \mandates -\tcode{is_copy_constructible_v} is \tcode{true} and -\tcode{is_convertible_v} is \tcode{true}. +\tcode{is_copy_constructible_v>>} is +\tcode{true} and +\tcode{is_convertible_v>>} is +\tcode{true}. \pnum \returns @@ -2642,14 +2670,17 @@ \indexlibrarymember{error_or}{expected}% \begin{itemdecl} -template constexpr E error_or(G&& e) &&; +template>> + constexpr remove_cv_t> error_or(G&& e) &&; \end{itemdecl} \begin{itemdescr} \pnum \mandates -\tcode{is_move_constructible_v} is \tcode{true} and -\tcode{is_convertible_v} is \tcode{true}. +\tcode{is_move_constructible_v>>} is +\tcode{true} and +\tcode{is_convertible_v>>} is +\tcode{true}. \pnum \returns @@ -3017,7 +3048,10 @@ constexpr const E&& error() const && noexcept; constexpr E&& error() && noexcept; template> constexpr remove_cv_t value_or(U&&) const; - template constexpr E error_or(G&&) const &; + template>> + constexpr remove_cv_t> error_or(G&&) const &; + template>> + constexpr remove_cv_t> error_or(G&&) &&; // \ref{expected.ref.monadic}, monadic operations template constexpr auto and_then(F&& f) &; @@ -3187,11 +3221,15 @@ \begin{itemdescr} \pnum \constraints -If \tcode{E} is an lvalue reference type, \tcode{G} is an lvalue reference -type, \tcode{is_convertible_v} is \tcode{true}, and -\tcode{reference_constructs_from_temporary_v} is \tcode{false}; -otherwise \tcode{is_constructible_v} (for the first overload) or -\tcode{is_constructible_v} (for the second) is \tcode{true}. +If \tcode{E} is an lvalue reference type, all of the following are true: +\begin{itemize} +\item \tcode{G} is an lvalue reference type, +\item \tcode{is_convertible_v} is \tcode{true}, and +\item \tcode{reference_constructs_from_temporary_v} is \tcode{false}. +\end{itemize} +Otherwise, \tcode{is_constructible_v} is \tcode{true} for the +first overload, or \tcode{is_constructible_v} is \tcode{true} for the +second overload. \pnum \effects @@ -3218,6 +3256,7 @@ \pnum \constraints \tcode{is_constructible_v} (respectively, +\tcode{is_reference_v} is \tcode{false} and \tcode{is_constructible_v\&, Args...>}) is \tcode{true}. If \tcode{E} is an lvalue reference type, the initialization shall not bind \tcode{E} to a temporary. @@ -3229,6 +3268,12 @@ \tcode{il}, and \tcode{std::forward(args)...}), so that the error \tcode{\exposid{unex}.error()} is constructed from those arguments. \tcode{has_value()} is \tcode{false}. + +\pnum +\remarks +The initializer-list overload is defined as deleted when \tcode{E} is an +lvalue reference type. An initializer list cannot provide the required +long-lived error referent. \end{itemdescr} \rSec3[expected.ref.dtor]{Destructor} @@ -3288,9 +3333,13 @@ \begin{itemdescr} \pnum \constraints -\tcode{remove_cvref_t} is not \tcode{expected} or a specialization of -\tcode{unexpected}, \tcode{is_constructible_v} is \tcode{true}, and -\tcode{reference_constructs_from_temporary_v} is \tcode{false}. +All of the following are true: +\begin{itemize} +\item \tcode{remove_cvref_t} is neither \tcode{expected} nor a + specialization of \tcode{unexpected}, +\item \tcode{is_constructible_v} is \tcode{true}, and +\item \tcode{reference_constructs_from_temporary_v} is \tcode{false}. +\end{itemize} \pnum \effects @@ -3447,8 +3496,10 @@ \pnum \throws -\tcode{bad_expected_access(as_const(error()))} (respectively, -\tcode{bad_expected_access(std::move(error()))}) if \tcode{has_value()} is +For the first overload, +\tcode{bad_expected_access(as_const(error()))} if \tcode{has_value()} is +\tcode{false}. For the second overload, +\tcode{bad_expected_access(std::move(error()))} if \tcode{has_value()} is \tcode{false}. Both overloads yield \tcode{T\&}; the value category of \tcode{*this} does not affect the returned reference. \end{itemdescr} @@ -3479,29 +3530,37 @@ \end{itemdecl} \begin{itemdescr} +\pnum +Let \tcode{X} be \tcode{remove_cv_t}. + \pnum \mandates -\tcode{is_convertible_v>} and -\tcode{is_convertible_v>} are \tcode{true}. +\tcode{is_convertible_v} and \tcode{is_convertible_v} are +\tcode{true}. \pnum \returns -\tcode{has_value() ? static_cast>(*\exposid{val}) : -static_cast>(std::forward(v))}. The result is an object, -never a reference. +\begin{codeblock} +has_value() + ? static_cast(*@\exposid{val}@) + : static_cast(std::forward(v)) +\end{codeblock} +The result is an object, never a reference. \end{itemdescr} \indexlibrarymember{error_or}{expected}% \begin{itemdecl} -template - constexpr E error_or(G&& e) const &; +template>> + constexpr remove_cv_t> error_or(G&& e) const &; +template>> + constexpr remove_cv_t> error_or(G&& e) &&; \end{itemdecl} \begin{itemdescr} \pnum \returns -\tcode{has_value() ? std::forward(e) : error()}, as for the primary -template. +Each overload behaves as the corresponding overload for the primary template. +The result is an object, never a reference. \end{itemdescr} \rSec3[expected.ref.monadic]{Monadic operations} diff --git a/papers/mybiblio.bib b/papers/mybiblio.bib index 2e4cea0..0e1f96d 100644 --- a/papers/mybiblio.bib +++ b/papers/mybiblio.bib @@ -1,9 +1,9 @@ @misc{D4270R0, author = {Downey, Stephen}, title = {Free Value Or Else}, -howpublished = {\url{https://github.com/steve-downey/free_value_or}}, +howpublished = {\url{https://github.com/steve-downey/free_value_or/tree/5060f1e5c22db1e36db0bdb270e4e0efdcbd575d}}, year = {2026}, -note = {D4270R0} +note = {Unpublished draft D4270R0, commit 5060f1e5c22db1e36db0bdb270e4e0efdcbd575d} } @misc{Downey_beman_expected, diff --git a/tests/beman/expected/CMakeLists.txt b/tests/beman/expected/CMakeLists.txt index e2960f9..f3368f2 100644 --- a/tests/beman/expected/CMakeLists.txt +++ b/tests/beman/expected/CMakeLists.txt @@ -63,6 +63,22 @@ target_link_libraries( ) catch_discover_tests(beman.expected.tests.expected) +if(BEMAN_EXPECTED_USE_MODULES) + add_library(beman.expected.tests.module_parity OBJECT) + target_sources( + beman.expected.tests.module_parity + PRIVATE header_idempotence.test.cpp + ) + target_compile_definitions( + beman.expected.tests.module_parity + PRIVATE BEMAN_EXPECTED_TEST_MODULE + ) + target_link_libraries( + beman.expected.tests.module_parity + PRIVATE beman::expected + ) +endif() + # --- std::expected parity gate --------------------------------------------- # Compile the behavioral tests against std::expected (requires C++23). No # behavioral difference from beman::expected is expected except those that @@ -319,6 +335,10 @@ add_fail_test(expected_ref_e_t_unexpected_fail expected_ref_e_t_unexpected_fail.test.cpp "T must not be a specialization of unexpected" ) +add_fail_test(expected_ref_e_initializer_list_fail + expected_ref_e_initializer_list_fail.test.cpp + "initializer-list error construction cannot bind a reference|use of deleted function|call to deleted" +) # Step 8 — expected monadic mandates add_fail_test(expected_ref_e_and_then_wrong_error_type_fail @@ -359,6 +379,10 @@ add_fail_test(expected_ref_both_inplace_value_fail expected_ref_both_inplace_value_fail.test.cpp "no in_place value constructor|use of deleted function|call to deleted|attempting to reference" ) +add_fail_test(expected_ref_both_initializer_list_fail + expected_ref_both_initializer_list_fail.test.cpp + "initializer-list error construction cannot bind a reference|use of deleted function|call to deleted" +) # Step 9 — expected monadic mandates add_fail_test(expected_ref_both_and_then_wrong_error_type_fail @@ -391,6 +415,10 @@ add_fail_test(expected_void_ref_e_assign_unexpected_fail expected_void_ref_e_assign_unexpected_fail.test.cpp "no assignment from unexpected|use of deleted function|call to deleted|invokes a deleted function|selected deleted operator|attempting to reference" ) +add_fail_test(expected_void_ref_e_initializer_list_fail + expected_void_ref_e_initializer_list_fail.test.cpp + "initializer-list error construction cannot bind a reference|use of deleted function|call to deleted" +) # ============================================================================= # Hardened precondition tests (compiled with -DBEMAN_EXPECTED_HARDENED) diff --git a/tests/beman/expected/expected_ref_both_initializer_list_fail.test.cpp b/tests/beman/expected/expected_ref_both_initializer_list_fail.test.cpp new file mode 100644 index 0000000..270479a --- /dev/null +++ b/tests/beman/expected/expected_ref_both_initializer_list_fail.test.cpp @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#include + +using namespace beman::expected; + +expected value(unexpect, {1, 2, 3}); diff --git a/tests/beman/expected/expected_ref_e.test.cpp b/tests/beman/expected/expected_ref_e.test.cpp index 77be6a6..184def1 100644 --- a/tests/beman/expected/expected_ref_e.test.cpp +++ b/tests/beman/expected/expected_ref_e.test.cpp @@ -23,12 +23,12 @@ using beman::expected::testing::type_name; // Presence of the macro is still a hard translation failure: without it there // is nothing for a test to inspect, and `#if` is the only tool that can ask. -#ifndef __cpp_lib_expected_ref - #error "__cpp_lib_expected_ref must be defined by " +#ifndef BEMAN_EXPECTED_HAS_REFERENCES + #error "BEMAN_EXPECTED_HAS_REFERENCES must be defined by " #endif -TEST_CASE("expected: feature-test macro has a positive value", "[expected_ref_e]") { - CHECK(__cpp_lib_expected_ref > 0); +TEST_CASE("expected: feature-test macro has the expected value", "[expected_ref_e]") { + CHECK(BEMAN_EXPECTED_HAS_REFERENCES == 1); } // ============================================================================= diff --git a/tests/beman/expected/expected_ref_e_initializer_list_fail.test.cpp b/tests/beman/expected/expected_ref_e_initializer_list_fail.test.cpp new file mode 100644 index 0000000..3c16c15 --- /dev/null +++ b/tests/beman/expected/expected_ref_e_initializer_list_fail.test.cpp @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#include + +using namespace beman::expected; + +expected value(unexpect, {1, 2, 3}); diff --git a/tests/beman/expected/expected_review_corrections.test.cpp b/tests/beman/expected/expected_review_corrections.test.cpp index 29fea32..eeb4e45 100644 --- a/tests/beman/expected/expected_review_corrections.test.cpp +++ b/tests/beman/expected/expected_review_corrections.test.cpp @@ -11,9 +11,66 @@ #include #include +#include +#include using namespace beman::expected; +namespace { +template +concept accepts_value_prvalue = requires { Ex(42); }; + +template +concept accepts_value_xvalue = requires(int& value) { Ex(std::move(value)); }; + +template +concept accepts_error_prvalue = requires { Ex(unexpect, 42); }; + +template +concept accepts_error_xvalue = requires(int& error) { Ex(unexpect, std::move(error)); }; +} // namespace + +// The type-level properties below are checked at runtime rather than with +// static_assert so that a violated rule is reported by the test run, with the +// offending specialization named, instead of stopping the build at the first +// failure and reporting nothing. + +TEST_CASE("initializer-list error construction is unavailable for reference errors", "[ref][init-list]") { + // Available for object errors, in all three class templates. + CHECK(std::is_constructible_v>, unexpect_t, std::initializer_list>); + CHECK(std::is_constructible_v>, unexpect_t, std::initializer_list>); + CHECK(std::is_constructible_v>, unexpect_t, std::initializer_list>); + + // Unavailable for reference errors: a reference cannot bind to a list. + CHECK_FALSE(std::is_constructible_v, unexpect_t, std::initializer_list>); + CHECK_FALSE(std::is_constructible_v, unexpect_t, std::initializer_list>); + CHECK_FALSE(std::is_constructible_v, unexpect_t, std::initializer_list>); +} + +TEST_CASE("dangling detection is type-based: prvalue and xvalue are both rejected", "[ref][dangling]") { + // A forwarding constructor sees the same deduced type for a prvalue and an + // xvalue, so rejecting one rejects the other. + CHECK_FALSE(accepts_value_prvalue>); + CHECK_FALSE(accepts_value_xvalue>); + CHECK_FALSE(accepts_error_prvalue>); + CHECK_FALSE(accepts_error_xvalue>); + CHECK_FALSE(accepts_error_prvalue>); + CHECK_FALSE(accepts_error_xvalue>); + CHECK_FALSE(accepts_error_prvalue>); + CHECK_FALSE(accepts_error_xvalue>); +} + +TEST_CASE("reference-error assignment keeps conservative exception specifications", "[ref][noexcept][assign]") { + // Rebinding the stored pointer cannot throw, but the exception + // specifications deliberately stay conservative. + CHECK_FALSE(std::is_nothrow_copy_assignable_v>); + CHECK_FALSE(std::is_nothrow_move_assignable_v>); + CHECK_FALSE(std::is_nothrow_copy_assignable_v>); + CHECK_FALSE(std::is_nothrow_move_assignable_v>); + CHECK_FALSE(std::is_nothrow_copy_assignable_v>); + CHECK_FALSE(std::is_nothrow_move_assignable_v>); +} + // ============================================================================= // F6 (Option B) — construction from unexpected for reference E. // diff --git a/tests/beman/expected/expected_void_ref_e_initializer_list_fail.test.cpp b/tests/beman/expected/expected_void_ref_e_initializer_list_fail.test.cpp new file mode 100644 index 0000000..11e072a --- /dev/null +++ b/tests/beman/expected/expected_void_ref_e_initializer_list_fail.test.cpp @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#include + +using namespace beman::expected; + +expected value(unexpect, {1, 2, 3}); diff --git a/tests/beman/expected/header_idempotence.test.cpp b/tests/beman/expected/header_idempotence.test.cpp index c7702c6..55e31d8 100644 --- a/tests/beman/expected/header_idempotence.test.cpp +++ b/tests/beman/expected/header_idempotence.test.cpp @@ -7,11 +7,26 @@ // here when those files were parameterized over the implementation under test // (see test_expected.hpp). Nothing to run at runtime — success is compilation. -#include -#include +#if defined(BEMAN_EXPECTED_TEST_MODULE) + #include -#include -#include +import beman.expected; +#else + #include + #include -#include -#include + #include + #include + + #include + #include +#endif + +#if defined(__cpp_lib_constexpr_exceptions) +consteval bool bad_expected_access_is_constexpr() { + beman::expected::bad_expected_access exception(42); + return exception.error() == 42; +} + +static_assert(bad_expected_access_is_constexpr()); +#endif