From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: Noah Misch <noah@leadboat.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: cleaning perl code
Date: Sat, 11 Apr 2020 12:48:50 -0400
Message-ID: <19843.1586623730@sss.pgh.pa.us> (raw)
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>
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 before
>> warnings are enabled" only. While we're changing this, I propose removing
>> Subroutines::RequireFinalReturn. Implicit return values were not a material
>> 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andrew.dunstan@2ndquadrant.com, noah@leadboat.com
Subject: Re: cleaning perl code
In-Reply-To: <19843.1586623730@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox