Skip to content

Deduce the expected operand of the comparison against a value - #382

Merged
Bronek merged 1 commit into
mainfrom
bronek/fix_expected_eq_recursion
Jul 25, 2026
Merged

Deduce the expected operand of the comparison against a value#382
Bronek merged 1 commit into
mainfrom
bronek/fix_expected_eq_recursion

Conversation

@Bronek

@Bronek Bronek commented Jul 25, 2026

Copy link
Copy Markdown
Member

Fixes #381pfn::expected's comparison against a value constrained itself on the other operand while spelling its own operand through the policy, a non-deduced context. Deduction could therefore reject nothing, so the constraint was evaluated for every left operand in the program; where the right operand reached the same operator by ADL, satisfaction depended on itself, which both compilers reject outright.

Verified on main before the fix, gcc 16.1.1 and clang 22.1.6:

using E = pfn::expected<P, C>;
using G = E (*)(int);          // a function pointer whose ADL reaches E

template <typename A, typename B>
concept eq = requires(A const &a, B const &b) { a == b; };

static_assert(not eq<P, G>);   // hard error: "satisfaction ... depends on itself"

and at the fn layer, where it was reachable through any class template over such a type — comparing fn::just<E (*)(int)>, or merely asking whether it compares, was a hard error.

The change

The operator is moved out of _expected_base to namespace scope, once per carrier (pfn for pfn::expected, fn for fn::expected), where its operand is deduced from its own class template. A left operand which is not that expected then fails deduction, and the constraint is never reached. ADL still finds it: associated entities include base classes regardless of access, which is how both carriers reached the hidden friends through their private base.

Three things ruled out along the way, each for a concrete reason:

  • Keeping the hidden friend with a deduced Self parameter (requires same_as<Self, Policy::type<T, E>>) works on gcc and clang but MSVC rejects it with C2995: function template has already been defined — the parameter list no longer mentions the enclosing class's T/E/Policy, so every instantiation of _expected_base declares the same signature. gcc and clang tell them apart by the requires-clause, which [defns.signature.templ] includes through the template-head.
  • A member operator, which would take the expected as its implicit object parameter: both carriers inherit the storage base privately, so an inherited member is inaccessible.
  • Dropping the constraint, as [expected.object.eq] specifies (*x == v is a Mandates, not a Constraint): that is what makes the standard's own hidden friend safe, but it would make asking about an invalid comparison a hard error inside the body instead of an answer — and it would diverge from libstdc++, which constrains exactly as pfn does and pairs it with a deduced operand for exactly this reason.

The other three comparison operators are unchanged. They constrain on the operands' channels (*x == *y, x.error() == y.error()), never on the other operand's type, so they cannot ask a question that re-enters them. Measured after fixing this one alone, pfn and std::expected now agree on every row of the same battery — including the rows that must answer false:

ask before after std::expected
expected == same expected / other expected / value / unexpected / reversed 1 1 1
value == fn-pointer returning that expected hard error 0 0
convertible-to-value == fn-pointer returning that expected hard error 0 0
value == unrelated struct 0 0 0

Tests

  • tests/pfn/expected.cpp — a section pinning that a left operand which is not the expected answers, with the positive controls that the comparison it exists for still applies. This file is re-compiled against std::expected by expected_validation.cpp under VALIDATE_CXX23, so these pins assert that pfn and std answer identically.
  • tests/fn/just.cpp — the fn-layer witness: just<F> where F returns an expected now compares.

Verification

Six-config gate (gcc/clang × Debug/Release/validate) green, C++26 mode green (86/86), dev 43/43, and MSVC 17 green.

🤖 Generated with Claude Code

https://claude.ai/code/session_0198ikYYxtCqwNG54SS1b249

The operator constrains itself on the other operand, which is only safe where
its own operand is deduced: as a hidden friend it could be spelled only as
Policy::type<T, E>, a non-deduced context, so deduction rejected nothing and the
constraint ran for every left operand there is. Where that operand reached this
same operator by ADL, satisfaction depended on itself - a hard error, where the
question should simply answer false. Declared at namespace scope instead, once
per carrier; its three siblings constrain on the operands' channels rather than
on the other operand, and keep their form.

Closes #381

Assisted-by: Claude:claude-opus-5
@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@augmentcode

augmentcode Bot commented Jul 25, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR fixes a hard-error recursion bug in expected’s equality comparison against a value.

Changes:

  • Moves the value-comparison `operator==` out of the shared `_expected_base` hidden-friend set into namespace scope (per carrier: `pfn` and `fn`) so the `expected` operand is deduced.
  • Keeps the other three equality operators unchanged (they constrain on channel comparisons and don’t self-reenter).
  • Updates the changelog with the rationale and standards/libstdc++ alignment.
  • Adds `pfn::expected` tests to pin that non-`expected` left operands now yield a clean “not comparable” answer rather than a hard error.
  • Adds an `fn::just` regression test covering the ADL-reachable function-pointer case.

Technical Notes: The fix prevents constraints from being evaluated for unrelated left operands, avoiding self-dependent constraint satisfaction when ADL reaches the same operator.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@sonarqubecloud

Copy link
Copy Markdown

@Bronek

Bronek commented Jul 25, 2026

Copy link
Copy Markdown
Member Author

On record:

  1. Visibility widened slightly. A hidden friend is reachable only by ADL; a namespace-scope template in pfn/fn is also found by ordinary unqualified lookup inside those namespaces. It's constrained on expected<T, E> as its first parameter, so it can never be selected for anything else — the difference is where the name lives, not what any program means.
  2. The operator now exists twice, once per carrier, and someone editing one could let them drift. The answer is that de-duplicating it is precisely what caused pfn::expected comparison constraints can depend on themselves #381 : the shared form could only reach its own operand through Policy::type<T, E>. The fn copy says "for the reason given where its siblings are declared in pfn", which points a future editor at the note rather than at the duplication.

So we can have "hidden friend" (as mandated by standard) or "constrained on the other operand", but not both — at least not when the class is reached through a policy indirection that makes its own name unspellable. We choose the latter, because it provides better user experience. Note, libstdc++ also constraints rather than mandates on the other operand - so we are not alone.

@Bronek
Bronek merged commit d72b15c into main Jul 25, 2026
66 checks passed
Bronek added a commit that referenced this pull request Jul 25, 2026
The operator constrains itself on the other operand, which is only safe where
its own operand is deduced: as a hidden friend it could be spelled only as
Policy::type<T, E>, a non-deduced context, so deduction rejected nothing and the
constraint ran for every left operand there is. Where that operand reached this
same operator by ADL, satisfaction depended on itself - a hard error, where the
question should simply answer false. Declared at namespace scope instead, once
per carrier; its three siblings constrain on the operands' channels rather than
on the other operand, and keep their form.

Closes #381

Assisted-by: Claude:claude-opus-5
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.

pfn::expected comparison constraints can depend on themselves

1 participant