From @cpansprout

Formerly the & prototype allowed ‘undef’ as an argument, contrary to the documentation. that changed in v5.21.6-51-ge41e986.

The question is​: Do we need to permit ‘undef’ and adjust the documentation? Or does BDB need to be fixed?

--

Father Chrysostomos

From @ikegami

On Sun, Dec 21, 2014 at 2​:33 PM, Father Chrysostomos <
perlbug-followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #123475]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=123475 >

Formerly the & prototype allowed ‘undef’ as an argument, contrary to the
documentation. that changed in v5.21.6-51-ge41e986.

The question is​: Do we need to permit ‘undef’ and adjust the
documentation? Or does BDB need to be fixed?

It feels wrong to allow undef, but I don't have a good argument for or
against.

The only thing I have is that permitting undef would be inconsistent

perl -we"sub f(\@​) { } f(@​a); f(undef);"
Type of arg 1 to main​::f must be array (not undef operator) at -e line 1,
near "undef)"
Execution of -e aborted due to compilation errors.

The RT System itself - Status changed from 'new' to 'open'

From @tonycoz

On Sun Dec 21 11​:33​:02 2014, sprout wrote​:

Formerly the & prototype allowed ‘undef’ as an argument, contrary to
the documentation. that changed in v5.21.6-51-ge41e986.

The question is​: Do we need to permit ‘undef’ and adjust the
documentation? Or does BDB need to be fixed?

I think it's a bug fix, and if BDB depends on that bug it needs to be fixed.

Tony

From @andk

also affected

JNQUINTIN/Math-Permute-Array-0.043.tar.gz

(brought to my attention by Slaven Rezić)

--
andreas

From @cpansprout

On Tue Mar 24 00​:20​:05 2015, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

also affected
-------------
JNQUINTIN/Math-Permute-Array-0.043.tar.gz

(brought to my attention by Slaven Rezić)

Patch at <https://rt.cpan.org/Ticket/Display.html?id=103041>.

--

Father Chrysostomos

From @rjbs

I can't easily compile BDB here. Is the fix as simple as changing the PROTOTYPE entry for set_sync_prepare in BDB.xs to ";&" ?

Could someone verify this?

--
rjbs

From @iabyn

On Mon, Apr 27, 2015 at 03​:15​:34PM -0700, Ricardo SIGNES via RT wrote​:

I can't easily compile BDB here. Is the fix as simple as changing the
PROTOTYPE entry for set_sync_prepare in BDB.xs to ";&" ?

Not really. It would also have to change the XS implementation to handle
a variable number of args, and still wouldn't handle the case of an
explicit 'undef' arg.

Looking at the usage in BDB, I think an undef value should still be
allowed as an argument to a '&' parameter.

BDB.pm does this as part of its initialisation​:

  set_sync_prepare (undef);

where set_sync_prepare() is used to register a callback. So in this case
it is explicitly initialising its state to "no callback registered". I
think this is a reasonable use of the & prototype.

Looking at the original '&' ticket,

  #123062​: & prototype is too permissive

I think it's correct that it should dis-allow array refs etc, but I think
undef should be re-allowed for 5.22.

--
Dave's first rule of Opera​:
If something needs saying, say it​: don't warble it.

From @iabyn

On Tue, May 05, 2015 at 04​:33​:21PM +0100, Dave Mitchell wrote​:

On Mon, Apr 27, 2015 at 03​:15​:34PM -0700, Ricardo SIGNES via RT wrote​:

I can't easily compile BDB here. Is the fix as simple as changing the
PROTOTYPE entry for set_sync_prepare in BDB.xs to ";&" ?

Not really. It would also have to change the XS implementation to handle
a variable number of args, and still wouldn't handle the case of an
explicit 'undef' arg.

Looking at the usage in BDB, I think an undef value should still be
allowed as an argument to a '&' parameter.

BDB.pm does this as part of its initialisation​:

set\_sync\_prepare \(undef\);

where set_sync_prepare() is used to register a callback. So in this case
it is explicitly initialising its state to "no callback registered". I
think this is a reasonable use of the & prototype.

Looking at the original '&' ticket,

\#123062​: & prototype is too permissive

I think it's correct that it should dis-allow array refs etc, but I think
undef should be re-allowed for 5.22.

If no one objects, I'll merge the following commit once it's smoked​:

commit 343d03e
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Wed May 6 11​:56​:47 2015 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Wed May 6 12​:01​:06 2015 +0100

  allow undef as an arg to '&' prototype
 
  RT #123475
 
  Commit e41e986 (to fix [perl #123062]) restricted the types of
  args allowed for a function with a '&' prototype - previously it allowed
  array refs and the like. It also removed undef, so this was now a
  compile-time error​:
 
  sub foo (&) {...}
  foo(undef)
 
  However, some CPAN code used the idiom register_callback(undef) to
  explicitly disable a registered callback.
 
  So re-allow an explicit undef.

--
print+qq&$}$"$/$s$,$a$d$g$s$@​$.$q$,$​:$.$q$^$,$@​$a$$;$.$q$m&if+map{m,^\d{0\,},,${$​::{$'}}=chr($"+=$&amp;||1)}q&10m22,42}6​:17a22.3@​3;^2dg3q/s"&=~m*\d\*.*g

From @rjbs

* Dave Mitchell <davem@​iabyn.com> [2015-05-05T11​:33​:21]

I think it's correct that it should dis-allow array refs etc, but I think
undef should be re-allowed for 5.22.

I wrote many, many long responses to this, each one different than the
previous, as I reached logical dead ends. The behavior and intent of
prototypes is, in my opinion, so incoherent as to make any reasoning from first
principles unlikely to bear fruit.

So, instead, I think we have a really simple choice. Here's what the docs say
about (&)​:

  An "&" requires an anonymous subroutine, which, if passed as the first
  argument, does not require the "sub" keyword or a subsequent comma.

The old behavior allows​:

  sub takes_sub (&) { ... }
  takes_sub(undef);

...which contradicts the documentation. We should change the behavior or we
should change the documentation.

Changing the documentation and leaving the code alone (relative to v5.20) will
let things like BDB continue to work, where a literal undef is passed to reset
a stored subroutine.

Changing the code and leaving the documentation alone (again, relative to
v5.20) will break anything that had relied on this behavior, but will expose
bugs where a literal undef had been passed to a subroutine with a (&)
prototype in effect. It will now issue a compile-time error. Presumably,
these situations are bugs iff later something attempts $arg->(), which would
currently be a runtime error. That doesn't mean that this wouldn't expose
existing bugs, but I think it does cut down on what we'd expect to see fixed.
Also, we should consider the prevention of future bugs, which won't ever need
to be detected at runtime.

Nonetheless, it is with heavy heart that I think I agree with the suggested
course of action​: go back to the old, worse behavior. If we were starting
from scratch, I would hold my ground, but I don't think the breakage is
justified here.

Boy, do I ever dislike prototypes!

If anybody wants to change my mind, do so quickly! ;) Or wait to try again in
5.23, I suppose...

--
rjbs

From @iabyn

On Wed, May 06, 2015 at 09​:26​:47AM -0400, Ricardo Signes wrote​:

I wrote many, many long responses to this, each one different than the
previous, as I reached logical dead ends. The behavior and intent of
prototypes is, in my opinion, so incoherent as to make any reasoning from first
principles unlikely to bear fruit.

So, instead, I think we have a really simple choice. Here's what the docs say
about (&)​:

An "&" requires an anonymous subroutine, which, if passed as the first
argument, does not require the "sub" keyword or a subsequent comma.

The old behavior allows​:

sub takes_sub (&) { ... }
takes_sub(undef);

...which contradicts the documentation. We should change the behavior or we
should change the documentation.

Changing the documentation and leaving the code alone (relative to v5.20) will
let things like BDB continue to work, where a literal undef is passed to reset
a stored subroutine.

Changing the code and leaving the documentation alone (again, relative to
v5.20) will break anything that had relied on this behavior, but will expose
bugs where a literal undef had been passed to a subroutine with a (&)
prototype in effect. It will now issue a compile-time error. Presumably,
these situations are bugs iff later something attempts $arg->(), which would
currently be a runtime error. That doesn't mean that this wouldn't expose
existing bugs, but I think it does cut down on what we'd expect to see fixed.
Also, we should consider the prevention of future bugs, which won't ever need
to be detected at runtime.

Nonetheless, it is with heavy heart that I think I agree with the suggested
course of action​: go back to the old, worse behavior. If we were starting
from scratch, I would hold my ground, but I don't think the breakage is
justified here.

Now merged with v5.21.11-77-g8283f70

--
The warp engines start playing up a bit, but seem to sort themselves out
after a while without any intervention from boy genius Wesley Crusher.
  -- Things That Never Happen in "Star Trek" #17

@iabyn - Status changed from 'open' to 'pending release'

From @khwilliamson

Thanks for submitting this ticket

The issue should be resolved with the release today of Perl v5.22, available at http​://www.perl.org/get.html
If you find that the problem persists, feel free to reopen this ticket

--
Karl Williamson for the Perl 5 porters team

@khwilliamson - Status changed from 'pending release' to 'resolved'