From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 44279 invoked by alias); 30 Oct 2017 16:01:33 -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 44265 invoked by uid 89); 30 Oct 2017 16:01:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Oct 2017 16:01:26 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v9UG1JaQ010556 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 30 Oct 2017 12:01:24 -0400 Received: by simark.ca (Postfix, from userid 112) id 6F6B31E533; Mon, 30 Oct 2017 12:01:19 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 9A9D01E4C4; Mon, 30 Oct 2017 12:01:18 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 30 Oct 2017 16:01:00 -0000 From: Simon Marchi To: John Baldwin Cc: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH] Introduce in_inclusive_range, fix -Wtautological-compare warnings In-Reply-To: References: <189cefb7-aa0c-16bd-d78c-cc6f1e5c1344@baldwin.cx> <1509373931-16340-1-git-send-email-simon.marchi@ericsson.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 30 Oct 2017 16:01:19 +0000 X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00901.txt.bz2 On 2017-10-30 11:57, John Baldwin wrote: > 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. Good point, I'll make that change. I'll see if it's possible in arm-tdep.c too or if it's a too daunting task. Simon