From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
cc: Noah Misch <noah@leadboat.com>,
        PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: cleaning perl code
In-reply-to: <0a4406d5-b428-4831-8ed9-c36764360f57@2ndQuadrant.com>
References: <e56f6fb6-38d5-8450-577c-24eb5d63f2e1@2ndQuadrant.com>
 <20200411043014.GA590991@rfd.leadboat.com>
 <0a4406d5-b428-4831-8ed9-c36764360f57@2ndQuadrant.com>
Comments: In-reply-to Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
	message dated "Sat, 11 Apr 2020 12:13:08 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <19842.1586623730.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sat, 11 Apr 2020 12:48:50 -0400
Message-ID: <19843.1586623730@sss.pgh.pa.us>
Precedence: bulk

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 4/11/20 12:30 AM, Noah Misch wrote:
>> In summary, among those warnings, I see non-negative value in "Code bef=
ore
>> warnings are enabled" only.  While we're changing this, I propose remov=
ing
>> Subroutines::RequireFinalReturn.  Implicit return values were not a mat=
erial
>> source of PostgreSQL bugs, yet we've allowed this to litter our code:

> That doesn't mean it won't be a source of problems in future, I've
> actually been bitten by this in the past.

Yeah, as I recall, the reason for the restriction is that if you fall out
without a "return", what's returned is the side-effect value of the last
statement, which might be fairly surprising.  Adding explicit "return;"
guarantees an undef result.  So when this does prevent a bug it could
be a pretty hard-to-diagnose one.  The problem is that it's a really
verbose/pedantic requirement for subs that no one ever examines the
result value of.

Is there a way to modify the test so that it only complains when
the final return is missing and there are other return(s) with values?
That would seem like a more narrowly tailored check.

			regards, tom lane