On 10/11/2016 09:22 AM, Kyrill Tkachov wrote:
On 11/10/16 16:13, Jeff Law wrote:
On 10/11/2016 05:01 AM, Bernd Schmidt wrote:
On 10/11/2016 12:56 PM, Jakub Jelinek wrote:
On Tue, Oct 11, 2016 at 11:47:21AM +0100, Kyrill Tkachov wrote:
check_GNU_style.sh complains a lot about dg-* directives in the
testsuite and in particular about line lengths.
There's nothing we can do about the directives and sometimes they're
supposed to be long, in particular the scan-assembler
checks in dg-final. Currently check_GNU_style.sh has code to avoid
warning for dg-* directives but it's too weak, it doesn't
catch dg-final or dg-options directives.
This patch makes the code ignore all "{ dg-.*" lines for length
purposes.
This eliminates a lot of false positives in my patches and didn't
filter any legitimate warnings in my patches.
I wonder if we just shouldn't ignore all line lengths in testcases (or
perhaps any coding style whatsoever). Some testcases are meant to be
formatted more-less according to our coding style (but, e.g.
/* PR tree-optimization/12345 */
comments never end with . and 2 spaces), except for dg- directives, but
others are entered in whatever form they came from the reporter.
I agree, probably best not to check testcases for style (but then I
don't trust automatic checkers anyway).
Agreed that checking testcases for style isn't desirable.
Ok, here's a patch that does that by filtering out everything between
"+++.*testsuite/" and "+++<not testsuite>".
Is this is an ok approach?
This eliminates all testsuite warnings on a few of my patches that I
tried it out on.
Disclaimer: I'm not an awk expert.
I've been meaning to propose a similar change for some time so thanks!
Just a couple of minor comments (nits really) about the AWK script.
+# 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} /^\+\+\+[[:space:]]/ && !
/testsuite/{testsuite=0} \
+ {if (!testsuite) print}
/\+\+\+[[:space:]].*testsuite\//{testsuite=1}'
+}
In the standard (POSIX) diff format, modified file names are prefixed
by a line matching the "^+++ [^ /]" regular expression (i.e., with
exactly one space after +++). [[:space:]] matches other whitespace
characters including newline and carriage return in the POSIX locale,
and possibly other characters in named locales. Using a literal space
in the regex should be safer.
Also, the pattern that starts with "/\+\+\+" looks like it's missing
the ^ anchor. Presumably it should be "/^\+\+\+ \/testsuite\//".
Martin
