From 1aa6ba9fac70ac0354f63b55dd587e7474f26467 Mon Sep 17 00:00:00 2001
From: Vincent Hengel <prelink835@gmail.com>
Date: Sat, 26 Sep 2026 21:20:22 +0200
Subject: [PATCH] fix(base): copy a const Function instead of wrapping it

---
 base/containers/static_function.h       |  7 ++++
 base/containers/static_function_test.cc | 45 +++++++++++++++++++++++++
 base/functional/function.h              | 11 ++++--
 base/functional/function_test.cc        | 18 ++++++++++
 4 files changed, 78 insertions(+), 3 deletions(-)
 create mode 100644 base/containers/static_function_test.cc

diff --git a/base/containers/static_function.h b/base/containers/static_function.h
index 5875c23a..da4d09f1 100644
--- a/base/containers/static_function.h
+++ b/base/containers/static_function.h
@@ -36,7 +36,12 @@ class StaticFunction<R(Args...), MaxSize> {
 
   StaticFunction(StaticFunction&& other) { other.swap(*this); }
 
+  // Not for StaticFunction itself: a non-const lvalue or a const rvalue would
+  // otherwise pick this over the copy constructor and try to store a
+  // StaticFunction inside itself.
   template <class F>
+    requires(!base::is_same_v<base::remove_cvref_t<F>, StaticFunction> &&
+             !base::is_same_v<base::remove_cvref_t<F>, base::nullptr_t>)
   StaticFunction(F&& f) {
     using f_type = base::decay_t<F>;
     static_assert(alignof(f_type) <= kStorageAlign, "invalid alignment");
@@ -72,6 +77,8 @@ class StaticFunction<R(Args...), MaxSize> {
   }
 
   template <typename F>
+    requires(!base::is_same_v<base::remove_cvref_t<F>, StaticFunction> &&
+             !base::is_same_v<base::remove_cvref_t<F>, base::nullptr_t>)
   StaticFunction& operator=(F&& f) {
     StaticFunction(base::forward<F>(f)).swap(*this);
     return *this;
diff --git a/base/containers/static_function_test.cc b/base/containers/static_function_test.cc
new file mode 100644
index 00000000..a685f91e
--- /dev/null
+++ b/base/containers/static_function_test.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2026 Vincent Hengel.
+// For licensing information see LICENSE at the root of this distribution.
+
+#include <gtest/gtest.h>
+
+#include <base/containers/static_function.h>
+#include <base/memory/move.h>
+
+namespace {
+
+using Fn = base::StaticFunction<int(int), 64>;
+
+TEST(StaticFunction, CallsAStoredLambda) {
+  int base_value = 3;
+  Fn fn([base_value](int x) { return x + base_value; });
+  EXPECT_EQ(fn(4), 7);
+}
+
+// A non-const lvalue and a const rvalue both have to take the copy
+// constructor, not the converting one, which would try to store a
+// StaticFunction inside itself.
+TEST(StaticFunction, CopiesFromANonConstLvalue) {
+  Fn fn([](int x) { return x * 3; });
+  Fn copy(fn);
+  EXPECT_EQ(copy(2), 6);
+  EXPECT_EQ(fn(3), 9);
+}
+
+TEST(StaticFunction, CopiesFromAConstRvalue) {
+  const Fn fn([](int x) { return x - 1; });
+  Fn copy(static_cast<const Fn&&>(fn));
+  EXPECT_EQ(copy(5), 4);
+}
+
+TEST(StaticFunction, AssignsFromAnotherStaticFunction) {
+  Fn source([](int x) { return x * x; });
+  Fn target;
+  target = source;
+  EXPECT_EQ(target(5), 25);
+  Fn moved;
+  moved = base::move(source);
+  EXPECT_EQ(moved(4), 16);
+}
+
+}  // namespace
diff --git a/base/functional/function.h b/base/functional/function.h
index e489a49c..d672d98c 100644
--- a/base/functional/function.h
+++ b/base/functional/function.h
@@ -6,6 +6,7 @@
 #include <base/memory/move.h>
 #include <base/memory/cxx_lifetime.h>
 #include <base/check.h>
+#include <base/meta/traits.h>
 #include <base/containers/container_traits.h>
 #include <new>
 #include <stddef.h>  // max_align_t
@@ -195,11 +196,15 @@ struct Function<R(Args...)> {
   bool operator==(decltype(nullptr)) const noexcept { return !has_target_; }
   bool operator!=(decltype(nullptr)) const noexcept { return has_target_; }
 
-  // SFINAE helper: true for everything except Function itself and nullptr_t
+  // SFINAE helper: true for everything except Function itself and nullptr_t.
+  // cv-qualifiers are stripped too: a const Function&& (moving a closure that
+  // captured a Function by copy) would otherwise bind here ahead of the copy
+  // constructor and wrap the Function in another Function, recursing without
+  // bound.
   template <typename T>
   static constexpr bool IsCallable =
-      !base::ISSame<typename base::remove_reference<T>::type, Function> &&
-      !base::ISSame<typename base::remove_reference<T>::type, decltype(nullptr)>;
+      !base::ISSame<base::remove_cvref_t<T>, Function> &&
+      !base::ISSame<base::remove_cvref_t<T>, decltype(nullptr)>;
 
   // Constructor from callable (lambda, functor, function pointer)
   template <typename T>
diff --git a/base/functional/function_test.cc b/base/functional/function_test.cc
index 20103322..c9513278 100644
--- a/base/functional/function_test.cc
+++ b/base/functional/function_test.cc
@@ -43,4 +43,22 @@ TEST(Function, CanInitializeWithMemberFunction) {
   EXPECT_EQ(func(obj, 2, 3), 5);
 }
 #endif
+// A closure that captured a Function by copy holds a const member; moving the
+// closure moves that member as a const Function&&, which must copy the
+// Function rather than wrap it in another Function.
+TEST(Function, CopiesFromAConstRvalue) {
+  const base::Function<int(int)> inner([](int x) { return x * 2; });
+  base::Function<int(int)> copy(static_cast<const base::Function<int(int)>&&>(inner));
+  EXPECT_EQ(copy(21), 42);
+  EXPECT_EQ(inner(4), 8);
+}
+
+TEST(Function, MovesAClosureThatCapturedAFunction) {
+  const base::Function<int(int)> hook([](int x) { return x + 1; });
+  auto closure = [hook](int x) { return hook(x) * 10; };
+  base::Function<int(int)> outer(base::move(closure));
+  base::Function<int(int)> moved(base::move(outer));
+  EXPECT_EQ(moved(1), 20);
+}
+
 }  // namespace