From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117463 invoked by alias); 17 Feb 2020 11:01:46 -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 117425 invoked by uid 89); 17 Feb 2020 11:01:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-qk1-f196.google.com Received: from mail-qk1-f196.google.com (HELO mail-qk1-f196.google.com) (209.85.222.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Feb 2020 11:01:44 +0000 Received: by mail-qk1-f196.google.com with SMTP id h4so15800617qkm.0 for ; Mon, 17 Feb 2020 03:01:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=XjijHh+cSzzcSS6oThGZx1Bu1O1nkOUi33U1H3p+004=; b=qwvgJ6PqvVPTY3B+JWbS3ev5OzoYTrmhUoIirPwkcilFN+QZozCBgSEligSW4XHheh UUgdvYU6JMx9JaQ4i+RCoNWquCZi/tnwBvH212p4tjKmfl77ievTjqeH+xB01Jg86+3f 2ypAA1U0r9RGM2ERipSRcc+l43jL8lbYV9x7Ge+WRSMA5BGPgNpwBiad97hHNaBs56j0 QXArAbsX7fFOZE/q64l7u9SNINCVMWBVVAyIiWBKCn2hMO5CcDzeIyW6wG1W8re0kKo7 zdPcLpXnDDez0Nu9gw0aXIxvy8y9n4w3jPlOh3iP4JalksclMOtT0gi2igVfnMKImXae C8Aw== Return-Path: Received: from [192.168.0.185] ([191.34.156.101]) by smtp.gmail.com with ESMTPSA id h20sm24848qkk.64.2020.02.17.03.01.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 17 Feb 2020 03:01:41 -0800 (PST) Subject: Re: [PATCH 3/5] gdb: allow duplicate enumerators in flag enums To: Simon Marchi , gdb-patches@sourceware.org References: <20200213203035.30157-1-simon.marchi@efficios.com> <20200213203035.30157-3-simon.marchi@efficios.com> From: Luis Machado Message-ID: Date: Mon, 17 Feb 2020 11:01:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200213203035.30157-3-simon.marchi@efficios.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00611.txt.bz2 On 2/13/20 5:30 PM, Simon Marchi wrote: > I have come across some uses cases where it would be desirable to treat > an enum that has duplicate values as a "flag enum". For example, this > one here [1]: > > enum membarrier_cmd { > MEMBARRIER_CMD_QUERY = 0, > MEMBARRIER_CMD_GLOBAL = (1 << 0), > MEMBARRIER_CMD_GLOBAL_EXPEDITED = (1 << 1), > MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED = (1 << 2), > MEMBARRIER_CMD_PRIVATE_EXPEDITED = (1 << 3), > MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = (1 << 4), > MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE = (1 << 5), > MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE = (1 << 6), > > /* Alias for header backward compatibility. */ > MEMBARRIER_CMD_SHARED = MEMBARRIER_CMD_GLOBAL, > }; > > The last enumerator is kept for backwards compatibility. Without this > patch, this enumeration wouldn't be considered a flag enum, because two > enumerators collide. With this patch, it would be considered a flag > enum, and the value 3 would be printed as: > > MEMBARRIER_CMD_GLOBAL | MEMBARRIER_CMD_GLOBAL_EXPEDITED > Sounds reasonable to me. > Although if people prefer, we could display both MEMBARRIER_CMD_GLOBAL > and MEMBARRIER_CMD_SHARED in the result. It wouldn't be wrong, and > could perhaps be useful in case a bit may have multiple meanings > (depending on some other bit value). It would be fine either way for me. Since it is a backwards compatibility value, i guess it will be less and less used as time goes by. Not sure if it would be great to keep displaying it when it is no longer so useful. I'd keep just one value. If others want it, maybe we could enable it via a separate display option. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/membarrier.h?id=0bf999f9c5e74c7ecf9dafb527146601e5c848b9#n125 > --- > gdb/dwarf2/read.c | 5 ----- > gdb/testsuite/gdb.base/printcmds.c | 7 ++++--- > 2 files changed, 4 insertions(+), 8 deletions(-) > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index b866cc2d5747..5c34667ef36d 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -15495,7 +15495,6 @@ update_enumeration_type_from_children (struct die_info *die, > struct die_info *child_die; > int unsigned_enum = 1; > int flag_enum = 1; > - ULONGEST mask = 0; > > auto_obstack obstack; > > @@ -15533,10 +15532,6 @@ update_enumeration_type_from_children (struct die_info *die, > > if (nbits != 0 && nbits && nbits != 1) > flag_enum = 0; > - else if ((mask & value) != 0) > - flag_enum = 0; > - else > - mask |= value; > } > > /* If we already know that the enum type is neither unsigned, nor > diff --git a/gdb/testsuite/gdb.base/printcmds.c b/gdb/testsuite/gdb.base/printcmds.c > index f0b4fa4b86b1..81d2efe1a037 100644 > --- a/gdb/testsuite/gdb.base/printcmds.c > +++ b/gdb/testsuite/gdb.base/printcmds.c > @@ -99,9 +99,10 @@ volatile enum some_volatile_enum some_volatile_enum = enumvolval1; > /* An enum considered as a "flag enum". */ > enum flag_enum > { > - FE_NONE = 0x00, > - FE_ONE = 0x01, > - FE_TWO = 0x02, > + FE_NONE = 0x00, > + FE_ONE = 0x01, > + FE_TWO = 0x02, > + FE_TWO_LEGACY = 0x02, > }; > > enum flag_enum three = FE_ONE | FE_TWO; > LGTM.