From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22971 invoked by alias); 6 Apr 2017 02:15:51 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 22932 invoked by uid 89); 6 Apr 2017 02:15:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 06 Apr 2017 02:15:44 +0000 Received: by simark.ca (Postfix, from userid 33) id 090E11E81B; Wed, 5 Apr 2017 22:15:40 -0400 (EDT) To: Pedro Alves Subject: Re: [PATCH 2/2] Class-ify ptid_t X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 06 Apr 2017 02:15:00 -0000 From: Simon Marchi Cc: Simon Marchi , gdb-patches@sourceware.org In-Reply-To: References: <20170404183235.10589-1-simon.marchi@ericsson.com> <20170404183235.10589-2-simon.marchi@ericsson.com> <580e9a8a-d59b-095c-cf56-ee2f50fe46df@redhat.com> <84b33a5c655af3f344494c9e9ee473d6@polymtl.ca> Message-ID: <03bc3933d454f9fa01567a0e1000e201@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.4 X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00134.txt.bz2 On 2017-04-05 17:31, Pedro Alves wrote: >>> It's probably going to be worth it to sprinkle "constexpr" >>> all over the new API. Helps with static_asserts in >>> unit testing too. *cough* :-) >> >> Ok, will look into it. > > Thanks. constexpr in C++11 forces you to write a single > return statement (C++14 gets rid of that requirement), > but it looks quite doable. > > Also, note that it's not true that this type can't have a > constructor. It can, as long as the type remains POD. Ah, so I was just missing the defaulted default constructor. Adding it makes the type trivial, which then makes it POD. >> Is the following ok? >> >> struct thread_resume default_action { null_ptid }; > > ISTR that in C++11 you'll need the double "{{" levels, like: > > thread_resume default_action {{null_ptid}}; > > and that C++14 removed that requirement (brace elision). > But I haven't confirmed. Whatever works with -std=c++11. It seems to work with a single pair of braces with c++11. I'll still check that it does what we want at runtime, but I'd be surprised if it didn't do the right thing. > From 63f3c4ed145a069df68991fabbf025ffaedf8cf6 Mon Sep 17 00:00:00 2001 > From: Pedro Alves > Date: Wed, 5 Apr 2017 22:21:58 +0100 > Subject: [PATCH] POD > > --- > gdb/common/ptid.c | 8 +++---- > gdb/common/ptid.h | 68 > ++++++++++++++++++++++++++++--------------------------- > 2 files changed, 39 insertions(+), 37 deletions(-) > > diff --git a/gdb/common/ptid.c b/gdb/common/ptid.c > index ca51a4e..dff0071 100644 > --- a/gdb/common/ptid.c > +++ b/gdb/common/ptid.c > @@ -22,8 +22,8 @@ > > /* See ptid.h for these. */ > > -ptid_t null_ptid = ptid_t::build (0, 0, 0); > -ptid_t minus_one_ptid = ptid_t::build (-1, 0, 0); > +ptid_t null_ptid = ptid_t (0, 0, 0); > +ptid_t minus_one_ptid = ptid_t (-1, 0, 0); > > > /* See ptid.h. */ > @@ -31,7 +31,7 @@ ptid_t minus_one_ptid = ptid_t::build (-1, 0, 0); > ptid_t > ptid_build (int pid, long lwp, long tid) > { > - return ptid_t::build (pid, lwp, tid); > + return ptid_t (pid, lwp, tid); > } > > /* See ptid.h. */ > @@ -39,7 +39,7 @@ ptid_build (int pid, long lwp, long tid) > ptid_t > pid_to_ptid (int pid) > { > - return ptid_t::build (pid); > + return ptid_t (pid); > } > > /* See ptid.h. */ > diff --git a/gdb/common/ptid.h b/gdb/common/ptid.h > index c8649ae..1cb57e1 100644 > --- a/gdb/common/ptid.h > +++ b/gdb/common/ptid.h > @@ -20,6 +20,8 @@ > #ifndef PTID_H > #define PTID_H > > +#include > + > class ptid_t; > > /* The null or zero ptid, often used to indicate no process. */ > @@ -44,69 +46,65 @@ extern ptid_t minus_one_ptid; > class ptid_t > { > public: > - static ptid_t build (int pid, long lwp = 0, long tid = 0) > - { > - ptid_t ptid; > + /* Must have a trivial defaulted default constructor so that the > + type remains POD. */ > + ptid_t () noexcept = default; > > - ptid.m_pid = pid; > - ptid.m_lwp = lwp; > - ptid.m_tid = tid; > + constexpr ptid_t (int pid, long lwp = 0, long tid = 0) > + : m_pid (pid), m_lwp (lwp), m_tid (tid) > + {} > > - return ptid; > - } > - > - bool is_pid () const > + constexpr bool is_pid () const > { > - if (is_any () || is_null()) > - return false; > - > - return m_lwp == 0 && m_tid == 0; > + return (!is_any () > + && !is_null() > + && m_lwp == 0 > + && m_tid == 0); > } > > - bool is_null () const > + constexpr bool is_null () const > { > return *this == null_ptid; > } > > - bool is_any () const > + constexpr bool is_any () const > { > return *this == minus_one_ptid; > } > > - int pid () const > + constexpr int pid () const > { return m_pid; } > > - bool lwp_p () const > + constexpr bool lwp_p () const > { return m_lwp != 0; } > > - long lwp () const > + constexpr long lwp () const > { return m_lwp; } > > - bool tid_p () const > + constexpr bool tid_p () const > { return m_tid != 0; } > > - long tid () const > + constexpr long tid () const > { return m_tid; } > > - bool operator== (const ptid_t &other) const > + constexpr bool operator== (const ptid_t &other) const > { > return (m_pid == other.m_pid > && m_lwp == other.m_lwp > && m_tid == other.m_tid); > } > > - bool matches (const ptid_t &filter) const > + constexpr bool matches (const ptid_t &filter) const > { > - /* If filter represents any ptid, it's always a match. */ > - if (filter.is_any ()) > - return true; > - > - /* If filter is only a pid, any ptid with that pid matches. */ > - if (filter.is_pid () && m_pid == filter.pid ()) > - return true; > - > - /* Otherwise, this ptid only matches if it's exactly equal to > filter. */ > - return *this == filter; > + return (/* If filter represents any ptid, it's always a match. */ > + filter.is_any () > + /* If filter is only a pid, any ptid with that pid > + matches. */ > + || (filter.is_pid () && m_pid == filter.pid ()) > + > + /* Otherwise, this ptid only matches if it's exactly equal > + to filter. */ > + || *this == filter); > } > > private: > @@ -120,6 +118,10 @@ private: > long m_tid; > }; > > +static_assert (std::is_pod::value, ""); > + > +static_assert (ptid_t(1).pid () == 1, ""); Wow, nice. So all the tests are probably going to be static. Just to be clear, do you suggest that we make a test that ensures ptid_t is a POD, or you wrote that one just to show me it works? I We don't really care if it is, it's just that the current situation (it being used in another POD) requires it. > + > /* Make a ptid given the necessary PID, LWP, and TID components. */ > ptid_t ptid_build (int pid, long lwp, long tid); Thanks, Simon