From 55133880277367e82fc22691b36e4d1f6343dd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domagoj=20=C5=A0ari=C4=87?= Date: Thu, 10 Sep 2026 10:22:42 +0200 Subject: [PATCH] fix(function_ref): do not mark an inline-stored callable as borrowed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A callable that is trivially copyable and fits the data word is COPIED into the ref by `make_c_callback` — the ref then owns everything it needs and points at nothing. The single converting constructor nevertheless carried `[[clang::lifetimebound]]`, so building a ref from a temporary of that kind was diagnosed as a dangling reference and, under `-Werror`, rejected outright: return function_ref{ [ p ]( int const v ) { *p = v; } }; error: returning address of local temporary object [-Wreturn-stack-address] The attribute cannot be applied conditionally, so split the constructor in two and let the constraints partition on whether the target is stored inline: - not stored inline -> the ref points at the caller's object, which must outlive it. Keeps `lifetimebound`, unchanged behaviour. - stored inline -> copied, borrows nothing, so a temporary is fine. The classification is exposed as `stored_inline` so callers can assert the property they are relying on. Note it is about storage, not value category: an lvalue of an inline-storable type was never borrowed either. Tests: a ref built from a temporary that captures one pointer stays valid after the full-expression and is returnable; a two-pointer capture is still classified as borrowing (and still diagnosed as such by the compiler when returned). --- include/psi/functionoid/function_ref.hpp | 42 +++++++++++++++++++++--- test/function_ref_test.cpp | 36 ++++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/include/psi/functionoid/function_ref.hpp b/include/psi/functionoid/function_ref.hpp index e2ec500..9c4afc5 100644 --- a/include/psi/functionoid/function_ref.hpp +++ b/include/psi/functionoid/function_ref.hpp @@ -33,13 +33,38 @@ class [[ clang::trivial_abi ]] function_ref public: constexpr function_ref() = default; + /// Whether \c make_c_callback stores \c F by value in the data word instead + /// of pointing at the caller's object — i.e. whether this ref borrows at all. + template + static bool constexpr stored_inline + { + std::is_trivially_copy_constructible_v> && + ( sizeof( std::remove_reference_t ) <= sizeof( void * ) ) + }; + + template + static bool constexpr invocable_as + { noexcept( std::declval()( std::declval()... ) ) >= ne }; + + /// Borrowing overload: the target stays where the caller put it, so it must + /// outlive this ref — hence \c lifetimebound. template function_ref( F && callable [[ clang::lifetimebound ]] ) noexcept - requires ( noexcept( callable( std::declval()... ) ) >= ne ) + requires ( invocable_as && !stored_inline ) { - auto const cb{ make_c_callback( std::forward( callable ) ) }; - data_ = cb.first; - function_ = static_cast( cb.second ); + bind( std::forward( callable ) ); + } + + /// Copying overload: a trivially copyable callable that fits the data word is + /// COPIED into this ref, which therefore borrows nothing — so binding a + /// temporary (a lambda built in the argument, a function pointer) is safe and + /// must not be diagnosed. It needs to be a separate overload because + /// `[[clang::lifetimebound]]` cannot be applied conditionally. + template + function_ref( F && callable ) noexcept + requires ( invocable_as && stored_inline ) + { + bind( std::forward( callable ) ); } template @@ -128,6 +153,15 @@ class [[ clang::trivial_abi ]] function_ref } } +private: + template + void bind( F && callable ) noexcept + { + auto const cb{ make_c_callback( std::forward( callable ) ) }; + data_ = cb.first; + function_ = static_cast( cb.second ); + } + private: R ( *function_ )( void *, Args... ) noexcept( ne ){}; void * data_{}; diff --git a/test/function_ref_test.cpp b/test/function_ref_test.cpp index 07880f7..4a9bbfa 100644 --- a/test/function_ref_test.cpp +++ b/test/function_ref_test.cpp @@ -34,3 +34,39 @@ TEST( FunctionRefTest, BoostAliasMatchesPsi ) ref(); EXPECT_EQ( g_value, 43 ); } + +// A callable small and trivial enough to live in the ref's data word is copied +// into it, not pointed at, so it does not outlive-borrow anything: a ref built +// from a temporary stays valid after the full-expression, and may be returned. +TEST( FunctionRefTest, InlineStoredCallableSurvivesItsTemporary ) +{ + int value{ 0 }; + auto const makeRef{ [ &value ]{ + // the lambda is a temporary of this full-expression; it captures one + // pointer, so the ref takes a copy rather than its address + return psi::functionoid::function_ref{ + [ p = &value ]( int const v ) { *p = v; } + }; + } }; + auto const ref{ makeRef() }; + ref( 11 ); + EXPECT_EQ( value, 11 ); +} + +TEST( FunctionRefTest, InlineStorageClassification ) +{ + using Ref = psi::functionoid::function_ref; + + int a{}, b{}; + auto const capturesOnePointer { [ pA = &a ] ( int const v ) { *pA = v; } }; + auto const capturesTwoPointers{ [ pA = &a, pB = &b ]( int const v ) { *pA = *pB = v; } }; + + // copied into the data word -> the ref borrows nothing + static_assert( Ref::stored_inline ); + // too large -> the ref points at the caller's object, which must outlive it + static_assert( !Ref::stored_inline ); + // a plain function pointer is copied too + static_assert( Ref::stored_inline ); + + EXPECT_TRUE( Ref{ capturesTwoPointers } ); // the borrowing overload still works for an lvalue +}