[PATCH][check_GNU_style.sh] More aggressively ignore dg-xxx directives
Kyrill Tkachov
kyrylo.tkachov@foss.arm.com
Fri Oct 21 13:57:00 GMT 2016
Ping.
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00982.html
Thanks,
Kyrill
On 13/10/16 09:11, Kyrill Tkachov wrote:
>
> 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 <kyrylo.tkachov@arm.com>
>
> * check_GNU_style.sh (remove_testsuite): New function.
> Use it to remove testsuite from the diff.
>
>> Martin
>
More information about the Gcc-patches
mailing list