Skip to content

Remove ext::optional shim — migrate to std::optional#7

Open
tobydrinkall wants to merge 2 commits into
masterfrom
devin/1780676112-migrate-ext-optional-to-std
Open

Remove ext::optional shim — migrate to std::optional#7
tobydrinkall wants to merge 2 commits into
masterfrom
devin/1780676112-migrate-ext-optional-to-std

Conversation

@tobydrinkall
Copy link
Copy Markdown

@tobydrinkall tobydrinkall commented Jun 5, 2026

Summary

Eliminates the ql/optional.hpp Boost compatibility shim — the one with the proven silent-correctness landmine (#error on Boost ≥ 1.91 because boost::optional silently changed pricing behavior). All 80 source files now use std::optional / std::nullopt directly via #include <optional>.

Commit 1 — Mechanical migration across 80 files:

ext::optional<T>  →  std::optional<T>
ext::nullopt      →  std::nullopt
#include <ql/optional.hpp>  →  #include <optional>

Files that previously got ext::optional transitively now #include <optional> directly.

Commit 2 — Remove the shim infrastructure:

  • ql/optional.hpp → thin deprecated #include <optional> (backward compat)
  • QL_USE_STD_OPTIONAL removed from CMakeLists.txt, CMakePresets.json, configure.ac, ql/userconfig.hpp, ql/config.hpp.cfg, .ci/userconfig*.alt.hpp, Docs/pages/config.docs
  • Docs/pages/modernization.docs and AGENTS.md updated to reflect completion

Verification (local — CI is disabled on this fork)

Gate Result
Full clean build (cmake -B build … && cmake --build build -j$(nproc)) ✓ 100%
Full regression suite (ctest --test-dir build, 1 321 test cases) 100% tests passed, 0 failed (313 s)
Numeric equivalence harness (6 examples: EquityOption, Bonds, BermudanSwaption, FRA, MulticurveBootstrapping, CDS) Byte-identical output vs master
grep for leftover ext::optional / ext::nullopt / boost::optional Zero matches in source

QL_USE_STD_OPTIONAL was already defaulted ON, so std::optional was already in use at runtime — this migration makes the source reflect that and removes the dead Boost fallback path.

This is step 1 of the recommended shim removal order per Docs/pages/modernization.docs: optional → any → functional/tuple → shared_ptr.

Link to Devin session: https://app.devin.ai/sessions/6cd4407d1a444c899fd3e01ef6375072
Requested by: @tobydrinkall


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)
Open in Devin Review

devin-ai-integration Bot and others added 2 commits June 5, 2026 16:20
Mechanical replacement across 80 source files:
- ext::optional<T> -> std::optional<T>
- ext::nullopt -> std::nullopt
- #include <ql/optional.hpp> -> #include <optional>
- Added #include <optional> to files that got ext:: transitively

QL_USE_STD_OPTIONAL was already ON by default, so std::optional was
already in use at runtime. This commit makes the source reflect that.

Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
- ql/optional.hpp: reduced to thin deprecated #include <optional>
  (ext:: namespace aliases removed; the Boost fallback path and
  #error guard are gone)
- QL_USE_STD_OPTIONAL removed from: CMakeLists.txt, CMakePresets.json,
  configure.ac, ql/userconfig.hpp, ql/config.hpp.cfg,
  .ci/userconfig*.alt.hpp, Docs/pages/config.docs
- Docs/pages/modernization.docs updated to reflect completion
- AGENTS.md updated: optional migration marked done, next shims listed

Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
@devin-ai-integration
Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant