Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bleadperl between fa1e92c and e08be60 breaks MAGGIEXYZ/PDL-Stats-0.5.5.tar.gz #11835

Closed
p5pRT opened this issue Jan 1, 2012 · 28 comments
Closed

Bleadperl between fa1e92c and e08be60 breaks MAGGIEXYZ/PDL-Stats-0.5.5.tar.gz #11835

p5pRT opened this issue Jan 1, 2012 · 28 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Jan 1, 2012

Migrated from rt.perl.org#107366 (status was 'resolved')

Searchable as RT107366$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 1, 2012

From @andk

git bisect


* v5.14.0-285-gfa1e92c broke PDL (tested with CHM/PDL-2.4.9_992.tar.gz)

* v5.14.0-408-ge08be60 made PDL pass tests again

* when PDL came back passing, PDL​::Stats (tested with
  MAGGIEXYZ/PDL-Stats-0.5.5.tar.gz) did not pass anymore

diagnostics


http​://www.cpantesters.org/cpan/report/0e156db2-9266-11e0-8ca7-e3acbd5b0f7c

perl -V


Summary of my perl5 (revision 5 version 15 subversion 0) configuration​:
  Commit id​: e08be60
  Platform​:
  osname=linux, osvers=2.6.32-5-amd64, archname=x86_64-linux-ld
  uname='linux k83 2.6.32-5-amd64 #1 smp thu nov 3 03​:41​:26 utc 2011 x86_64 gnulinux '
  config_args='-Dprefix=/home/src/perl/repoperls/installed-perls/perl/v5.14.0-408-ge08be60/127e -Dmyhostname=k83 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Uuseithreads -Duselongdouble -DDEBUGGING=-g'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=define
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.4.5', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  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 -L/usr/local/lib'
  libpth=/usr/local/lib /lib/../lib /usr/lib/../lib /lib /usr/lib /lib64 /usr/lib64
  libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=/lib/libc-2.11.2.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.11.2'
  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'

Characteristics of this binary (from libperl)​:
  Compile-time options​: PERL_DONT_CREATE_GVSV PERL_MALLOC_WRAP
  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_LONG_DOUBLE USE_PERLIO
  USE_PERL_ATOF
  Built under linux
  Compiled at Jan 1 2012 11​:18​:53
  @​INC​:
  /home/src/perl/repoperls/installed-perls/perl/v5.14.0-408-ge08be60/127e/lib/site_perl/5.15.0/x86_64-linux-ld
  /home/src/perl/repoperls/installed-perls/perl/v5.14.0-408-ge08be60/127e/lib/site_perl/5.15.0
  /home/src/perl/repoperls/installed-perls/perl/v5.14.0-408-ge08be60/127e/lib/5.15.0/x86_64-linux-ld
  /home/src/perl/repoperls/installed-perls/perl/v5.14.0-408-ge08be60/127e/lib/5.15.0
  .

--
andreas

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 1, 2012

From @cpansprout

On Sun Jan 01 12​:06​:27 2012, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

git bisect
----------
* v5.14.0-285-gfa1e92c broke PDL (tested with CHM/PDL-
2.4.9_992.tar.gz)

* v5.14.0-408-ge08be60 made PDL pass tests again

* when PDL came back passing, PDL​::Stats (tested with
MAGGIEXYZ/PDL-Stats-0.5.5.tar.gz) did not pass anymore

At least on a Mac, with v5.15.6-235-g7d9189d, I can’t even get as far as
compiling PDL. I get some sort of typemap failure, that I don’t yet
understand​:

...
Extracting Core.xs (WITH bad value support)
/usr/local/bin/perl5.15.6 -I../../blib/arch -I../../blib/lib
-I/usr/local/lib/perl5/5.15.6/darwin-thread-multi-2level
-I/usr/local/lib/perl5/5.15.6 Core.xs.PL Core.xs
Extracting Core.xs (WITH bad value support)
/usr/local/bin/perl5.15.6 /usr/local/lib/perl5/5.15.6/ExtUtils/xsubpp
-typemap /usr/local/lib/perl5/5.15.6/ExtUtils/typemap -typemap typemap
Core.xs > Core.xsc && mv Core.xsc Core.c
Could not find a typemap for C type 'PDL_Long *' in Core.xs, line 1144
make[2]​: *** [Core.o] Error 1
make[1]​: *** [subdirs] Error 2
make​: *** [subdirs] Error 2
  CHM/PDL-2.4.9.tar.gz
  /usr/bin/make -- NOT OK

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 1, 2012

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 3, 2012

From dcmertens.perl@gmail.com

Thanks for bringing this up. I've forwarded it to the PDL porters. I can't
look at this right now, but hopefully I or one of the other porters will
have a chance to look at this in the next couple of days.

David

On Sun, Jan 1, 2012 at 4​:10 PM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Sun Jan 01 12​:06​:27 2012, andreas.koenig.7os6VVqR@​franz.ak.mind.dewrote​:

git bisect
----------
* v5.14.0-285-gfa1e92c broke PDL (tested with CHM/PDL-
2.4.9_992.tar.gz)

* v5.14.0-408-ge08be60 made PDL pass tests again

* when PDL came back passing, PDL​::Stats (tested with
MAGGIEXYZ/PDL-Stats-0.5.5.tar.gz) did not pass anymore

At least on a Mac, with v5.15.6-235-g7d9189d, I can’t even get as far as
compiling PDL. I get some sort of typemap failure, that I don’t yet
understand​:

...
Extracting Core.xs (WITH bad value support)
/usr/local/bin/perl5.15.6 -I../../blib/arch -I../../blib/lib
-I/usr/local/lib/perl5/5.15.6/darwin-thread-multi-2level
-I/usr/local/lib/perl5/5.15.6 Core.xs.PL Core.xs
Extracting Core.xs (WITH bad value support)
/usr/local/bin/perl5.15.6 /usr/local/lib/perl5/5.15.6/ExtUtils/xsubpp
-typemap /usr/local/lib/perl5/5.15.6/ExtUtils/typemap -typemap typemap
Core.xs > Core.xsc && mv Core.xsc Core.c
Could not find a typemap for C type 'PDL_Long *' in Core.xs, line 1144
make[2]​: *** [Core.o] Error 1
make[1]​: *** [subdirs] Error 2
make​: *** [subdirs] Error 2
CHM/PDL-2.4.9.tar.gz
/usr/bin/make -- NOT OK

--

Father Chrysostomos

---
via perlbug​: queue​: perl5 status​: new
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=107366

--
Sent via my carrier pigeon.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 3, 2012

From @cpansprout

On Sun Jan 01 14​:10​:26 2012, sprout wrote​:

On Sun Jan 01 12​:06​:27 2012, andreas.koenig.7os6VVqR@​franz.ak.mind.de
wrote​:

git bisect
----------
* v5.14.0-285-gfa1e92c broke PDL (tested with CHM/PDL-
2.4.9_992.tar.gz)

* v5.14.0-408-ge08be60 made PDL pass tests again

* when PDL came back passing, PDL​::Stats (tested with
MAGGIEXYZ/PDL-Stats-0.5.5.tar.gz) did not pass anymore

At least on a Mac,

$ uname -a
Darwin Pint.local 10.5.0 Darwin Kernel Version 10.5.0​: Fri Nov 5
23​:20​:39 PDT 2010; root​:xnu-1504.9.17~1/RELEASE_I386 i386

And Linux, too​:

$ uname -a
Linux dromedary 2.6.18-274.3.1.el5 #1 SMP Tue Sep 6 20​:13​:52 EDT 2011
x86_64 x86_64 x86_64 GNU/Linux

On the Mac I was compiling with -Dusedevel -Dmad -Duseithreads,
on Linux with -Dusedevel -Doptimize=-g -DDEBUGGING.

with v5.15.6-235-g7d9189d, I can’t even get as far as
compiling PDL. I get some sort of typemap failure, that I don’t yet
understand​:

...
Extracting Core.xs (WITH bad value support)
/usr/local/bin/perl5.15.6 -I../../blib/arch -I../../blib/lib
-I/usr/local/lib/perl5/5.15.6/darwin-thread-multi-2level
-I/usr/local/lib/perl5/5.15.6 Core.xs.PL Core.xs
Extracting Core.xs (WITH bad value support)
/usr/local/bin/perl5.15.6 /usr/local/lib/perl5/5.15.6/ExtUtils/xsubpp
-typemap /usr/local/lib/perl5/5.15.6/ExtUtils/typemap -typemap typemap
Core.xs > Core.xsc && mv Core.xsc Core.c
Could not find a typemap for C type 'PDL_Long *' in Core.xs, line 1144
make[2]​: *** [Core.o] Error 1
make[1]​: *** [subdirs] Error 2
make​: *** [subdirs] Error 2
CHM/PDL-2.4.9.tar.gz
/usr/bin/make -- NOT OK

I’ve done more than a few blead builds to find out when things changed.
It’s actually quite complicated.

In some versions, the .= 0 in this script has no effect​:

$ ~/blead/bin/perl5.15.0 -Mblib -e 'use PDL​::LiteF; my $a = sequence
5,2; warn $a; $a->nslice("X" ,1) .= 0; warn $a'

[
[0 1 2 3 4]
[5 6 7 8 9]
]

[
[0 1 2 3 4]
[5 6 7 8 9]
]

I’m calling that ‘bad .=’.

In others, it dies with ‘Can't modify non-lvalue subroutine call’, but
when called like this it works​:

$ ~/blead/bin/perl5.15.0 -Mblib -e 'use PDL​::LiteF; my $a = sequence
5,2; warn $a; ${\$a->nslice("X" ,1)} .= 0; warn $a'

[
[0 1 2 3 4]
[5 6 7 8 9]
]

[
[0 1 2 3 4]
[0 0 0 0 0]
]

I’m calling that one ‘non-lvalue’.

The typemap error when building PDL I’m calling ‘typemap problem’.

Here is the timeline, starting with the commit that made PDL tass its
tests again​:

v5.14.0-408-ge08be60 (v5.15.0~284) bad .=

v5.15.0 (v5.15.1~391) bad .=

v5.15.1~336 bad .=

v5.15.1~335 non-lvalue

v5.15.1~93 non-lvalue

(ExtUtils​::ParseXS branch follows)

9689328~38 non-lvalue

9689328~25 ExtUtils​::ParseXS won’t build

9689328~18 non-lvalue

9689328~13 non-lvalue

9689328~12 typemap problem

v5.15.1~92 (ExtUtils​::ParseXS branch merged)

v5.15.1~91 typemap problem

v5.15.1 typemap problem

(This sort of thing is far too complicated to do with git bisect.
That’s why I still don’t like the practice of merging branches with
-no-ff. It makes it harder to do this manually.)

Most of the changes are my fault. I haven’t looked into PDL in detail
yet to see what’s happening. The typemap problem starts with one of
Steffen Müller’s commits​:

commit 5a784a6
Author​: Steffen Mueller <smueller@​cpan.org>
Date​: Sat Apr 16 16​:58​:57 2011 +0200

  Error handling/message improvements
 
  - Move line number calculation to separate method
  - Make death/Warn/blurt proper methods
  They pretended to be methods all along, but never were.
  - Pass XS file name and line no. to typemap parser
  ... for better error messages from the typemap parser in
  case of embedded typemaps

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 3, 2012

From @sisyphus

----- Original Message -----
From​: "Father Chrysostomos via RT"

Core.xs > Core.xsc && mv Core.xsc Core.c
Could not find a typemap for C type 'PDL_Long *' in Core.xs, line 1144
make[2]​: *** [Core.o] Error 1
make[1]​: *** [subdirs] Error 2
make​: *** [subdirs] Error 2
CHM/PDL-2.4.9.tar.gz
/usr/bin/make -- NOT OK

This is the result of some poor coding practices in Core.xs.pl (from which
Core.xs is generated).
It doesn't seem to matter prior to the 3.xx versions of ExtUtils​::ParseXS.

This poorly written code was fixed a while ago, and the problem no longer
arises with recent devel releases of PDL.

Cheers,
Rob

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 4, 2012

From @tsee

On 01/03/2012 10​:11 PM, Sisyphus wrote​:

Core.xs > Core.xsc && mv Core.xsc Core.c
Could not find a typemap for C type 'PDL_Long *' in Core.xs, line 1144
make[2]​: *** [Core.o] Error 1
make[1]​: *** [subdirs] Error 2
make​: *** [subdirs] Error 2
CHM/PDL-2.4.9.tar.gz
/usr/bin/make -- NOT OK

This is the result of some poor coding practices in Core.xs.pl (from
which Core.xs is generated).
It doesn't seem to matter prior to the 3.xx versions of ExtUtils​::ParseXS.

This poorly written code was fixed a while ago, and the problem no
longer arises with recent devel releases of PDL.

Thanks for that. I dimly remember investigating this back when I did the
PXS 3.0 testing.

What's the likelihood of seeing a stable, fixed PDL release soon? Or am
I misreading your comments by assuming there hasn't been one since?

Cheers,
Steffen

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 4, 2012

From moocow.bovine@gmail.com

On Tue Jan 03 22​:56​:29 2012, smueller@​cpan.org wrote​:

What's the likelihood of seeing a stable, fixed PDL release soon?

The devel release CHM/PDL-2.4.9_992.tar.gz just built ok here with perl
5.15.6. Although I myself can't speak for the PDL porters, they seem to
be in the final preparations (code freeze) for a 2.4.10 release.

I would also like to note that MOOCOW/PDL-CCS-1.17.tar.gz also fails
tests with almost all 5.15.x perl versions. The culprit seems to be the
"non-lvalue" behavior described above ("Can't modify non-lvalue
subroutine call" errors) in constructions such as​:

$a=sequence(4);
$a->where($a%2) .= 42;

The most recent perl under which this seems to work according to
cpantesters for PDL​::CCS is commit aa80f64 (5.15.0; Tue, 21 Jun 2011
15​:43​:11 +0000 (08​:43 -0700)). PDL​::CCS fails under a near-identical
configuration for perl commit 5655754 (5.15.0; Sun, 10 Jul 2011 00​:13​:10
+0000 (18​:13 -0600)). The respective cpantesters reports are here​:

PASS​:
http​://www.cpantesters.org/cpan/report/332403aa-365f-11e1-a168-b1789aeef8c6
FAIL​:
http​://www.cpantesters.org/cpan/report/74d7e262-35fb-11e1-93c9-af259aeef8c6

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 4, 2012

From [Unknown Contact. See original ticket]

On Tue Jan 03 22​:56​:29 2012, smueller@​cpan.org wrote​:

What's the likelihood of seeing a stable, fixed PDL release soon?

The devel release CHM/PDL-2.4.9_992.tar.gz just built ok here with perl
5.15.6. Although I myself can't speak for the PDL porters, they seem to
be in the final preparations (code freeze) for a 2.4.10 release.

I would also like to note that MOOCOW/PDL-CCS-1.17.tar.gz also fails
tests with almost all 5.15.x perl versions. The culprit seems to be the
"non-lvalue" behavior described above ("Can't modify non-lvalue
subroutine call" errors) in constructions such as​:

$a=sequence(4);
$a->where($a%2) .= 42;

The most recent perl under which this seems to work according to
cpantesters for PDL​::CCS is commit aa80f64 (5.15.0; Tue, 21 Jun 2011
15​:43​:11 +0000 (08​:43 -0700)). PDL​::CCS fails under a near-identical
configuration for perl commit 5655754 (5.15.0; Sun, 10 Jul 2011 00​:13​:10
+0000 (18​:13 -0600)). The respective cpantesters reports are here​:

PASS​:
http​://www.cpantesters.org/cpan/report/332403aa-365f-11e1-a168-b1789aeef8c6
FAIL​:
http​://www.cpantesters.org/cpan/report/74d7e262-35fb-11e1-93c9-af259aeef8c6

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 4, 2012

From @sisyphus

----- Original Message -----
From​: "Steffen Mueller"

What's the likelihood of seeing a stable, fixed PDL release soon?

PDL-2.4.10 is imminent.
I think it's planned for later this month. (Should certainly be out before
the end of Feb.)

Cheers,
Rob

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 5, 2012

From @cpansprout

On Wed Jan 04 09​:45​:26 2012, sisyphus1@​optusnet.com.au wrote​:

----- Original Message -----
From​: "Steffen Mueller"

What's the likelihood of seeing a stable, fixed PDL release soon?

PDL-2.4.10 is imminent.
I think it's planned for later this month. (Should certainly be out
before
the end of Feb.)

Unless there will be another PDL release after that before May, I think
it should wait until this issue is resolved.

I’ve noticed that the latest dev PDL (2.4.9_992) has ‘no warnings
'misc'’ when applying the lvalue attribute. Hiding the warning does not
eliminate the problem that the warning is warning about. Details follow.

I made a mistake in my timeline. The non-lvalue problem was introduced
by v5.15.1~334, aka bb3abb0.

I(t) changed attributes.pm to warn when applying the lvalue attribute to
a defined Perl (non-XS) sub instead of applying the attribute.

sub foo​:lvalue started doing that in 5.12, and I was trying to make
things more or less consistent.

Perl 5.12 started warning and not applying the attribute, because

  sub foo :lvalue
  sub foo { @​a }

and

  sub foo { @​a }
  sub foo :lvalue

do not do the same thing. The lvalue attribute affects the way the
subroutine is compiled. Applying the lvalue attribute afterwords won’t
work, in that it won’t allow the thing returned to be assigned to,
because it gets copied (with some exceptions for explicit return in
5.15; details subject to change).

PDL happens to be unaffected by the fact that the compiled op tree is
not lvaluified, because it returns objects with .= overloading, and .=
is its raison d’être for using lvalue subs. (Disclaimer​: I have only
looked at nslice.)

So it seems to me that PDL could easily create lvalue stubs via
eval("sub PDL​::nslice :lvalue") or attributes.pm before defining the
subs, instead of after. (That would mean changing the load order, so
that anything that defines those subs loads PDL​::Lvalue itself first.)

On the other hand, attributes.pm is rarely used directly, so I would not
be opposed to reverting bb3abb0, as long as we add caveats to the
documentation for attributes.pm, explaining that you may not be getting
what you are thinking with ‘use attributes $pack, \&sub, 'lvalue'’.

Giving people enough rope, with a caveat (‘caution​: you may hang’ :-),
is often a good thing.

BTW, reverting bb3abb0 makes everything work, including PDL​::Stats and
PDL​::CCS. Whichever bug was causing the bad .= that I mentioned earlier
(<https://rt-archive.perl.org/perl5/Ticket/Display.html?id=107366#txn-1050006>)
has since been fixed.

So now I have to ask both p5p and pdl-porters​: Which is the best way
forward?

(And could someone please forward this to pdl-porters? Thank you.)

P.S.​: I think PDL’s test suite needs to cover lvalue subroutines. It is
passing its own tests in 5.15.6, when it shouldn’t.

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 6, 2012

From @sisyphus

----- Original Message -----
From​: "Father Chrysostomos via RT"

Unless there will be another PDL release after that before May, I think
it should wait until this issue is resolved.

We are aiming for a 2.4.11 before then.

I’ve noticed that the latest dev PDL (2.4.9_992) has ‘no warnings
'misc'’ when applying the lvalue attribute. Hiding the warning does not
eliminate the problem that the warning is warning about.

We'll turn them back on for 5.15 and later. (Is there any reason that they
should be enabled for earlier perls ?)

(And could someone please forward this to pdl-porters? Thank you.)

Done.

P.S.​: I think PDL’s test suite needs to cover lvalue subroutines. It is
passing its own tests in 5.15.6, when it shouldn’t.

Noted.
I don't know if this will be addressed for the 2.4.10 test suite, but it
certainly needs to be done for 2.4.11.

Thanks for taking the time to have a look (and a think) about this.

Cheers,
Rob

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 6, 2012

From @cpansprout

On Thu Jan 05 17​:32​:44 2012, sisyphus1@​optusnet.com.au wrote​:

----- Original Message -----
From​: "Father Chrysostomos via RT"

Unless there will be another PDL release after that before May, I think
it should wait until this issue is resolved.

We are aiming for a 2.4.11 before then.

I’ve noticed that the latest dev PDL (2.4.9_992) has ‘no warnings
'misc'’ when applying the lvalue attribute. Hiding the warning does not
eliminate the problem that the warning is warning about.

We'll turn them back on for 5.15 and later. (Is there any reason that
they
should be enabled for earlier perls ?)

Is there any reason to disable them for older perls?

(From reading the changelog quickly, I thought ‘no warnings 'misc'’ had
been added for the sake of 5.15.6. Maybe I’m mistaken.)

Of course, whether you have warnings on or not is irrelevant now that we
know about the problem. :-)

(And could someone please forward this to pdl-porters? Thank you.)

Done.

P.S.​: I think PDL’s test suite needs to cover lvalue subroutines. It is
passing its own tests in 5.15.6, when it shouldn’t.

Noted.
I don't know if this will be addressed for the 2.4.10 test suite, but it
certainly needs to be done for 2.4.11.

Thanks for taking the time to have a look (and a think) about this.

Cheers,
Rob

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 29, 2012

From @rjbs

There were at least two distinct threads about this on p5p recently, at least one of which
included me mistakenly believing that just never worked. Oops!

What's the current status of this blocker?

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 29, 2012

From [Unknown Contact. See original ticket]

There were at least two distinct threads about this on p5p recently, at least one of which
included me mistakenly believing that just never worked. Oops!

What's the current status of this blocker?

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 29, 2012

@rjbs - Status changed from 'open' to 'resolved'

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 29, 2012

From @cpansprout

On Tue Feb 28 20​:16​:32 2012, rjbs wrote​:

There were at least two distinct threads about this on p5p recently,
at least one of which
included me mistakenly believing that just never worked. Oops!

What's the current status of this blocker?

It’s currently marked ‘resolved’, by you. :-)

PDL currently uses attributes.pm to set the lvalue attribute on a
pure-Perl sub.

Because :lvalue has never actually worked properly on existing Perl
subs, it was changed to warn and do nothing. I modified attributes.pm
to do the same, for the sake of consistency. (Turning on the lvalue
flag may or may not work, depending on what code is inside the sub.)

Now, the inability of :lvalue to make an already-defined Perl sub into a
true lvalue sub has never been a problem for PDL, because of its
particular use case (assignment overloading on returned object).

So the question is​: Should PDL work around this, or should we give
people a bit of rope (called attributes.pm) for those who really know
what they are doing, with appropriate caveats in the docs?

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 29, 2012

@rjbs - Status changed from 'resolved' to 'open'

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 29, 2012

From @rjbs

On Tue Feb 28 20​:44​:19 2012, sprout wrote​:

On Tue Feb 28 20​:16​:32 2012, rjbs wrote​:

There were at least two distinct threads about this on p5p recently,
at least one of which
included me mistakenly believing that just never worked. Oops!

What's the current status of this blocker?

It’s currently marked ‘resolved’, by you. :-)

Whoops! Must've clicked "resolve" instead of "reply." Unresolved.

PDL currently uses attributes.pm to set the lvalue attribute on a
pure-Perl sub.

Because :lvalue has never actually worked properly on existing Perl
subs, it was changed to warn and do nothing. I modified attributes.pm
to do the same, for the sake of consistency. (Turning on the lvalue
flag may or may not work, depending on what code is inside the sub.)

Got it.

So the question is​: Should PDL work around this, or should we give
people a bit of rope (called attributes.pm) for those who really know
what they are doing, with appropriate caveats in the docs?

I *think* this is an unusual case, so I am tempted to say that PDL should
adapt. But I'm concerned that there are going to be other users affected
by the same thing.

My next reaction then would be that​:

* we should restore the rope
* ...and add a warning and docs
* ..,so PDL keeps working
* ...but is urged (both by the warning and by this thread) to work around it
* ...and any other darkpan code that would be broken will now get this same warning.

I'm very interested in your thoughts on that plan.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 29, 2012

From @cpansprout

On Wed Feb 29 06​:58​:21 2012, rjbs wrote​:

My next reaction then would be that​:

* we should restore the rope
* ...and add a warning and docs
* ..,so PDL keeps working
* ...but is urged (both by the warning and by this thread) to work
around it
* ...and any other darkpan code that would be broken will now get this
same warning.

I'm very interested in your thoughts on that plan.

That sounds good to me.

I was started to get a bit concerned about forcing PDL to adapt *now*,
by applying the lvalue attribute before/when defining the sub, because
that would cause lvalue bugs in older Perls to crop up.

Restoring the rope but leaving the warning (which PDL already
suppresses) seems a good compromise.

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 1, 2012

From @rjbs

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2012-02-29T12​:22​:23]

On Wed Feb 29 06​:58​:21 2012, rjbs wrote​:

My next reaction then would be that​:

* we should restore the rope
* ...and add a warning and docs
* ..,so PDL keeps working
* ...but is urged (both by the warning and by this thread) to work
around it
* ...and any other darkpan code that would be broken will now get this
same warning.

I'm very interested in your thoughts on that plan.

That sounds good to me.

Great. I'm, uh, gonna leave that to you to take care of? :) That work for
you?

--
rjbs

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 1, 2012

From @cpansprout

On Wed Feb 29 17​:51​:12 2012, perl.p5p@​rjbs.manxome.org wrote​:

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2012-02-
29T12​:22​:23]

On Wed Feb 29 06​:58​:21 2012, rjbs wrote​:

My next reaction then would be that​:

* we should restore the rope
* ...and add a warning and docs
* ..,so PDL keeps working
* ...but is urged (both by the warning and by this thread) to work
around it
* ...and any other darkpan code that would be broken will now get
this
same warning.

I'm very interested in your thoughts on that plan.

That sounds good to me.

Great. I'm, uh, gonna leave that to you to take care of? :) That
work for
you?

Yes, but it may be a few days.

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 1, 2012

From @cpansprout

On Wed Feb 29 06​:58​:21 2012, rjbs wrote​:

* we should restore the rope
* ...and add a warning

Done with commit 345d70e.

and docs

Commit c217304.

* ..,so PDL keeps working

Untested. I have to go now, so I would appreciate it if someone would
beat me to it. :-)

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 1, 2012

From dcmertens.perl@gmail.com

Wow, wonderful. Sorry I've been mostly absent from the discussion. The
concern has been noted and discussed on the pdl porters list and we're
working towards a solution, slowly. It's nice to have this not break
behavior anymore, though I happen to agree that this is a weird edge case
that PDL should probably stop assuming.

On Thu, Mar 1, 2012 at 2​:36 PM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Wed Feb 29 06​:58​:21 2012, rjbs wrote​:

* we should restore the rope
* ...and add a warning

Done with commit 345d70e.

and docs

Commit c217304.

* ..,so PDL keeps working

Untested. I have to go now, so I would appreciate it if someone would
beat me to it. :-)

--

Father Chrysostomos

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=107366

--
"Debugging is twice as hard as writing the code in the first place.
  Therefore, if you write the code as cleverly as possible, you are,
  by definition, not smart enough to debug it." -- Brian Kernighan

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 1, 2012

From @rjbs

* David Mertens <dcmertens.perl@​gmail.com> [2012-03-01T18​:18​:58]

Wow, wonderful. Sorry I've been mostly absent from the discussion. The
concern has been noted and discussed on the pdl porters list and we're
working towards a solution, slowly. It's nice to have this not break
behavior anymore, though I happen to agree that this is a weird edge case
that PDL should probably stop assuming.

Does this mean that you've tested it with bleadperl and confirmed things are
okay? :) I'd love to close the ticket.

--
rjbs

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 2, 2012

From @sisyphus

On Thu Mar 01 15​:30​:09 2012, perl.p5p@​rjbs.manxome.org wrote​:

Does this mean that you've tested it with bleadperl and confirmed
things are
okay? :) I'd love to close the ticket.

I don't know if David has tested it - but I've just tested this on MS
Windows (which *did* suffer from the breakage), and it's now fine. It
was also fine when I tested by reverting sprout's commit, as per his
suggestion earlier in this thread from a few weeks back.

However, I don't see any warnings during the running of the
PDL-Stats-0.5.5 test suite. Should I ? Perhaps PDL​::Stats (or, more
likely, PDL) has taken steps to mask them ?

I think you can safely close this bug report - unless this "no visible
warnings" issue warrants keeping it open.

Cheers,
Rob

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 2, 2012

@rjbs - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Mar 2, 2012
@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 2, 2012

From @rjbs

* Sisyphus via RT <perlbug-followup@​perl.org> [2012-03-01T22​:49​:01]

However, I don't see any warnings during the running of the
PDL-Stats-0.5.5 test suite. Should I ? Perhaps PDL​::Stats (or, more
likely, PDL) has taken steps to mask them ?

I think you can safely close this bug report - unless this "no visible
warnings" issue warrants keeping it open.

FC wrote, earlier​:

  Restoring the rope but leaving the warning (which PDL already
  suppresses) seems a good compromise.

Will close.

Thanks very much, Rob.

--
rjbs

@p5pRT p5pRT added the Severity Low label Oct 19, 2019
@p5pRT p5pRT added the distro-Linux label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.