From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24664 invoked by alias); 17 Feb 2020 17:40:55 -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 24648 invoked by uid 89); 17 Feb 2020 17:40:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.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-qt1-f193.google.com Received: from mail-qt1-f193.google.com (HELO mail-qt1-f193.google.com) (209.85.160.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Feb 2020 17:40:52 +0000 Received: by mail-qt1-f193.google.com with SMTP id c5so12578066qtj.6 for ; Mon, 17 Feb 2020 09:40:52 -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=bXSXgsfmjrZwmG2y4TQFy0w2lbhmNa+NtZs+7pWJLow=; b=nwu1boXKUuGQXvAhz88Mb0H+RWFqpw/N2V+XNS0QSKi8Sb2iI7TN2keVYRQzeHPdVN j8u4qzStE48ccJXZcbcHpa+6IlfaHQ8LPY5UAsfGFz9N5988ELfD1DVV+u86MBu5lWcc FvT2mzvinGR5BjKniRPNKtKx9jNv6WgAUfkVUxi3PPdkNHqYhM5C6yU31fbFJZfjtamf b1DsU0ZEyt0dir0yqJfWVVJum1Ec4AE+vA/Hcu+RwJisLmefuI5LByvXJRnBYo+RPsoa v+/OzRjU6Nb0U2NOgaH8Nfi9Gln/WbD0CV7XOEQnH9iNyQJqqukGYCN2FVOJRy4fvpzU cR7g== Return-Path: Received: from [192.168.0.185] ([191.34.156.101]) by smtp.gmail.com with ESMTPSA id s42sm506191qtk.87.2020.02.17.09.40.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 17 Feb 2020 09:40:49 -0800 (PST) Subject: Re: [PATCH 2/5] gdb: fix printing of flag enums with multi-bit enumerators To: Simon Marchi , Simon Marchi , gdb-patches@sourceware.org References: <20200213203035.30157-1-simon.marchi@efficios.com> <20200213203035.30157-2-simon.marchi@efficios.com> <2e83ba29-99c4-d611-45ec-ee98ac81fc34@linaro.org> <78d03fba-9901-01d5-c1ed-9631986e2579@simark.ca> From: Luis Machado Message-ID: Date: Mon, 17 Feb 2020 17:40: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: <78d03fba-9901-01d5-c1ed-9631986e2579@simark.ca> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00684.txt.bz2 On 2/17/20 2:27 PM, Simon Marchi wrote: > On 2020-02-17 5:56 a.m., Luis Machado wrote: >>> @@ -15526,10 +15527,17 @@ update_enumeration_type_from_children (struct die_info *die, >>>         unsigned_enum = 0; >>>         flag_enum = 0; >>>       } >>> -      else if ((mask & value) != 0) >>> -    flag_enum = 0; >>>         else >>> -    mask |= value; >>> +    { >>> +      int nbits = count_one_bits_ll (value); >>> + >>> +      if (nbits != 0 && nbits && nbits != 1) >> >> Isn't this the same as nbits >= 2? popcount shouldn't return a negative number, should it? > > I think I wrote that because count_one_bits_ll returns a signed int, so I > indeed thought "what if it returns a negative number". But if it did, there > would be some quite more serious problems, so we probably don't have to think > about it here. I'll change it as "nbits >= 2". I did some research and did not see a clear reason why popcount returns an int. There was a mention of popcount returning negative for some obscure implementation if it was passed a negative number. GCC's documentation doesn't make it clear either. > > Oh and there was a spurious "&& nbits" in there. > >> >>> +        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 >>>        a flag type, no need to look at the rest of the enumerates.  */ >>> diff --git a/gdb/testsuite/gdb.base/printcmds.c b/gdb/testsuite/gdb.base/printcmds.c >>> index 57e04e6c01f3..f0b4fa4b86b1 100644 >>> --- a/gdb/testsuite/gdb.base/printcmds.c >>> +++ b/gdb/testsuite/gdb.base/printcmds.c >>> @@ -96,9 +96,35 @@ enum some_volatile_enum { enumvolval1, enumvolval2 }; >>>      name.  See PR11827.  */ >>>   volatile enum some_volatile_enum some_volatile_enum = enumvolval1; >>>   -enum flag_enum { ONE = 1, TWO = 2 }; >>> +/* An enum considered as a "flag enum".  */ >>> +enum flag_enum >>> +{ >>> +  FE_NONE = 0x00, >>> +  FE_ONE  = 0x01, >>> +  FE_TWO  = 0x02, >>> +}; >>> + >>> +enum flag_enum three = FE_ONE | FE_TWO; >>> + >>> +/* Another enum considered as a "flag enum", but with enumerator with value >>> +   0.  */ >>> +enum flag_enum_without_zero >>> +{ >>> +  FEWZ_ONE = 0x01, >>> +  FEWZ_TWO = 0x02, >>> +}; >>> + >> >> Typo maybe? There is no enum with value 0 in flag_enum_without_zero. Maybe you meant flag_enum to contain a 0 value with FE_NONE? >> >>> +enum flag_enum_without_zero flag_enum_without_zero = 0; >>> + >> >> Or maybe you were referring to the above? > > Do you mean a typo in the comment, or the type name? Because there indeed seems > to be a typo, it should read "but with no enumerator with value", not > "but with enumerator with value". Sorry, i should've been more clear. I meant the comment saying "but with enumerator with value". You guessed it right.