From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35400 invoked by alias); 30 Oct 2017 15:57:17 -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 35366 invoked by uid 89); 30 Oct 2017 15:57:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=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=H*MI:sk:1509373, H*f:sk:1509373, H*i:sk:1509373 X-HELO: mail.baldwin.cx Received: from bigwig.baldwin.cx (HELO mail.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Oct 2017 15:57:14 +0000 Received: from dhcp-10-248-127-128.eduroam.wireless.private.cam.ac.uk (global-5-144.nat-2.net.cam.ac.uk [131.111.5.144]) by mail.baldwin.cx (Postfix) with ESMTPSA id A156410A87D; Mon, 30 Oct 2017 11:57:12 -0400 (EDT) Subject: Re: [PATCH] Introduce in_inclusive_range, fix -Wtautological-compare warnings To: Simon Marchi , gdb-patches@sourceware.org References: <189cefb7-aa0c-16bd-d78c-cc6f1e5c1344@baldwin.cx> <1509373931-16340-1-git-send-email-simon.marchi@ericsson.com> From: John Baldwin Message-ID: Date: Mon, 30 Oct 2017 15:57:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1509373931-16340-1-git-send-email-simon.marchi@ericsson.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00898.txt.bz2 On 10/30/17 2:32 PM, Simon Marchi wrote: > When compiling with clang or gcc 8, we see warnings like this: > > /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:10013:13: error: comparison of 0 <= unsigned expression is always true [-Werror,-Wtautological-compare] > if (0 <= insn_op1 && 3 >= insn_op1) > ~ ^ ~~~~~~~~ > /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:11722:20: error: comparison of unsigned expression >= 0 is always true [-Werror,-Wtautological-compare] > else if (opB >= 0 && opB <= 2) > ~~~ ^ ~ > > This is because an unsigned integer (opB in this case) will always be >= > 0. It is still useful to keep both bounds of the range in the > expression, even if one is at the edge of the data type range. This > patch introduces a utility function in_inclusive_range that gets rid of > the warning while conveying that we are checking for a range. > > Tested by rebuilding. > > gdb/ChangeLog: > > * common/common-utils.h (in_inclusive_range): New function. > * arm-tdep.c (arm_record_extension_space): Use > in_inclusive_range. > * cris-tdep.c (cris_spec_reg_applicable): Use > in_inclusive_range. > --- > gdb/arm-tdep.c | 4 ++-- > gdb/common/common-utils.h | 9 +++++++++ > gdb/cris-tdep.c | 4 ++-- > 3 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c > index d623eb6..209e29f 100644 > --- a/gdb/cris-tdep.c > +++ b/gdb/cris-tdep.c > @@ -1434,7 +1434,7 @@ cris_spec_reg_applicable (struct gdbarch *gdbarch, > /* Indeterminate/obsolete. */ > return 0; > case cris_ver_v0_3: > - return (version >= 0 && version <= 3); > + return in_inclusive_range (version, 0U, 3U); > case cris_ver_v3p: > return (version >= 3); > case cris_ver_v8: > @@ -1442,7 +1442,7 @@ cris_spec_reg_applicable (struct gdbarch *gdbarch, > case cris_ver_v8p: > return (version >= 8); > case cris_ver_v0_10: > - return (version >= 0 && version <= 10); > + return in_inclusive_range (version, 0U, 10U); > case cris_ver_v3_10: > return (version >= 3 && version <= 10); > case cris_ver_v8_10: I wonder if in this file it wouldn't be best to use the new function throughout the various cases so that the style is more consistent? LGTM regardless. -- John Baldwin