Skip to content

Migrate ext::optional → std::optional and remove Boost optional shim#6

Open
tobydrinkall wants to merge 1 commit into
masterfrom
devin/1780670475-migrate-ext-optional-to-std
Open

Migrate ext::optional → std::optional and remove Boost optional shim#6
tobydrinkall wants to merge 1 commit into
masterfrom
devin/1780670475-migrate-ext-optional-to-std

Conversation

@tobydrinkall
Copy link
Copy Markdown

@tobydrinkall tobydrinkall commented Jun 5, 2026

Summary

Completes the highest-priority modernization: replaces all ext::optional/ext::nullopt with std::optional/std::nullopt and removes the now-dead QL_USE_STD_OPTIONAL build flag. This eliminates the correctness landmine in ql/optional.hpp that #errors on Boost ≥ 1.91 due to silent pricing-behavior changes in boost::optional.

Why this shim first: optional sits on public, price-affecting APIs — Settings::includeTodaysCashFlows_ (controls NPV inclusion) and Event::hasOccurred(Date, optional<bool>) (controls accrual logic). A wrong empty-state semantics silently misprices.

What changed (93 files, ~79 source files + config/docs)

  • Source migration (mechanical): ext::optional<T>std::optional<T>, ext::nulloptstd::nullopt, #include <ql/optional.hpp>#include <optional> across all ql/ and test-suite/ .hpp/.cpp files.
  • Tombstoned ql/optional.hpp: reduced to a deprecated #include <optional> (matches functional.hpp / tuple.hpp pattern).
  • Removed QL_USE_STD_OPTIONAL from: CMakeLists.txt, CMakePresets.json, configure.ac, ql/userconfig.hpp, ql/config.hpp.cfg, .ci/userconfig{2019,2022,2026}.alt.hpp, Docs/pages/config.docs.
  • Updated AGENTS.md and Docs/pages/modernization.docs to reflect the completed migration and point to the next shim (any, then shared_ptr).

Blast radius by layer

Layer Files
Core (settings, event, cashflow) 6
cashflows/* 8
instruments/* 21
pricingengines/* 15
termstructures/* 11
experimental/*, time/* 10
test-suite/* 9

Densest consumer: ql/cashflows/cashflows.cpp (44 ext::optionalstd::optional replacements).

Verification (local — CI is disabled on this fork)

  • Build: 100% clean (cmake --build build -j$(nproc))
  • Test suite: 100% tests passed, 0 failed (288s)
  • Grep proof: zero remaining ext::optional, ext::nullopt, boost::optional, or QL_USE_STD_OPTIONAL in source files

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


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)
Open in Devin Review

Replace all ext::optional<T> with std::optional<T> and ext::nullopt with
std::nullopt across 79 source files. Replace #include <ql/optional.hpp>
with #include <optional>.

Tombstone ql/optional.hpp to a thin deprecated #include <optional>
(matching the pattern of functional.hpp and tuple.hpp).

Remove the QL_USE_STD_OPTIONAL build flag from CMakeLists.txt,
CMakePresets.json, configure.ac, ql/userconfig.hpp, ql/config.hpp.cfg,
.ci/userconfig*.alt.hpp, and Docs/pages/config.docs.

Update AGENTS.md and Docs/pages/modernization.docs to reflect the
completed migration.

This is the highest-priority modernization because ql/optional.hpp
carried a #error on Boost >= 1.91 due to silent pricing-correctness
breakage, and optional appeared on public price-affecting APIs
(Settings::includeTodaysCashFlows_, Event::hasOccurred).

Local verification: 100% build, 100% tests passed (0 failed).

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 5 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