Message ID | 8f9ccb7e-27cb-63a1-05d4-044b00a078d3@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 60028 invoked by alias); 5 Jun 2018 17:32:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 59021 invoked by uid 89); 5 Jun 2018 17:32:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=rolled X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 05 Jun 2018 17:32:00 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8B41D401EF0B; Tue, 5 Jun 2018 17:31:58 +0000 (UTC) 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 5264563F32; Tue, 5 Jun 2018 17:31:57 +0000 (UTC) Subject: Re: [PATCH (for gdb)] enum-flags.h: Add trailing semicolon to example in comment To: David Malcolm <dmalcolm@redhat.com> References: <5c0b4830-4c06-adb2-5cfb-60c30d876ff6@redhat.com> <1528221387-44895-1-git-send-email-dmalcolm@redhat.com> Cc: Trevor Saunders <tbsaunde@tbsaunde.org>, Richard Biener <richard.guenther@gmail.com>, GCC Patches <gcc-patches@gcc.gnu.org>, GDB Patches <gdb-patches@sourceware.org> From: Pedro Alves <palves@redhat.com> Message-ID: <8f9ccb7e-27cb-63a1-05d4-044b00a078d3@redhat.com> Date: Tue, 5 Jun 2018 18:31:56 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <1528221387-44895-1-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit |
Commit Message
Pedro Alves
June 5, 2018, 5:31 p.m. UTC
[adding gdb-patches] On 06/05/2018 06:56 PM, David Malcolm wrote: > On Tue, 2018-06-05 at 17:13 +0100, Pedro Alves wrote: >> On 06/05/2018 03:49 PM, David Malcolm wrote: >>> On Tue, 2018-06-05 at 04:40 -0400, Trevor Saunders wrote: >>>> You may want to look at gdb's enum-flags.h which I think already >>>> implements what your doing here. >>> >>> Aha! Thanks! >>> >>> Browsing the git web UI, that gdb header was introduced by Pedro >>> Alves >>> in: >>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit >>> diff;h=8d297bbf604c8318ffc72d5a7b3db654409c5ed9 >>> and the current state is here: >>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f >>> =gdb/common/enum-flags.h;hb=HEAD >>> >>> I'll try this out with GCC; hopefully it works with C++98. >> >> It should -- it was written/added while GDB still required C++98. > > Thanks. > >> Note I have a WIP series here: >> >> https://github.com/palves/gdb/commits/palves/cxx11-enum-flags >> >> that fixes a few things, and adds a bunch of unit tests. In >> the process, it uses C++11 constructs (hence the branch's name), >> but I think that can be reverted to still work with C++98. >> >> I had seen some noises recently about GCC maybe considering >> requiring C++11. Is that reasonable to expect in this cycle? >> (GDB requires GCC 4.8 or later, FWIW.) > > I'm expecting that GCC 9 will still require C++98. OK. > >> Getting that branch into master had fallen lower on my TODO, >> but if there's interest, I can try to finish it off soon, so we >> can share a better baseline. (I posted it once, but found >> some issues which I fixed since but never managed to repost.) > > Interestingly, it looks like gdb is using Google Test for selftests; > gcc is using a hand-rolled API that is similar, but has numerous > differences (e.g. explicit calling of test functions, rather than > implicit test discovery). So that's another area of drift between > the projects. Nope, the unit tests framework is hand rolled too. See gdb/selftest.h/c. It can be invoked with gdb's "maint selftest" command. You can see the list of tests with "maint info selftests", and you can pass a filter to "maint selftest" too. > >>> >>> Presumably it would be good to share this header between GCC and >>> GDB; >>> CCing Pedro; Pedro: hi! Does this sound sane? >>> (for reference, the GCC patch we're discussing here is: >>> https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01685.html ) >> >> Sure! > > Alternatively, if GCC needs to stay at C++98 and GDB moves on to C++11, > then we can at least take advantage of this tested and FSF-assigned > C++98 code (better than writing it from scratch). Agreed, but I'll try to see about making the fixes in the branch C++98 compatible. > > I ran into one issue with the header, e.g. with: > > /* Dump flags type. */ > DEF_ENUM_FLAGS_TYPE(enum dump_flag, dump_flags_t); > > This doesn't work: > TDF_SLIM | flags > but this does: > flags | TDF_SLIM > where TDF_SLIM is one of the values of "enum dump_flag". ISTR that that's fixed in the branch. > BTW, I spotted this trivial issue in a comment in enum-flags.h whilst > trying it out for gcc: > > > > The DEF_ENUM_FLAGS_TYPE macro should be used with a trailing > semicolon, but the example in the comment lacks one. > > * enum-flags.h: Add trailing semicolon to example in comment. Thanks. I've merged it. From 115f7325b5b76450b9a1d16848a2a54f54e49747 Mon Sep 17 00:00:00 2001 From: David Malcolm <dmalcolm@redhat.com> Date: Tue, 5 Jun 2018 18:22:25 +0100 Subject: [PATCH] Fix typo in common/enum-flags.h example The DEF_ENUM_FLAGS_TYPE macro should be used with a trailing semicolon, but the example in the comment lacks one. gdb/ChangeLog: 2018-06-05 David Malcolm <dmalcolm@redhat.com> * common/enum-flags.h: Add trailing semicolon to example in comment. --- gdb/ChangeLog | 5 +++++ gdb/common/enum-flags.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)
Comments
On Tue, 2018-06-05 at 18:31 +0100, Pedro Alves wrote: > [adding gdb-patches] > > On 06/05/2018 06:56 PM, David Malcolm wrote: > > On Tue, 2018-06-05 at 17:13 +0100, Pedro Alves wrote: > > > On 06/05/2018 03:49 PM, David Malcolm wrote: > > > > On Tue, 2018-06-05 at 04:40 -0400, Trevor Saunders wrote: > > > > > You may want to look at gdb's enum-flags.h which I think > > > > > already > > > > > implements what your doing here. > > > > > > > > Aha! Thanks! > > > > > > > > Browsing the git web UI, that gdb header was introduced by > > > > Pedro > > > > Alves > > > > in: > > > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=co > > > > mmit > > > > diff;h=8d297bbf604c8318ffc72d5a7b3db654409c5ed9 > > > > and the current state is here: > > > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=bl > > > > ob;f > > > > =gdb/common/enum-flags.h;hb=HEAD > > > > > > > > I'll try this out with GCC; hopefully it works with C++98. > > > > > > It should -- it was written/added while GDB still required C++98. > > > > Thanks. > > > > > Note I have a WIP series here: > > > > > > https://github.com/palves/gdb/commits/palves/cxx11-enum-flags > > > > > > that fixes a few things, and adds a bunch of unit tests. In > > > the process, it uses C++11 constructs (hence the branch's name), > > > but I think that can be reverted to still work with C++98. > > > > > > I had seen some noises recently about GCC maybe considering > > > requiring C++11. Is that reasonable to expect in this cycle? > > > (GDB requires GCC 4.8 or later, FWIW.) > > > > I'm expecting that GCC 9 will still require C++98. > > OK. > > > > > > Getting that branch into master had fallen lower on my TODO, > > > but if there's interest, I can try to finish it off soon, so we > > > can share a better baseline. (I posted it once, but found > > > some issues which I fixed since but never managed to repost.) > > > > Interestingly, it looks like gdb is using Google Test for > > selftests; > > gcc is using a hand-rolled API that is similar, but has numerous > > differences (e.g. explicit calling of test functions, rather than > > implicit test discovery). So that's another area of drift between > > the projects. > > Nope, the unit tests framework is hand rolled too. See > gdb/selftest.h/c. > It can be invoked with gdb's "maint selftest" command. > You can see the list of tests with "maint info selftests", and > you can pass a filter to "maint selftest" too. Aha. Thanks. Looks like gdb and gcc each have different hand-rolled selftest APIs: * gdb's selftest.h/c has manual test registration; test failures are handled via exceptions * gcc's selftest.h/c has no test registration (test functions are called explicitly); test failures are handled via "abort" I'm not suggesting we try to unify these APIs at this time. > > > > > > > > Presumably it would be good to share this header between GCC > > > > and > > > > GDB; > > > > CCing Pedro; Pedro: hi! Does this sound sane? > > > > (for reference, the GCC patch we're discussing here is: > > > > https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01685.html ) > > > > > > Sure! > > > > Alternatively, if GCC needs to stay at C++98 and GDB moves on to > > C++11, > > then we can at least take advantage of this tested and FSF-assigned > > C++98 code (better than writing it from scratch). > > Agreed, but I'll try to see about making the fixes in the branch > C++98 compatible. Thanks. > > > > I ran into one issue with the header, e.g. with: > > > > /* Dump flags type. */ > > DEF_ENUM_FLAGS_TYPE(enum dump_flag, dump_flags_t); > > > > This doesn't work: > > TDF_SLIM | flags > > but this does: > > flags | TDF_SLIM > > where TDF_SLIM is one of the values of "enum dump_flag". > > ISTR that that's fixed in the branch. Interesting; I'll have a look. > > BTW, I spotted this trivial issue in a comment in enum-flags.h > > whilst > > trying it out for gcc: > > > > > > > > The DEF_ENUM_FLAGS_TYPE macro should be used with a trailing > > semicolon, but the example in the comment lacks one. > > > > * enum-flags.h: Add trailing semicolon to example in comment. > > Thanks. I've merged it. Thanks. [...snip...] Dave
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 85c62dbd86c..54205a6ea85 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2018-06-05 David Malcolm <dmalcolm@redhat.com> + + * common/enum-flags.h: Add trailing semicolon to example in + comment. + 2018-06-05 Tom Tromey <tom@tromey.com> PR cli/12326: diff --git a/gdb/common/enum-flags.h b/gdb/common/enum-flags.h index 65732c11a46..82568a5a3d9 100644 --- a/gdb/common/enum-flags.h +++ b/gdb/common/enum-flags.h @@ -32,7 +32,7 @@ flag_val3 = 1 << 3, flag_val4 = 1 << 4, }; - DEF_ENUM_FLAGS_TYPE(enum some_flag, some_flags) + DEF_ENUM_FLAGS_TYPE(enum some_flag, some_flags); some_flags f = flag_val1 | flag_val2; f |= flag_val3;