From patchwork Mon Sep 4 16:19:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 22605 Received: (qmail 86192 invoked by alias); 4 Sep 2017 16:19:24 -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 86177 invoked by uid 89); 4 Sep 2017 16:19:24 -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=VIEW 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; Mon, 04 Sep 2017 16:19:17 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0E5F672F16 for ; Mon, 4 Sep 2017 16:19:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 0E5F672F16 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 54AC4C14C4 for ; Mon, 4 Sep 2017 16:19:15 +0000 (UTC) Subject: Re: [PATCH 1/3] Introduce gdb::array_view To: GDB Patches References: <1503343667-6790-1-git-send-email-palves@redhat.com> <1503343667-6790-2-git-send-email-palves@redhat.com> From: Pedro Alves Message-ID: <42937af4-68de-afa2-9b0f-52221b8d5e4e@redhat.com> Date: Mon, 4 Sep 2017 17:19:14 +0100 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: <1503343667-6790-2-git-send-email-palves@redhat.com> Here's what I pushed. It's basically the same except for some unit tests improvements around some comments and simplifications. From 7c44b49cb63662b76c6301fdc8e022d7aca655bf Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 4 Sep 2017 17:10:12 +0100 Subject: [PATCH] Introduce gdb::array_view An array_view is an abstraction that provides a non-owning view over a sequence of contiguous objects. A way to put it is that array_view is to std::vector (and std::array and built-in arrays with rank==1) like std::string_view is to std::string. The main intent of array_view is to use it as function input parameter type, making it possible to pass in any sequence of contiguous objects, irrespective of whether the objects live on the stack or heap and what actual container owns them. Implicit construction from the element type is supported too, making it easy to call functions that expect an array of elements when you only have one element (usually on the stack). For example: struct A { .... }; void function (gdb::array_view as); std::vector std_vec = ...; std::array std_array = ...; A array[] = {...}; A elem; function (std_vec); function (std_array); function (array); function (elem); Views can be either mutable or const. A const view is simply created by specifying a const T as array_view template parameter, in which case operator[] of non-const array_view objects ends up returning const references. (Making the array_view itself const is analogous to making a pointer itself be const. I.e., disables re-seating the view/pointer.) Normally functions will pass around array_views by value. Uses of gdb::array_view (other than the ones in the unit tests) will be added in a follow up patch. gdb/ChangeLog 2017-09-04 Pedro Alves * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add unittests/array-view-selftests.c. (SUBDIR_UNITTESTS_OBS): Add array-view-selftests.o. * common/array-view.h: New file. * unittests/array-view-selftests.c: New file. --- gdb/ChangeLog | 8 + gdb/Makefile.in | 2 + gdb/common/array-view.h | 179 +++++++++++++ gdb/unittests/array-view-selftests.c | 494 +++++++++++++++++++++++++++++++++++ 4 files changed, 683 insertions(+) create mode 100644 gdb/common/array-view.h create mode 100644 gdb/unittests/array-view-selftests.c diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2d12899..f0e9797 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,13 @@ 2017-09-04 Pedro Alves + * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add + unittests/array-view-selftests.c. + (SUBDIR_UNITTESTS_OBS): Add array-view-selftests.o. + * common/array-view.h: New file. + * unittests/array-view-selftests.c: New file. + +2017-09-04 Pedro Alves + * cli/cli-cmds.c (edit_command): Pass message to ambiguous_line_spec. (list_command): Pass message to ambiguous_line_spec. Say diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 85de646..d9094fd 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -526,6 +526,7 @@ SUBDIR_PYTHON_LDFLAGS = SUBDIR_PYTHON_CFLAGS = SUBDIR_UNITTESTS_SRCS = \ + unittests/array-view-selftests.c \ unittests/environ-selftests.c \ unittests/function-view-selftests.c \ unittests/offset-type-selftests.c \ @@ -534,6 +535,7 @@ SUBDIR_UNITTESTS_SRCS = \ unittests/scoped_restore-selftests.c SUBDIR_UNITTESTS_OBS = \ + array-view-selftests.o \ environ-selftests.o \ function-view-selftests.o \ offset-type-selftests.o \ diff --git a/gdb/common/array-view.h b/gdb/common/array-view.h new file mode 100644 index 0000000..85d189c --- /dev/null +++ b/gdb/common/array-view.h @@ -0,0 +1,179 @@ +/* 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_ARRAY_VIEW_H +#define COMMON_ARRAY_VIEW_H + +#include "traits.h" +#include + +/* An array_view is an abstraction that provides a non-owning view + over a sequence of contiguous objects. + + A way to put it is that array_view is to std::vector (and + std::array and built-in arrays with rank==1) like std::string_view + is to std::string. + + The main intent of array_view is to use it as function input + parameter type, making it possible to pass in any sequence of + contiguous objects, irrespective of whether the objects live on the + stack or heap and what actual container owns them. Implicit + construction from the element type is supported too, making it easy + to call functions that expect an array of elements when you only + have one element (usually on the stack). For example: + + struct A { .... }; + void function (gdb::array_view as); + + std::vector std_vec = ...; + std::array std_array = ...; + A array[] = {...}; + A elem; + + function (std_vec); + function (std_array); + function (array); + function (elem); + + Views can be either mutable or const. A const view is simply + created by specifying a const T as array_view template parameter, + in which case operator[] of non-const array_view objects ends up + returning const references. Making the array_view itself const is + analogous to making a pointer itself be const. I.e., disables + re-seating the view/pointer. + + Since array_view objects are small (pointer plus size), and + designed to be trivially copyable, they should generally be passed + around by value. + + You can find unit tests covering the whole API in + unittests/array-view-selftests.c. */ + +namespace gdb { + +template +class array_view +{ + /* True iff decayed T is the same as decayed U. E.g., we want to + say that 'T&' is the same as 'const T'. */ + template + using IsDecayedT = typename std::is_same::type, + typename std::decay::type>; + + /* True iff decayed T is the same as decayed U, and 'U *' is + implicitly convertible to 'T *'. This is a requirement for + several methods. */ + template + using DecayedConvertible = gdb::And, + std::is_convertible>; + +public: + using value_type = T; + using reference = T &; + using const_reference = const T &; + using size_type = size_t; + + /* Default construction creates an empty view. */ + constexpr array_view () noexcept + : m_array (nullptr), m_size (0) + {} + + /* Create an array view over a single object of the type of an + array_view element. The created view as size==1. This is + templated on U to allow constructing a array_view over a + (non-const) T. The "convertible" requirement makes sure that you + can't create an array_view over a const T. */ + template>> + constexpr array_view (U &elem) noexcept + : m_array (&elem), m_size (1) + {} + + /* Same as above, for rvalue references. */ + template>> + constexpr array_view (U &&elem) noexcept + : m_array (&elem), m_size (1) + {} + + /* Create an array view from a pointer to an array and an element + count. */ + template>> + constexpr array_view (U *array, size_t size) noexcept + : m_array (array), m_size (size) + {} + + /* Create an array view from a range. This is templated on both U + an V to allow passing in a mix of 'const T *' and 'T *'. */ + template>, + typename = Requires>> + constexpr array_view (U *begin, V *end) noexcept + : m_array (begin), m_size (end - begin) + {} + + /* Create an array view from an array. */ + template>> + constexpr array_view (U (&array)[Size]) noexcept + : m_array (array), m_size (Size) + {} + + /* Create an array view from a contiguous container. E.g., + std::vector and std::array. */ + template>>, + typename + = Requires ().data ()), + T *>>, + typename + = Requires ().size ()), + size_type>>> + constexpr array_view (Container &&c) noexcept + : m_array (c.data ()), m_size (c.size ()) + {} + + /* Observer methods. Some of these can't be constexpr until we + require C++14. */ + /*constexpr14*/ T *data () noexcept { return m_array; } + constexpr const T *data () const noexcept { return m_array; } + + /*constexpr14*/ T *begin () noexcept { return m_array; } + constexpr const T *begin () const noexcept { return m_array; } + + /*constexpr14*/ T *end () noexcept { return m_array + m_size; } + constexpr const T *end () const noexcept { return m_array + m_size; } + + /*constexpr14*/ reference operator[] (size_t index) noexcept + { return m_array[index]; } + constexpr const_reference operator[] (size_t index) const noexcept + { return m_array[index]; } + + constexpr size_type size () const noexcept { return m_size; } + constexpr bool empty () const noexcept { return m_size == 0; } + +private: + T *m_array; + size_type m_size; +}; + +} /* namespace gdb */ + +#endif diff --git a/gdb/unittests/array-view-selftests.c b/gdb/unittests/array-view-selftests.c new file mode 100644 index 0000000..c4c95bc --- /dev/null +++ b/gdb/unittests/array-view-selftests.c @@ -0,0 +1,494 @@ +/* Self tests for array_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/array-view.h" + +namespace selftests { +namespace array_view_tests { + +/* Triviality checks. */ +#define CHECK_TRAIT(TRAIT) \ + static_assert (std::TRAIT>::value, "") + +#if HAVE_IS_TRIVIALLY_COPYABLE + +CHECK_TRAIT (is_trivially_copyable); +CHECK_TRAIT (is_trivially_move_assignable); +CHECK_TRAIT (is_trivially_move_constructible); +CHECK_TRAIT (is_trivially_destructible); + +#endif + +#undef CHECK_TRAIT + +/* Wrapper around std::is_convertible to make the code using it a bit + shorter. (With C++14 we'd use a variable template instead.) */ + +template +static constexpr bool +is_convertible () +{ + return std::is_convertible::value; +} + +/* Check for implicit conversion to immutable and mutable views. */ + +static constexpr bool +check_convertible () +{ + using T = gdb_byte; + using gdb::array_view; + + return (true + /* immutable array_view */ + && is_convertible> () + && is_convertible> () + && is_convertible> () + && is_convertible> () + + /* mutable array_view */ + && is_convertible> () + && !is_convertible> () + && is_convertible> () + && !is_convertible> () + + /* While float is implicitly convertible to gdb_byte, we + don't want implicit float->array_view + conversion. */ + && !is_convertible> () + && !is_convertible> ()); +} + +static_assert (check_convertible (), ""); + +namespace no_slicing +{ +struct A { int i; }; +struct B : A { int j; }; +struct C : A { int l; }; + +/* Check that there's no array->view conversion for arrays of derived + types or subclasses. */ +static constexpr bool +check () +{ + using gdb::array_view; + + return (true + + /* array->view */ + + && is_convertible > () + && !is_convertible > () + && !is_convertible > () + + && !is_convertible > () + && is_convertible > () + && !is_convertible > () + + /* elem->view */ + + && is_convertible > () + && !is_convertible > () + && !is_convertible > () + + && !is_convertible > () + && is_convertible > () + && !is_convertible > ()); +} + +} /* namespace no_slicing */ + +static_assert (no_slicing::check (), ""); + +/* Check that array_view implicitly converts from std::vector. */ + +static constexpr bool +check_convertible_from_std_vector () +{ + using gdb::array_view; + using T = gdb_byte; + + /* Note there's no such thing as std::vector. */ + + return (true + && is_convertible , array_view> () + && is_convertible , array_view> ()); +} + +static_assert (check_convertible_from_std_vector (), ""); + +/* Check that array_view implicitly converts from std::array. */ + +static constexpr bool +check_convertible_from_std_array () +{ + using gdb::array_view; + using T = gdb_byte; + + /* Note: a non-const T view can't refer to a const T array. */ + + return (true + && is_convertible , array_view> () + && is_convertible , array_view> () + && !is_convertible , array_view> () + && is_convertible , array_view> ()); +} + +static_assert (check_convertible_from_std_array (), ""); + +/* Check that VIEW views C (a container like std::vector/std::array) + correctly. */ + +template +static bool +check_container_view (const View &view, const Container &c) +{ + if (view.empty ()) + return false; + if (view.size () != c.size ()) + return false; + if (view.data () != c.data ()) + return false; + for (size_t i = 0; i < c.size (); i++) + { + if (&view[i] != &c[i]) + return false; + if (view[i] != c[i]) + return false; + } + return true; +} + +/* Check that VIEW views E (an object of the type of a view element) + correctly. */ + +template +static bool +check_elem_view (const View &view, const Elem &e) +{ + if (view.empty ()) + return false; + if (view.size () != 1) + return false; + if (view.data () != &e) + return false; + if (&view[0] != &e) + return false; + if (view[0] != e) + return false; + return true; +} + +/* Check for operator[]. The first overload is taken iff + 'view()[0] = T()' is a valid expression. */ + +template ()[0] + = std::declval ())> +static bool +check_op_subscript (const View &view) +{ + return true; +} + +/* This overload is taken iff 'view()[0] = T()' is not a valid + expression. */ + +static bool +check_op_subscript (...) +{ + return false; +} + +/* Check construction with pointer + size. This is a template in + order to test both gdb_byte and const gdb_byte. */ + +template +static void +check_ptr_size_ctor () +{ + T data[] = {0x11, 0x22, 0x33, 0x44}; + + gdb::array_view view (data + 1, 2); + + SELF_CHECK (!view.empty ()); + SELF_CHECK (view.size () == 2); + SELF_CHECK (view.data () == &data[1]); + SELF_CHECK (view[0] == data[1]); + SELF_CHECK (view[1] == data[2]); + + gdb::array_view cview (data + 1, 2); + SELF_CHECK (!cview.empty ()); + SELF_CHECK (cview.size () == 2); + SELF_CHECK (cview.data () == &data[1]); + SELF_CHECK (cview[0] == data[1]); + SELF_CHECK (cview[1] == data[2]); +} + +/* Asserts std::is_constructible. */ + +template +static constexpr bool +require_not_constructible () +{ + static_assert (!std::is_constructible::value, ""); + + /* constexpr functions can't return void in C++11 (N3444). */ + return true; +}; + +/* Check the array_view(PTR, SIZE) ctor, when T is a pointer. */ + +void +check_ptr_size_ctor2 () +{ + struct A {}; + A an_a; + + A *array[] = { &an_a }; + const A * const carray[] = { &an_a }; + + gdb::array_view v1 = {array, ARRAY_SIZE (array)}; + gdb::array_view v2 = {array, (char) ARRAY_SIZE (array)}; + gdb::array_view v3 = {array, ARRAY_SIZE (array)}; + gdb::array_view cv1 = {carray, ARRAY_SIZE (carray)}; + + require_not_constructible, decltype (carray), size_t> (); + + SELF_CHECK (v1[0] == array[0]); + SELF_CHECK (v2[0] == array[0]); + SELF_CHECK (v3[0] == array[0]); + + SELF_CHECK (!v1.empty ()); + SELF_CHECK (v1.size () == 1); + SELF_CHECK (v1.data () == &array[0]); + + SELF_CHECK (cv1[0] == carray[0]); + + SELF_CHECK (!cv1.empty ()); + SELF_CHECK (cv1.size () == 1); + SELF_CHECK (cv1.data () == &carray[0]); +} + +/* Check construction with a pair of pointers. This is a template in + order to test both gdb_byte and const gdb_byte. */ + +template +static void +check_ptr_ptr_ctor () +{ + T data[] = {0x11, 0x22, 0x33, 0x44}; + + gdb::array_view view (data + 1, data + 3); + + SELF_CHECK (!view.empty ()); + SELF_CHECK (view.size () == 2); + SELF_CHECK (view.data () == &data[1]); + SELF_CHECK (view[0] == data[1]); + SELF_CHECK (view[1] == data[2]); + + gdb_byte array[] = {0x11, 0x22, 0x33, 0x44}; + const gdb_byte *p1 = array; + gdb_byte *p2 = array + ARRAY_SIZE (array); + gdb::array_view view2 (p1, p2); +} + +/* Check construction with a pair of pointers of mixed constness. */ + +static void +check_ptr_ptr_mixed_cv () +{ + gdb_byte array[] = {0x11, 0x22, 0x33, 0x44}; + const gdb_byte *cp = array; + gdb_byte *p = array; + gdb::array_view view1 (cp, p); + gdb::array_view view2 (p, cp); + SELF_CHECK (view1.empty ()); + SELF_CHECK (view2.empty ()); +} + +/* Check range-for support (i.e., begin()/end()). This is a template + in order to test both gdb_byte and const gdb_byte. */ + +template +static void +check_range_for () +{ + T data[] = {1, 2, 3, 4}; + gdb::array_view view (data); + + typename std::decay::type sum = 0; + for (auto &elem : view) + sum += elem; + SELF_CHECK (sum == 1 + 2 + 3 + 4); +} + +/* Entry point. */ + +static void +run_tests () +{ + /* Empty views. */ + { + constexpr gdb::array_view view1; + constexpr gdb::array_view view2; + + static_assert (view1.empty (), ""); + static_assert (view1.data () == nullptr, ""); + static_assert (view1.size () == 0, ""); + static_assert (view2.empty (), ""); + static_assert (view2.size () == 0, ""); + static_assert (view2.data () == nullptr, ""); + } + + std::vector vec = {0x11, 0x22, 0x33, 0x44 }; + std::array array = {{0x11, 0x22, 0x33, 0x44}}; + + /* Various tests of views over std::vector. */ + { + gdb::array_view view = vec; + SELF_CHECK (check_container_view (view, vec)); + gdb::array_view cview = vec; + SELF_CHECK (check_container_view (cview, vec)); + } + + /* Likewise, over std::array. */ + { + gdb::array_view view = array; + SELF_CHECK (check_container_view (view, array)); + gdb::array_view cview = array; + SELF_CHECK (check_container_view (cview, array)); + } + + /* op=(std::vector/std::array/elem) */ + { + gdb::array_view view; + + view = vec; + SELF_CHECK (check_container_view (view, vec)); + view = std::move (vec); + SELF_CHECK (check_container_view (view, vec)); + + view = array; + SELF_CHECK (check_container_view (view, array)); + view = std::move (array); + SELF_CHECK (check_container_view (view, array)); + + gdb_byte elem = 0; + view = elem; + SELF_CHECK (check_elem_view (view, elem)); + view = std::move (elem); + SELF_CHECK (check_elem_view (view, elem)); + } + + /* Test copy/move ctor and mutable->immutable conversion. */ + { + gdb_byte data[] = {0x11, 0x22, 0x33, 0x44}; + gdb::array_view view1 = data; + gdb::array_view view2 = view1; + gdb::array_view view3 = std::move (view1); + gdb::array_view cview1 = data; + gdb::array_view cview2 = cview1; + gdb::array_view cview3 = std::move (cview1); + SELF_CHECK (view1[0] == data[0]); + SELF_CHECK (view2[0] == data[0]); + SELF_CHECK (view3[0] == data[0]); + SELF_CHECK (cview1[0] == data[0]); + SELF_CHECK (cview2[0] == data[0]); + SELF_CHECK (cview3[0] == data[0]); + } + + /* Same, but op=(view). */ + { + gdb_byte data[] = {0x55, 0x66, 0x77, 0x88}; + gdb::array_view view1; + gdb::array_view view2; + gdb::array_view view3; + gdb::array_view cview1; + gdb::array_view cview2; + gdb::array_view cview3; + + view1 = data; + view2 = view1; + view3 = std::move (view1); + cview1 = data; + cview2 = cview1; + cview3 = std::move (cview1); + SELF_CHECK (view1[0] == data[0]); + SELF_CHECK (view2[0] == data[0]); + SELF_CHECK (view3[0] == data[0]); + SELF_CHECK (cview1[0] == data[0]); + SELF_CHECK (cview2[0] == data[0]); + SELF_CHECK (cview3[0] == data[0]); + } + + /* op[] */ + { + std::vector vec = {0x11, 0x22}; + gdb::array_view view = vec; + gdb::array_view cview = vec; + + /* Check that op[] on a non-const view of non-const T returns a + mutable reference. */ + view[0] = 0x33; + SELF_CHECK (vec[0] == 0x33); + + /* OTOH, check that assigning through op[] on a view of const T + wouldn't compile. */ + SELF_CHECK (!check_op_subscript (cview)); + /* For completeness. */ + SELF_CHECK (check_op_subscript (view)); + } + + check_ptr_size_ctor (); + check_ptr_size_ctor (); + check_ptr_size_ctor2 (); + check_ptr_ptr_ctor (); + check_ptr_ptr_ctor (); + check_ptr_ptr_mixed_cv (); + + check_range_for (); + check_range_for (); + + /* Check that the right ctor overloads are taken when the element is + a container. */ + { + using Vec = std::vector; + Vec vecs[3]; + + gdb::array_view view_array = vecs; + SELF_CHECK (view_array.size () == 3); + + Vec elem; + gdb::array_view view_elem = elem; + SELF_CHECK (view_elem.size () == 1); + } +} + +} /* namespace array_view_tests */ +} /* namespace selftests */ + +void +_initialize_array_view_selftests () +{ + selftests::register_test (selftests::array_view_tests::run_tests); +}