develooper Front page | perl.perl5.porters | Postings from December 2018

Re: regexec.c: LGTM analysis warns about two comparisons, butweactually need them

Thread Previous | Thread Next
From:
Karl Williamson
Date:
December 8, 2018 04:20
Subject:
Re: regexec.c: LGTM analysis warns about two comparisons, butweactually need them
Message ID:
be6c06bb-329d-eb34-efa8-11b986626892@khwilliamson.com
On 12/6/18 12:54 PM, Karl Williamson wrote:
> On 12/6/18 6:30 AM, Dominic Hargreaves wrote:
>> On Thu, Dec 06, 2018 at 08:20:50AM -0500, James E Keenan wrote:
>>> On 12/6/18 3:16 AM, Tony Cook wrote:
>>>> On Wed, Dec 05, 2018 at 10:28:54PM -0500, James E Keenan wrote:
>>>>> Here is a case where the LGTM.com analysis of the Perl 5 source code
>>>>> produces results that are at first plausible but ultimately incorrect.
>>>>>
>>>>> The following section of regexec.c is cited 
>>>>> (https://lgtm.com/projects/g/Perl/perl5/alerts/?mode=tree&ruleFocus=2154840804) 
>>>>>
>>>>> with the warning-level alert "Comparison result is always the same":
>>>>
>>>> This was discussed in #133686, the comparison needs to stay to allow
>>>> for local configuration of larger values of NUM_ANYOF_CODE_POINTS.
>>>>
>>>
>>> Would it be okay if I inserted a comment to that effect into 
>>> regexec.c so
>>> that we don't stumble upon this again in the future?
> 
> I have pushed a branch with this commit that changes those numbers to 
> symbols to make things clear
> 
> https://perl5.git.perl.org/perl.git/commitdiff/ef5a8af164793929880381dcf36b727440050d44 

The branch has not been pushed to blead as g540fa6e13e.  Hopefully this 
fixes the LGTM notice, as the hard-coded numbers have been replaced by 
the appropriate mnemonic.
> 
>>
>> That sounds sensible to me regardless of systems like LGTM. You *could*
>> even use the LGTM suppression syntax (in addition to a human-readable
>> explanation); this would mean that if in the future we can implement
>> /* */ suppression comments in LGTM, they would be recognised.
>>
>> You can watch this topic for more updates on that feature request:
>>
>> https://discuss.lgtm.com/t/rfe-support-for-supression-comments-in-comment-blocks/1364 
>>
>>
>> In case you weren't aware, if you think that a certain type of alert
>> will *never* be interesting for a given code-base you can disable it
>> entirely in .lgtm.yml:
>>
>> https://lgtm.com/help/lgtm/showing-hiding-query-results
>>
>> Cheers,
>> Dominic.
>>
> 

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About