fix: address repository review findings

.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:

.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

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,15 +65,15 @@ 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:
 
 ```shell
 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:
 

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

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
 <details>
 <summary> For Windows </summary>
 
-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.
 
 </details>
 
@@ -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 <identity.hpp>`) 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,11 +308,14 @@ 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
     └── cmake
         └── beman.expected
             ├── beman.expected-config.cmake
             ├── beman.expected-config-version.cmake
-            └── beman.expected-targets.cmake```
+            └── beman.expected-targets.cmake
+```

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<T&, E>` specialization for a reference value, a relaxation of the primary and `void` templates to admit a reference error `E`, and a new `unexpected<E&>` — three `expected` class templates in all, each accepting an object or reference error. The reference semantics follow P2988 (`optional<T&>`): 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<T&>` 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<T, E>::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<T&, E>` 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<T, E&>`, `expected<T&, E&>`, or `expected<void, E&>`.** The primary template has `expected_constraints.test.cpp` and `expected<T&, E>` 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<T, E&>`, `expected<T&, E&>`, and `expected<void, E&>` 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<T&, E>` 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<T, E>`, this is wasted work. The module build path (`expected.cppm`) mitigates this but is opt-in and not the default.