From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | "Hamlin, Garick L" <ghamlin(at)isc(dot)upenn(dot)edu> |
Cc: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: cleaning perl code |
Date: | 2020-04-16 15:12:48 |
Message-ID: | 20200416151248.GA8282@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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).
An example. This eval in Catalog.pm
+ ## no critic (ProhibitStringyEval)
+ ## no critic (RequireCheckingReturnValueOfEval)
+ eval '$hash_ref = ' . $_;
is really weird stuff generally speaking, and the fact that we have to
mark it specially for critic is a good indicator of that -- it serves as
documentation. Catalog.pm is all a huge weird hack, but it's a critically
important hack. Heck, what about RequireCheckingReturnValueOfEval --
should we instead consider actually checking the return value of eval?
It would seem to make sense, would it not? (Not for this patch, though
-- I would be fine with just adding the nocritic line now, and removing
it later while fixing that).
All in all, I think it's a positive value in having this code be checked
with a bit more strength -- checks that are pointless in, say, t/00*.pl
prove files.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-04-16 15:39:06 | Re: Race condition in SyncRepGetSyncStandbysPriority |
Previous Message | Pierre Giraud | 2020-04-16 15:12:28 | Re: Poll: are people okay with function/operator table redesign? |