From patchwork Thu Feb 23 14:50:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 19357 Received: (qmail 20534 invoked by alias); 23 Feb 2017 14:50:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 20186 invoked by uid 89); 23 Feb 2017 14:50:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Feb 2017 14:50:14 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F2A697FB7F; Thu, 23 Feb 2017 14:50:14 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1NEoDj1031504; Thu, 23 Feb 2017 09:50:13 -0500 Subject: [PATCH v1.2 1/3] Introduce gdb::function_view To: Yao Qi References: <1487775033-32699-1-git-send-email-palves@redhat.com> <1487775033-32699-2-git-send-email-palves@redhat.com> <8636f39b5e1e2cbdeabbf8dfd999e709@polymtl.ca> <4c8528d8-0115-3bf3-0a16-42f44be580a9@redhat.com> <20baebcd-0f6a-89ca-ef34-503795171d43@redhat.com> <20170222222316.xaeqa67lmil52c7j@localhost> Cc: Simon Marchi , gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Thu, 23 Feb 2017 14:50:11 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170222222316.xaeqa67lmil52c7j@localhost> On 02/22/2017 10:23 PM, Yao Qi wrote: > On 17-02-22 17:49:22, Pedro Alves wrote: >> LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c >> >> @@ -1800,7 +1807,8 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \ >> xml-syscall.o \ >> xml-tdesc.o \ >> xml-utils.o \ >> - $(SUBDIR_GCC_COMPILE_OBS) >> + $(SUBDIR_GCC_COMPILE_OBS) \ >> + $(SUBDIR_UNITTESTS_OBS) >> > > Can we add SUBDIR_UNITESTS_OBS to CONFIG_OBS (in configure.ac) if > GDB_SELF_TEST is defined? Good idea, this works nicely. Here's the updated patch. From 735124ed86a210dd4cda290258f7c3bbe076fa0e Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 23 Feb 2017 14:11:17 +0000 Subject: [PATCH] Introduce gdb::function_view This commit adds a new function_view type. This type holds a non-owning reference to a callable. It is meant to be used as callback type of functions, instead of using the C-style pair of function pointer and 'void *data' arguments. function_view allows passing references to stateful function objects / lambdas with captures as callbacks efficiently, while function pointer + 'void *' does not. See the intro in the new function-view.h header for more. Unit tests included, put into a new gdb/unittests/ subdir. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * Makefile.in (SUBDIR_UNITTESTS_SRCS, SUBDIR_UNITTESTS_OBS): New. (SFILES): Add $(SUBDIR_UNITTEST_SRCS). (COMMON_OBS): Add $(SUBDIR_UNITTEST_OBS). (%.o) : New pattern. (INIT_FILES): Add $(SUBDIR_UNITTESTS_SRCS). * common/function-view.h: New file. * unittests/function-view-selftests.c: New file. --- gdb/Makefile.in | 10 + gdb/common/function-view.h | 355 ++++++++++++++++++++++++++++++++ gdb/configure | 2 + gdb/configure.ac | 2 + gdb/unittests/function-view-selftests.c | 178 ++++++++++++++++ 5 files changed, 547 insertions(+) create mode 100644 gdb/common/function-view.h create mode 100644 gdb/unittests/function-view-selftests.c diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 43253d3..268c2c6 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -523,6 +523,12 @@ SUBDIR_PYTHON_DEPS = SUBDIR_PYTHON_LDFLAGS = SUBDIR_PYTHON_CFLAGS = +SUBDIR_UNITTESTS_SRCS = \ + unittests/function-view-selftests.c + +SUBDIR_UNITTESTS_OBS = \ + function-view-selftests.o + # Opcodes currently live in one of two places. Either they are in the # opcode library, typically ../opcodes, or they are in a header file # in INCLUDE_DIR. @@ -1909,6 +1915,10 @@ all: gdb$(EXEEXT) $(CONFIG_ALL) $(COMPILE) $< $(POSTCOMPILE) +%.o: ${srcdir}/unittests/%.c + $(COMPILE) $< + $(POSTCOMPILE) + # Specify an explicit rule for gdb/common/agent.c, to avoid a clash with the # object file generate by gdb/agent.c. common-agent.o: $(srcdir)/common/agent.c diff --git a/gdb/common/function-view.h b/gdb/common/function-view.h new file mode 100644 index 0000000..66a691b --- /dev/null +++ b/gdb/common/function-view.h @@ -0,0 +1,355 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef COMMON_FUNCTION_VIEW_H +#define COMMON_FUNCTION_VIEW_H + +/* function_view is a polymorphic type-erasing wrapper class that + encapsulates a non-owning reference to arbitrary callable objects. + + A way to put it is that function_view is to std::function like + std::string_view is to std::string. While std::function stores a + type-erased callable object internally, function_view holds a + type-erased reference to an external callable object. + + This is meant to be used as callback type of a function that: + + #1 - Takes a callback as parameter. + + #2 - Wants to support arbitrary callable objects as callback type + (e.g., stateful function objects, lambda closures, free + functions). + + #3 - Does not store the callback anywhere; instead the function + just calls the callback directly or forwards it to some + other function that calls it. + + #4 - Can't be, or we don't want it to be, a template function + with the callable type as template parameter. For example, + when the callback is a parameter of a virtual member + function, or when putting the function template in a header + would expose too much implementation detail. + + Note that the C-style "function pointer" + "void *data" callback + parameter idiom fails requirement #2 above. Please don't add new + uses of that idiom. I.e., something like this wouldn't work; + + typedef bool (iterate_over_foos_cb) (foo *f, void *user_data), + void iterate_over_foos (iterate_over_foos_cb *callback, void *user_data); + + foo *find_foo_by_type (int type) + { + foo *found = nullptr; + + iterate_over_foos ([&] (foo *f, void *data) + { + if (foo->type == type) + { + found = foo; + return true; // stop iterating + } + return false; // continue iterating + }, NULL); + + return found; + } + + The above wouldn't compile, because lambdas with captures can't be + implicitly converted to a function pointer (because a capture means + some context data must be passed to the lambda somehow). + + C++11 gave us std::function as type-erased wrapper around arbitrary + callables, however, std::function is not an ideal fit for transient + callbacks such as the use case above. For this use case, which is + quite pervasive, a function_view is a better choice, because while + function_view is light and does not require any heap allocation, + std::function is a heavy-weight object with value semantics that + generally requires a heap allocation on construction/assignment of + the target callable. In addition, while it is possible to use + std::function in such a way that avoids most of the overhead by + making sure to only construct it with callables of types that fit + std::function's small object optimization, such as function + pointers and std::reference_wrapper callables, that is quite + inconvenient in practice, because restricting to free-function + callables would imply no state/capture/closure, which we need in + most cases, and std::reference_wrapper implies remembering to use + std::ref/std::cref where the callable is constructed, with the + added inconvenience that std::ref/std::cref have deleted rvalue-ref + overloads, meaning you can't use unnamed/temporary lambdas with + them. + + Note that because function_view is a non-owning view of a callable, + care must be taken to ensure that the callable outlives the + function_view that calls it. This is not really a problem for the + use case function_view is intended for, such as passing a temporary + function object / lambda to a function that accepts a callback, + because in those cases, the temporary is guaranteed to be live + until the called function returns. + + Calling a function_view with no associated target is undefined, + unlike with std::function, which throws std::bad_function_call. + This is by design, to avoid the otherwise necessary NULL check in + function_view::operator(). + + Since function_view objects are small (a pair of pointers), they + should generally be passed around by value. + + Usage: + + Given this function that accepts a callback: + + void + iterate_over_foos (gdb::function_view callback) + { + for (auto &foo : foos) + callback (&foo); + } + + you can call it like this, passing a lambda as callback: + + iterate_over_foos ([&] (foo *f) + { + process_one_foo (f); + }); + + or like this, passing a function object as callback: + + struct function_object + { + void operator() (foo *f) + { + if (s->check ()) + process_one_foo (f); + } + + // some state + state *s; + }; + + state mystate; + function_object matcher {&mystate}; + iterate_over_foos (matcher); + + or like this, passing a function pointer as callback: + + iterate_over_foos (process_one_foo); + + You can find unit tests covering the whole API in + unittests/function-view-selftests.c. */ + +namespace gdb { + +namespace traits { + /* A few trait helpers. */ + template + struct Not : public std::integral_constant + {}; + + template + struct Or; + + template<> + struct Or<> : public std::false_type + {}; + + template + struct Or : public B1 + {}; + + template + struct Or + : public std::conditional::type + {}; + + template + struct Or + : public std::conditional>::type + {}; +} /* namespace traits */ + +namespace fv_detail { +/* Bits shared by all function_view instantiations that do not depend + on the template parameters. */ + +/* Storage for the erased callable. This is a union in order to be + able to save both a function object (data) pointer or a function + pointer without triggering undefined behavior. */ +union erased_callable +{ + /* For function objects. */ + void *data; + + /* For function pointers. */ + void (*fn) (); +}; + +} /* namespace fv_detail */ + +/* Use partial specialization to get access to the callable's + signature. */ +template +struct function_view; + +template +class function_view +{ + template + using CompatibleReturnType + = traits::Or, + std::is_same, + std::is_convertible>; + + /* True if Func can be called with Args, and either the result is + Res, convertible to Res or Res is void. */ + template::type> + struct IsCompatibleCallable : CompatibleReturnType + {}; + + /* True if Callable is a function_view. Used to avoid hijacking the + copy ctor. */ + template + struct IsFunctionView + : std::is_same::type> + {}; + + /* Helper to make SFINAE logic easier to read. */ + template + using Requires = typename std::enable_if::type; + + public: + + /* NULL by default. */ + constexpr function_view () noexcept + : m_erased_callable {}, + m_invoker {} + {} + + /* Default copy/assignment is fine. */ + function_view (const function_view &) = default; + function_view &operator= (const function_view &) = default; + + /* This is the main entry point. Use SFINAE to avoid hijacking the + copy constructor and to ensure that the target type is + compatible. */ + template + >>, + typename = Requires>> + function_view (Callable &&callable) noexcept + { + bind (callable); + } + + /* Construct a NULL function_view. */ + constexpr function_view (std::nullptr_t) noexcept + : m_erased_callable {}, + m_invoker {} + {} + + /* Clear a function_view. */ + function_view &operator= (std::nullptr_t) noexcept + { + m_invoker = nullptr; + return *this; + } + + /* Return true if the wrapper has a target, false otherwise. Note + we check M_INVOKER instead of M_ERASED_CALLABLE because we don't + know which member of the union is active right now. */ + constexpr explicit operator bool () const noexcept + { return m_invoker != nullptr; } + + /* Call the callable. */ + Res operator () (Args... args) const + { return m_invoker (m_erased_callable, std::forward (args)...); } + + private: + + /* Bind this function_view to a compatible function object + reference. */ + template + void bind (Callable &callable) noexcept + { + m_erased_callable.data = (void *) std::addressof (callable); + m_invoker = [] (fv_detail::erased_callable ecall, Args... args) + noexcept (noexcept (callable (std::forward (args)...))) -> Res + { + auto &restored_callable = *static_cast (ecall.data); + /* The explicit cast to Res avoids a compile error when Res is + void and the callable returns non-void. */ + return (Res) restored_callable (std::forward (args)...); + }; + } + + /* Bind this function_view to a compatible function pointer. + + Making this a separate function allows avoiding one indirection, + by storing the function pointer directly in the storage, instead + of a pointer to pointer. erased_callable is then a union in + order to avoid storing a function pointer as a data pointer here, + which would be undefined. */ + template + void bind (Res2 (*fn) (Args2...)) noexcept + { + m_erased_callable.fn = reinterpret_cast (fn); + m_invoker = [] (fv_detail::erased_callable ecall, Args... args) + noexcept (noexcept (fn (std::forward (args)...))) -> Res + { + auto restored_fn = reinterpret_cast (ecall.fn); + /* The explicit cast to Res avoids a compile error when Res is + void and the callable returns non-void. */ + return (Res) restored_fn (std::forward (args)...); + }; + } + + /* Storage for the erased callable. */ + fv_detail::erased_callable m_erased_callable; + + /* The invoker. This is set to a capture-less lambda by one of the + 'bind' overloads. The lambda restores the right type of the + callable (which is passed as first argument), and forwards the + args. */ + Res (*m_invoker) (fv_detail::erased_callable, Args...); +}; + +/* Allow comparison with NULL. Defer the work to the in-class + operator bool implementation. */ + +template +constexpr inline bool +operator== (const function_view &f, std::nullptr_t) noexcept +{ return !static_cast (f); } + +template +constexpr inline bool +operator== (std::nullptr_t, const function_view &f) noexcept +{ return !static_cast (f); } + +template +constexpr inline bool +operator!= (const function_view &f, std::nullptr_t) noexcept +{ return static_cast (f); } + +template +constexpr inline bool +operator!= (std::nullptr_t, const function_view &f) noexcept +{ return static_cast (f); } + +} /* namespace gdb */ + +#endif diff --git a/gdb/configure b/gdb/configure index 6df88d9..7fbae9f 100755 --- a/gdb/configure +++ b/gdb/configure @@ -17410,6 +17410,8 @@ if $development; then $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h + CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS)" + CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS)" fi diff --git a/gdb/configure.ac b/gdb/configure.ac index d2d854d..50f6f59 100644 --- a/gdb/configure.ac +++ b/gdb/configure.ac @@ -2350,6 +2350,8 @@ AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8", if $development; then AC_DEFINE(GDB_SELF_TEST, 1, [Define if self-testing features should be enabled]) + CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS)" + CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS)" fi GDB_AC_TRANSFORM([gdb], [GDB_TRANSFORM_NAME]) diff --git a/gdb/unittests/function-view-selftests.c b/gdb/unittests/function-view-selftests.c new file mode 100644 index 0000000..310f2ad --- /dev/null +++ b/gdb/unittests/function-view-selftests.c @@ -0,0 +1,178 @@ +/* Self tests for function_view for GDB, the GNU debugger. + + Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include "defs.h" +#include "selftest.h" +#include "common/function-view.h" + +namespace selftests { +namespace function_view { + +static int +plus_one_fn_int (int val) +{ + return ++val; +} + +static short +plus_one_fn_short (short val) +{ + return ++val; +} + +static int +call_callback_int (int val, gdb::function_view callback) +{ + return callback (val); +} + +static void +call_callback_void (int val, gdb::function_view callback) +{ + callback (val); +} + +struct plus_one_int_func_obj +{ + int operator () (int val) + { + ++call_count; + return ++val; + } + + /* Number of times called. */ + int call_count = 0; +}; + +static void +run_tests () +{ + /* A simple lambda. */ + auto plus_one_lambda = [] (int val) { return ++val; }; + + /* A function_view that references the lambda. */ + gdb::function_view plus_one_func_view (plus_one_lambda); + + /* Check calling the lambda directly. */ + SELF_CHECK (plus_one_lambda (0) == 1); + SELF_CHECK (plus_one_lambda (1) == 2); + + /* Check calling lambda via the view. */ + SELF_CHECK (plus_one_func_view (2) == 3); + SELF_CHECK (plus_one_func_view (3) == 4); + + /* Check calling a function that takes a function_view as argument, + by value. Pass a lambda, making sure a function_view is properly + constructed implicitly. */ + SELF_CHECK (call_callback_int (1, [] (int val) + { + return val + 2; + }) == 3); + + /* Same, passing a named/lvalue lambda. */ + SELF_CHECK (call_callback_int (1, plus_one_lambda) == 2); + /* Same, passing a named/lvalue function_view (should copy). */ + SELF_CHECK (call_callback_int (1, plus_one_func_view) == 2); + + /* Check constructing a function view over a function-object + callable, and calling it. */ + plus_one_int_func_obj func_obj; + SELF_CHECK (func_obj (0) == 1); + SELF_CHECK (call_callback_int (1, func_obj) == 2); + /* Check that the callable was referenced, not copied. */ + SELF_CHECK (func_obj.call_count == 2); + + /* Check constructing a function_view over a free-function callable, + and calling it. */ + SELF_CHECK (call_callback_int (1, plus_one_fn_int) == 2); + + /* Check calling a function with a + compatible-but-not-exactly-the-same prototype. */ + SELF_CHECK (call_callback_int (1, [] (short val) -> short + { + return val + 2; + }) == 3); + /* Same, but passing a function pointer. */ + SELF_CHECK (call_callback_int (1, plus_one_fn_short) == 2); + + /* Like std::function, a function_view that expects a void return + can reference callables with non-void return type. The result is + simply discarded. Check a lambda, function object and a function + pointer. */ + call_callback_void (1, [] (int val) -> int + { + return val + 2; + }); + call_callback_void (1, func_obj); + call_callback_void (1, plus_one_fn_int); + + /* Check that the main ctor doesn't hijack the copy ctor. */ + auto plus_one_func_view2 (plus_one_func_view); + auto plus_one_func_view3 (plus_one_func_view2); + static_assert (std::is_same::value, ""); + static_assert (std::is_same::value, ""); + + SELF_CHECK (plus_one_func_view3 (1) == 2); + + /* Likewise, but propagate a NULL callable. If this calls the main + function_view ctor instead of the copy ctor by mistake, then + null_func_2 ends up non-NULL (because it'd instead reference + null_func_1 as just another callable). */ + constexpr gdb::function_view null_func_view_1 = nullptr; + constexpr auto null_func_view_2 (null_func_view_1); + + /* While at it, check whether the function_view is bound using + various forms, op==, op!= and op bool. */ + + /* op== */ + static_assert (null_func_view_2 == nullptr, ""); + static_assert (nullptr == null_func_view_2, ""); + static_assert (null_func_view_2 == NULL, ""); + static_assert (NULL == null_func_view_2, ""); + + /* op!= */ + static_assert (!(null_func_view_2 != nullptr), ""); + static_assert (!(nullptr != null_func_view_2), ""); + static_assert (!(null_func_view_2 != NULL), ""); + static_assert (!(NULL != null_func_view_2), ""); + + /* op bool */ + static_assert (!null_func_view_2, ""); + + /* Check the nullptr_t ctor. */ + constexpr gdb::function_view check_ctor_nullptr (nullptr); + static_assert (!check_ctor_nullptr, ""); + + /* Check the nullptr_t op= */ + gdb::function_view check_op_eq_null (plus_one_fn_int); + SELF_CHECK (check_op_eq_null); + check_op_eq_null = nullptr; + SELF_CHECK (!check_op_eq_null); +} + +} /* namespace function_view */ +} /* namespace selftests */ + +void +_initialize_function_view_selftests () +{ + register_self_test (selftests::function_view::run_tests); +}