On Sat, Apr 11, 2020 at 11:15 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Noah Misch <noah@leadboat.com> writes:
> > In summary, among those warnings, I see non-negative value in "Code before
> > warnings are enabled" only. While we're changing this, I propose removing
> > Subroutines::RequireFinalReturn.
>
> If it's possible to turn off just that warning, then +several.
> It's routinely caused buildfarm failures, yet I can detect exactly
> no value in it. If there were sufficient cross-procedural analysis
> backing it to detect whether any caller examines the subroutine's
> result value, then it'd be worth having. But there isn't, so those
> extra returns are just pedantic verbosity.
We've actually gone out of our way to enable that particular warning.
See src/tools/perlcheck/perlcriticrc.
The idea of that warning is not entirely without merit, but in
practice it's usually pretty clear whether a function is intended to
return anything or not, and it's unlikely that someone is going to
rely on the return value when they really shouldn't be doing so. I'd
venture to suggest that the language is lax about this sort of thing
precisely because it isn't very important, and thus not worth
bothering users about.
I agree with Noah's comment about CPAN: it would be worth being more
careful about things like this if we were writing code that was likely
to be used by a wide variety of people and a lot of code over which we
have no control and which we do not get to even see. But that's not
the case here. It does not seem worth stressing the authors of TAP
tests over such things.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
