From @andk

bisect

commit 1d7e644
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed Aug 17 09​:52​:48 2016 +0100

  av_fetch()​: use AvFILLp rather than AvFILL

diagnostics

http​://www.cpantesters.org/cpan/report/4145b580-64b0-11e6-a8a1-60d858b9f28c

perl -V

Summary of my perl5 (revision 5 version 25 subversion 4) configuration​:
  Commit id​: cc89058
  Platform​:
  osname=linux
  osvers=4.3.0-1-amd64
  archname=x86_64-linux-ld
  uname='linux k83 4.3.0-1-amd64 #1 smp debian 4.3.3-7 (2016-01-19) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942 -Dmyhostname=k83 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m crypt sec util c cposix posix ucb BSD gdbm_compat -Uuseithreads -Duselongdouble -DDEBUGGING=-g'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=undef
  usemultiplicity=undef
  use64bitint=define
  use64bitall=define
  uselongdouble=define
  usemymalloc=n
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
  optimize='-O2 -g'
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='5.3.1 20160121'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='long double'
  nvsize=16
  Off_t='off_t'
  lseeksize=8
  alignbytes=16
  prototype=define
  Linker and Libraries​:
  ld='cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/5/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -lgdbm -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.21.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.21'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​:
  HAS_TIMES
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_LONG_DOUBLE
  USE_PERLIO
  USE_PERL_ATOF
  Built under linux
  Compiled at Aug 17 2016 15​:33​:39
  %ENV​:
  PERL5LIB=""
  PERL5OPT=""
  PERL5_CPANPLUS_IS_RUNNING="30984"
  PERL5_CPAN_IS_RUNNING="30984"
  PERL_CANARY_STABILITY_NOPROMPT="1"
  PERL_MM_USE_DEFAULT="1"
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942/lib/site_perl/5.25.4/x86_64-linux-ld
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942/lib/site_perl/5.25.4
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942/lib/5.25.4/x86_64-linux-ld
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942/lib/5.25.4
  .

--
andreas

From perl@profvince.com

Le 18/08/2016 à 19​:22, (Andreas J. Koenig) (via RT) a écrit :

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

bisect
------
commit 1d7e644
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed Aug 17 09​:52​:48 2016 +0100

av\_fetch\(\)​: use AvFILLp rather than AvFILL

diagnostics
-----------
http​://www.cpantesters.org/cpan/report/4145b580-64b0-11e6-a8a1-60d858b9f28c

Thanks for the bisect. I had seen the report, but I did not know exactly
which change caused the failure. Now I'm pretty sure the failing test is
the one that checks whether len magic triggers for "$a[-1] = 2".

I'm too busy right now to check this and cut out a release, but I'll try
to find some time for that Monday.

Maybe it should be documented somewhere that len magic is practically
useless now from an user perspective.

Vincent

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

From @iabyn

On Thu, Aug 18, 2016 at 07​:42​:25PM -0300, Vincent Pit (VPIT) wrote​:

Le 18/08/2016 à 19​:22, (Andreas J. Koenig) (via RT) a écrit :

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

bisect
------
commit 1d7e644
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed Aug 17 09​:52​:48 2016 +0100

av\_fetch\(\)​: use AvFILLp rather than AvFILL

diagnostics
-----------
http​://www.cpantesters.org/cpan/report/4145b580-64b0-11e6-a8a1-60d858b9f28c

Thanks for the bisect. I had seen the report, but I did not know exactly
which change caused the failure. Now I'm pretty sure the failing test is the
one that checks whether len magic triggers for "$a[-1] = 2".

I'm too busy right now to check this and cut out a release, but I'll try to
find some time for that Monday.

Maybe it should be documented somewhere that len magic is practically
useless now from an user perspective.

That particular commit changed the code path in av_fetch() which handles
negative indices to match the path for +ve indices. So if this commit
is wrong, the +ve path has been wrong too. AFAIKT, the +ve branch of
av_fetch() has been using AvFILLp() rather than AvFILL() back to at least
perl5.005_03.

av_fetch() uses AvFILLp() unless both the RMG flag is set and it can find
tied magic. Perhaps the bug here is that av_fetch() should handle the
combination of RMG but not tied specially too?
But this combination doesn't seem to make much sense​: it says we
have to call mg_size() to find the size of the array (in order to check for
key being out of bounds), but we then ignore any magic to actually retrieve
the element from AvARRAY)av)[key].

--
O Unicef Clearasil!
Gibberish and Drivel!
  -- "Bored of the Rings"

From @iabyn

On Fri, Aug 19, 2016 at 12​:12​:14PM +0100, Dave Mitchell wrote​:

On Thu, Aug 18, 2016 at 07​:42​:25PM -0300, Vincent Pit (VPIT) wrote​:

Thanks for the bisect. I had seen the report, but I did not know exactly
which change caused the failure. Now I'm pretty sure the failing test is the
one that checks whether len magic triggers for "$a[-1] = 2".

I'm too busy right now to check this and cut out a release, but I'll try to
find some time for that Monday.

Maybe it should be documented somewhere that len magic is practically
useless now from an user perspective.

That particular commit changed the code path in av_fetch() which handles
negative indices to match the path for +ve indices. So if this commit
is wrong, the +ve path has been wrong too. AFAIKT, the +ve branch of
av_fetch() has been using AvFILLp() rather than AvFILL() back to at least
perl5.005_03.

av_fetch() uses AvFILLp() unless both the RMG flag is set and it can find
tied magic. Perhaps the bug here is that av_fetch() should handle the
combination of RMG but not tied specially too?
But this combination doesn't seem to make much sense​: it says we
have to call mg_size() to find the size of the array (in order to check for
key being out of bounds), but we then ignore any magic to actually retrieve
the element from AvARRAY)av)[key].

Indeed, changing $val[-1] to $val[2] in t/22-len.t (which should be
equivalent for a 3-element array) makes the test fail with all perls back
to 5.8.0.

So len magic on arrays has never worked for Variable​::Magic, except for
negative indices.

I'd suggest that Variable​::Magic is updated to reflect this reality,
rather than making blead support it - unless a strong case can be made
otherwise.

--
It's not that I'm afraid to die, I just don't want to be there when it
happens.
  -- Woody Allen

From @iabyn

On Mon, Aug 22, 2016 at 09​:41​:43AM +0100, Dave Mitchell wrote​:

On Fri, Aug 19, 2016 at 12​:12​:14PM +0100, Dave Mitchell wrote​:

On Thu, Aug 18, 2016 at 07​:42​:25PM -0300, Vincent Pit (VPIT) wrote​:

Thanks for the bisect. I had seen the report, but I did not know exactly
which change caused the failure. Now I'm pretty sure the failing test is the
one that checks whether len magic triggers for "$a[-1] = 2".

I'm too busy right now to check this and cut out a release, but I'll try to
find some time for that Monday.

Maybe it should be documented somewhere that len magic is practically
useless now from an user perspective.

That particular commit changed the code path in av_fetch() which handles
negative indices to match the path for +ve indices. So if this commit
is wrong, the +ve path has been wrong too. AFAIKT, the +ve branch of
av_fetch() has been using AvFILLp() rather than AvFILL() back to at least
perl5.005_03.

av_fetch() uses AvFILLp() unless both the RMG flag is set and it can find
tied magic. Perhaps the bug here is that av_fetch() should handle the
combination of RMG but not tied specially too?
But this combination doesn't seem to make much sense​: it says we
have to call mg_size() to find the size of the array (in order to check for
key being out of bounds), but we then ignore any magic to actually retrieve
the element from AvARRAY)av)[key].

Indeed, changing $val[-1] to $val[2] in t/22-len.t (which should be
equivalent for a 3-element array) makes the test fail with all perls back
to 5.8.0.

So len magic on arrays has never worked for Variable​::Magic, except for
negative indices.

I'd suggest that Variable​::Magic is updated to reflect this reality,
rather than making blead support it - unless a strong case can be made
otherwise.

Interestingly, if I change av_fetch() so that it does AvFILL() (rather
than AvFILLp()) for positive indexes, then a bunch of tests in 31-array.t
fail; e.g.​:

  # Failed test 'array​: assign element to triggers magic correctly'
  # at t/31-array.t line 27.
  # Structures begin differing at​:
  # $got->{len} = '1'
  # $expected->{len} = Does not exist

I strongly suspect that that the correct fix is to update V​::M's tests and
docs to match the new reality in perl. But in the meantime, I've
temporarily reverted to the old behaviour with the following commit.
Note that that 5.25.4 has just been released with the behaviour that
causes t/22-len.t to fail.

commit 8d168aa
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Wed Aug 24 13​:57​:56 2016 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Wed Aug 24 13​:57​:56 2016 +0100

  tmp fix for Bleadperl breaks Variable-Magic
 
  RT #128989
 
  Prior to my commit v5.25.3-266-g1d7e644, in the absence of the SVs_RMG
  flag, av_fetch() used AvFILL() for -ve keys and AvFILLp() for positive
  keys. That commit changed it so they both use AvFILLp. This has broken
  Variable​::Magic 0.59.
 
  As an interim measure, restore the old behaviour.

Affected files ...
  M av.c

Differences ...

diff --git a/av.c b/av.c
index 21828a9..e3c6d5a 100644
--- a/av.c
+++ b/av.c
@@ -272,7 +272,8 @@ Perl_av_fetch(pTHX_ AV *av, SSize_t key, I32 lval)
     }
 
     neg  = (key < 0);
-    size = AvFILLp(av) + 1;
+    /* XXX tmp restore old behaviour to make Variable::Magic pass */
+    size = (neg ? AvFILL(av): AvFILLp(av)) + 1;
     key += neg * size; /* handle negative index without using branch */
 
     /* the cast from SSize_t to Size_t allows both (key < 0) and (key >= size)


-- 

"But Sidley Park is already a picture, and a most amiable picture too.
The slopes are green and gentle. The trees are companionably grouped at
intervals that show them to advantage. The rill is a serpentine ribbon
unwound from the lake peaceably contained by meadows on which the right
amount of sheep are tastefully arranged." -- Lady Croom, "Arcadia"

From perl@profvince.com

commit 8d168aa
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Wed Aug 24 13​:57​:56 2016 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Wed Aug 24 13​:57​:56 2016 +0100

tmp fix for Bleadperl breaks Variable\-Magic

RT \#128989

Prior to my commit v5\.25\.3\-266\-g1d7e644\, in the absence of the SVs\_RMG
flag\, av\_fetch\(\) used AvFILL\(\) for \-ve keys and AvFILLp\(\) for positive
keys\. That commit changed it so they both use AvFILLp\. This has broken
Variable​::Magic 0\.59\.

As an interim measure\, restore the old behaviour\.

Affected files ...
M av.c

Differences ...

diff --git a/av.c b/av.c
index 21828a9..e3c6d5a 100644
--- a/av.c
+++ b/av.c
@​@​ -272,7 +272,8 @​@​ Perl_av_fetch(pTHX_ AV *av, SSize_t key, I32 lval)
}

 neg  = \(key \< 0\);

- size = AvFILLp(av) + 1;
+ /* XXX tmp restore old behaviour to make Variable​::Magic pass */
+ size = (neg ? AvFILL(av)​: AvFILLp(av)) + 1;
key += neg * size; /* handle negative index without using branch */

 /\* the cast from SSize\_t to Size\_t allows both \(key \< 0\) and \(key >= size\)

I've released version 0.60 that skips the test, so you can revert your
revert if you want.

Vincent

From @iabyn

On Tue, Sep 06, 2016 at 07​:36​:20PM -0300, Vincent Pit (VPIT) wrote​:

I've released version 0.60 that skips the test, so you can revert your
revert if you want.

Ok thanks. Now unreverted.

--
Counsellor Troi states something other than the blindingly obvious.
  -- Things That Never Happen in "Star Trek" #16

@iabyn - Status changed from 'open' to 'rejected'