> On Apr 16, 2020, at 2:07 PM, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
>
>
> On 4/16/20 11:12 AM, Alvaro Herrera wrote:
>> On 2020-Apr-16, Hamlin, Garick L wrote:
>>
>>> With the old expression 'something;' would be stripped away.
>>> Is that an issue where this this is used? Why are we parsing
>>> these headers?
>> These are files from which bootstrap catalog data is generated, which is
>> why we parse from Perl; but also where C structs are declared, which is
>> why they're C.
>>
>> I think switching to non-greedy is a win in itself. Non-capturing
>> parens is probably a wash (this doesn't run often so the performance
>> argument isn't very interesting).
>
>
> Yeah, I'm inclined to fix this independently of the perlcritic stuff.
> The change is more readable and more correct as well as being perlcritic
> friendly.
>
>
> I might take a closer look at Catalog.pm.
>
>
> Meanwhile, the other regex highlighted in the patch, in Solution.pm:
>
>
> if (/^AC_INIT\(\[([^\]]+)\], \[([^\]]+)\], \[([^\]]+)\], \[([^\]]*)\],
> \[([^\]]+)\]/)
>
>
> is sufficiently horrid that I think we should see if we can rewrite it,
my $re = qr/
\[ # literal opening bracket
( # Capture anything but a closing bracket
(?> # without backtracking
[^\]]+
)
)
\] # literal closing bracket
/x;
if (/^AC_INIT\($re, $re, $re, $re, $re/)
> maybe as an extended regex. And a better fix here instead of marking the
> fourth group as non-capturing would be simply to get rid of the parens
> altogether. The serve no purpose at all.
But then you'd have to use something else in position 4, which complicates the code.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
