From patchwork Thu Oct 13 08:11:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrill Tkachov X-Patchwork-Id: 681655 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3svk3C6bV0z9s2Q for ; Thu, 13 Oct 2016 19:11:50 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=KBUhZaxl; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=apTgr0cLxyQyzx3+d 4OtITwGVqXxbS/dnvH9l3D+uBzjPCfyMwa4SKPO8fHKgRtu/7rdzynGfAHO6NKdC cftitJQ/io9jBbkHAcZ+FQ//IRtUmCTDqw8o4raWR1hikutc0glKfiqn+oc929Xx nOhywweujgoB6757HXqdyL+mEU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=g6GbT39utSFHSA8I5QOrKPa 97k8=; b=KBUhZaxlJqGNWcFBm18Fd1n1tsEu2N8Uq4EouXQvmlLDbnaDAVp8kzZ pHQ9KYaPuIcRhGeS+fkk4MVX1sDzFfliR04Asxka+4gUhXqKMgHt2OPQWbQUvEdB mnqgz+Hw7qXytxKZMGmP/04x0C5o/syh21M7+jDITH8y+Fhw756o= Received: (qmail 48899 invoked by alias); 13 Oct 2016 08:11:38 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 48879 invoked by uid 89); 13 Oct 2016 08:11:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.6 required=5.0 tests=BAYES_20, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=substr, 81, 7, $0, 1, 80, *dg X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 13 Oct 2016 08:11:27 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8A470105A; Thu, 13 Oct 2016 01:11:25 -0700 (PDT) Received: from [10.2.207.77] (e100706-lin.cambridge.arm.com [10.2.207.77]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A73AF3F32C; Thu, 13 Oct 2016 01:11:24 -0700 (PDT) Message-ID: <57FF41AB.4020301@foss.arm.com> Date: Thu, 13 Oct 2016 09:11:23 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Martin Sebor , Jakub Jelinek CC: Jeff Law , Bernd Schmidt , GCC Patches Subject: Re: [PATCH][check_GNU_style.sh] More aggressively ignore dg-xxx directives References: <57FCC339.3040902@foss.arm.com> <20161011105633.GT7282@tucnak.redhat.com> <26030f42-af5c-f9d5-1e26-a0f222b97220@redhat.com> <57FD03A7.5030409@foss.arm.com> <20161011191953.GA7282@tucnak.redhat.com> <57FE08FF.5020302@foss.arm.com> <57FE0DF4.2020603@foss.arm.com> <57FE2FDC.4020206@foss.arm.com> <7ea488ea-a604-5314-32cf-870e7bee7a01@gmail.com> In-Reply-To: <7ea488ea-a604-5314-32cf-870e7bee7a01@gmail.com> On 12/10/16 17:49, Martin Sebor wrote: > On 10/12/2016 06:43 AM, Kyrill Tkachov wrote: >> >> On 12/10/16 11:18, Kyrill Tkachov wrote: >>> >>> On 12/10/16 10:57, Kyrill Tkachov wrote: >>>> >>>> On 11/10/16 20:19, Jakub Jelinek wrote: >>>>> On Tue, Oct 11, 2016 at 01:11:04PM -0600, Martin Sebor wrote: >>>>>> Also, the pattern that starts with "/\+\+\+" looks like it's missing >>>>>> the ^ anchor. Presumably it should be "/^\+\+\+ \/testsuite\//". >>>>> No, it will be almost never +++ /testsuite/ >>>>> There needs to be .* in between "+++ " and "/testsuite/", and perhaps >>>>> it should also ignore "+++ testsuite/". >>>>> So /^\+\+\+ (.*\/)?testsuite\// ? >>>>> Also, normally (when matching $0) there won't be newlines in the text. >>>>> >>>>> Jakub >>>> >>>> Thanks. >>>> Here is the updated patch with your suggestions. >>>> >>> >>> Actually, I've encountered a problem: >>> >>> 85 # Remove the testsuite part of the diff. We don't care about GNU >>> style >>> 86 # in testcases and the dg-* directives give too many false positives. >>> 87 remove_testsuite () >>> 88 { >>> 89 awk 'BEGIN{testsuite=0} /\+\+\+ / && ! /testsuite\//{testsuite=0} \ >>> 90 {if (!testsuite) print} /^\+\+\+ >>> (.*\/)?testsuite\//{testsuite=1}' >>> 91 } >>> 92 >>> 93 grep $format '^+' $files \ >>> 94 | remove_testsuite \ >>> 95 | grep -v ':+++' \ >>> 96 > $inp >>> >>> >>> The /^\+\+\+ (.*\/)?testsuite\// doesn't ever match when the ^ anchor >>> is used. >>> The awk command matches fine by itself but not when fed from the "grep >>> $format '^+' $files" >>> command because grep adds the line numbers and file names. >>> So is it okay to omit the ^ here? > > I think the AWK regex will not work correctly when the patch has > the line number prefix like "1234: " (AFAICT, this can only happen > in the second invocation of the remove_testsuite function which > also has the problem below making me wonder if your testing > exercised that mode). > Huh, you're right, but it didn't cause problems in my testing, which is weird. > I think the AWK regex needs to be changed to handle that. It should > start with something like "^([1-9][0-9]*:)?\+\+\+" I think it needs to be ^(.*:)?([1-9][0-9]*:)?\+\+\+ because grep -nH would add the filename as well as the line number in the first invocation of remove_testsuite. This revision does that. > > I tried to test the patch but it doesn't seem to work. When passed > a patch as an argument it hangs. The hunk below isn't quite right: > > # Don't reuse $inp, which may be generated using -H and thus contain a > - # file prefix. > - grep -n '^+' $f \ > + # file prefix. Re-remove the testsuite since we're not using $inp. > + remove_testsuite $f \ > + | grep -n '^+' \ > | grep -v ':+++' \ > > $tmp > > The remove_testsuite function ignores arguments so passing $f to it > won't do anything except hang waiting for input. This should look > closer to this (it worked in my very limited testing): > > cat $f | remove_testsuite \ > Thanks for the help, Kyrill 2016-10-13 Kyrylo Tkachov * check_GNU_style.sh (remove_testsuite): New function. Use it to remove testsuite from the diff. > Martin diff --git a/contrib/check_GNU_style.sh b/contrib/check_GNU_style.sh index 87a276c9cf47b5e07c4407f740ce05dce1928c30..fb7494661ee8ff4d4e58ed05137bb69aab7c46a7 100755 --- a/contrib/check_GNU_style.sh +++ b/contrib/check_GNU_style.sh @@ -81,7 +81,17 @@ if [ $nfiles -eq 1 ]; then else format="-nH" fi + +# Remove the testsuite part of the diff. We don't care about GNU style +# in testcases and the dg-* directives give too many false positives. +remove_testsuite () +{ + awk 'BEGIN{testsuite=0} /^(.*:)?([1-9][0-9]*:)?\+\+\+ / && ! /testsuite\//{testsuite=0} \ + {if (!testsuite) print} /^(.*:)?([1-9][0-9]*:)?\+\+\+ (.*\/)?testsuite\//{testsuite=1}' +} + grep $format '^+' $files \ + | remove_testsuite \ | grep -v ':+++' \ > $inp @@ -160,8 +170,9 @@ col (){ fi # Don't reuse $inp, which may be generated using -H and thus contain a - # file prefix. - grep -n '^+' $f \ + # file prefix. Re-remove the testsuite since we're not using $inp. + cat $f | remove_testsuite \ + | grep -n '^+' \ | grep -v ':+++' \ > $tmp @@ -174,11 +185,10 @@ col (){ # Expand tabs to spaces according to tab positions. # Keep long lines, make short lines empty. Print the part past 80 chars # in red. - # Don't complain about dg-xxx directives in tests. cat "$tmp" \ | sed 's/^[0-9]*:+//' \ | expand \ - | awk '$0 !~ /{[[:space:]]*dg-(error|warning|message)[[:space:]]/ { \ + | awk '{ \ if (length($0) > 80) \ printf "%s\033[1;31m%s\033[0m\n", \ substr($0,1,80), \