Skip to content

Reject binding reference holders to temporaries - #454

Merged
Bronek merged 12 commits into
mainfrom
bronek/dangling_references_from_ctor_fix
Oct 12, 2026
Merged

Bronek merged 12 commits into
mainfrom
bronek/dangling_references_from_ctor_fix

Conversation

@Bronek

@Bronek Bronek commented Oct 11, 2026

Copy link
Copy Markdown
Member

Addresses the pfn and pack parts of #447: optional<T&>, pack elements and invoke_r's reference result no longer bind to temporaries.

  • pfn::reference_constructs_from_temporary and pfn::reference_converts_from_temporary (new <pfn/type_traits.hpp>): use the compiler builtin where available; elsewhere (GCC 12, MSVC), an emulation that never reports a false temporary but misses one created by converting an unrelated class source.
  • pfn::optional<T&> and fn::optional<T&>: the [optional.ref.ctor] / [optional.ref.assign] guards replace the documented deferral.
  • pfn::invoke_r: not viable when its reference result would bind to a temporary ([func.require]/2); libc++ previously accepted such a call.
  • pack: append(std::in_place_type<T&>, ...) and as_pack with explicit element types reject a temporary; value elements still convert, including braced lists.

Breaking: as in the standard, xvalues of the referenced type are also rejected, e.g. optional<int const &>{std::move(x)}.

copack / choice reference alternatives (#451) and just<T&> (#437) adopt the traits after rebasing, so this does not close #447.

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

…om_temporary polyfills

The C++23 traits use the compiler builtin where __has_builtin reports it.
Elsewhere (GCC 12, MSVC) an emulation answers: it reports a temporary
only where the binding rules make one certain, because a false report
would reject valid code on those compilers alone. It misses a temporary
created by converting a class source not reference-related to the
referent, which the documentation states.

The tests also run against the standard traits in C++23 validation
builds, and test the emulation directly so it runs on every compiler.

Assisted-by: Claude:claude-opus-5-5
Implement the [optional.ref.ctor] and [optional.ref.assign] guards in
pfn::optional<T&> and fn::optional<T&>: each converting constructor has
a deleted twin selected where reference_constructs_from_temporary_v
holds, and the in_place_t constructor and emplace are constrained on it.
As in the standard, an xvalue of the referenced type is refused too.

Assisted-by: Claude:claude-opus-5-5
[func.require]/2 makes INVOKE<R> ill-formed when
reference_converts_from_temporary_v holds. libstdc++ applies this inside
is_invocable_r_v even in C++20; libc++ does not, so with libc++
pfn::invoke_r<int const &>([] { return 42; }) compiled and dangled.

Assisted-by: Claude:claude-opus-5-5
append(std::in_place_type<T &>, ...) asks the trait as a forwarding
parameter deduces, so an xvalue is refused alongside a prvalue, as
optional<T&> refuses both. The spelled as_pack gives a reference element
a parameter whose constructor deduces the argument, keeping every element
type spelled and braced lists working for value elements.

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

codecov Bot commented Oct 11, 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 Oct 11, 2026 •

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Reject reference bindings that would leave holders dangling.

  • Add C++23 temporary-binding traits in pfn/type_traits.hpp.
  • Use compiler builtins, with a conservative fallback where unavailable.
  • Guard construction and emplacement of both reference-optional implementations.
  • Reject unsafe explicitly typed as_pack bindings and reference-element appends.
  • Preserve value conversions and overloaded function-reference binding.
  • Add lifetime-aware invocability traits and constrain pfn::invoke_r.
  • Propagate these checks through fn::apply_r and applicability traits.
  • Add binding, conversion, invocation, and standard-library validation tests.
  • Document rejected xvalues, aggregate-initialization exclusions, and fallback limitations.

🤖 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. 1 suggestion posted.

Fix All in Augment

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

Comment thread include/pfn/functional.hpp Outdated
MSVC's preprocessor rejects __has_builtin, so MSVC is gated by
_MSC_FULL_VER: 19.51.36246 (Visual Studio 2026 18.6) is the earliest
version with both builtins, checked on Compiler Explorer, where 19.44
and 19.50 have neither. CI's Visual Studio 2026 lanes take the builtin
arm, its Visual Studio 2022 lanes the emulation.

The emulation and the dispatch become trait specializations and variable
templates: as consteval functions they only ran at compile time, which
line coverage reports as uncovered.

Assisted-by: Claude:claude-opus-5-5
fn's apply_r paths were constrained by std::is_invocable_r_v but called
pfn::invoke_r. Where the trait predates [func.require]/2 (libc++, and
libstdc++ without the builtin), the constraint allowed calls that
invoke_r rejected:
fn::is_applicable_r_v<int const &, int (*)(), std::tuple<>> was a hard
error instead of false, and the pack<> form returned true for a call
that could not be made.

pfn::is_invocable_r and pfn::is_nothrow_invocable_r give the C++23
answer on every standard library; pfn::invoke_r and fn's apply_r paths
now use them.

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

Bronek commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

augment review

@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. 1 suggestion posted.

Fix All in Augment

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

Comment thread include/fn/pack.hpp
A function reference binds only a function lvalue, never a temporary.
Only object references need argument deduction; using it for function
references broke overload resolution in as_pack<int (&)(int)>(f).

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

Bronek commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

augment review

@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. 1 suggestion posted.

Fix All in Augment

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

Comment thread include/fn/detail/pack_impl.hpp Outdated
Direct-initializing the reference member also considered explicit
conversion functions, so binding could differ from the implicit
conversion checked by the constraint and noexcept-specifier. clang
rejected such a source as ambiguous. The overloaded function-reference
test skips its constant-evaluation check on MSVC, whose evaluator reads
a function-reference member of an aggregate base as uninitialized.

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

Bronek commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

augment review

@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. 1 suggestion posted.

Fix All in Augment

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

Comment thread include/pfn/type_traits.hpp Outdated
The standard's VAL<U> is a prvalue with cv qualification removed when
U is a non-reference, non-class, non-array type. The emulation kept it,
so reference_constructs_from_temporary_v<int &&, const int> returned
false; Clang 19 and 20 builtins do the same. Both the emulation and
builtins now use the cv-unqualified type.

Comparisons with the builtins found that the emulation reported
temporaries for array references where GCC's builtin did not.
Compilers disagree on these bindings, so the emulation now reports
none for array referents, documented alongside missed temporaries
from class sources.

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

Bronek commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

augment review

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

@Bronek
Bronek force-pushed the bronek/dangling_references_from_ctor_fix branch from 7e7d2fc to 2e7f402 Compare October 12, 2026 19:53
Assisted-by: Codex:gpt-6.1-sol
@Bronek
Bronek force-pushed the bronek/dangling_references_from_ctor_fix branch from 2e7f402 to d3484c8 Compare October 12, 2026 20:03
@sonarqubecloud

Copy link
Copy Markdown

@Bronek
Bronek merged commit 38c2f9f into main Oct 12, 2026
59 checks passed
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.

Reject binding reference payloads to temporaries

1 participant