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 v5.27.5-398-g19a8de4862 breaks MLEHMANN/AnyEvent-HTTP-2.23.tar.gz #16284

Closed
p5pRT opened this issue Dec 1, 2017 · 10 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Dec 1, 2017

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

Searchable as RT132527$

@p5pRT
Copy link
Author

p5pRT commented Dec 1, 2017

From @andk

Thanks to Slaven for discovering this one.

bisect points to​:

: commit 19a8de4
: Author​: Zefram <zefram@​fysh.org>
: Date​: Thu Nov 16 14​:56​:11 2017 +0000
:
: fix lvalue context for 4-arg substr

This is the second BBC for this distro in a short row. The other is
covered in https://rt.perl.org/Public/Bug/Display.html?id=132142

To produce the new one I applied Ilmari's patch for AnyEvent-7.14.

Diagnostics now​:

: Can't modify delete in substr at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1104, near """)"
: Type of arg 1 to AnyEvent​::Util​::guard must be block or sub {} (not reference constructor) at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1238, near "}"
: syntax error at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1433, near "$ENV{http_proxy"
: syntax error at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1434, near "}"
: Compilation failed in require at t/00_load.t line 3.
: BEGIN failed--compilation aborted at t/00_load.t line 3.

perl -V


Summary of my perl5 (revision 5 version 27 subversion 6) configuration​:
  Commit id​: 19a8de4
  Platform​:
  osname=linux
  osvers=4.12.0-2-amd64
  archname=x86_64-linux-thread-multi
  uname='linux k93msid 4.12.0-2-amd64 #1 smp debian 4.12.13-1 (2017-09-19) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.5-398-g19a8de4862/1e0c -Dmyhostname=k93msid -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 -Duseithreads -Uuselongdouble -DEBUGGING=-g'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=define
  usemultiplicity=define
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-D_REENTRANT -D_GNU_SOURCE -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='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='7.2.0'
  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='double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  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/7/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 -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.24.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.24'
  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
  MULTIPLICITY
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_IMPLICIT_CONTEXT
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_ITHREADS
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_PERLIO
  USE_PERL_ATOF
  USE_REENTRANT_API
  Built under linux
  Compiled at Nov 16 2017 15​:19​:59
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.5-398-g19a8de4862/1e0c/lib/site_perl/5.27.6/x86_64-linux-thread-multi
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.5-398-g19a8de4862/1e0c/lib/site_perl/5.27.6
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.5-398-g19a8de4862/1e0c/lib/5.27.6/x86_64-linux-thread-multi
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.5-398-g19a8de4862/1e0c/lib/5.27.6

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2017

From @cpansprout

On Fri, 01 Dec 2017 15​:27​:39 -0800, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

Thanks to Slaven for discovering this one.

bisect points to​:

: commit 19a8de4
: Author​: Zefram <zefram@​fysh.org>
: Date​: Thu Nov 16 14​:56​:11 2017 +0000
:
: fix lvalue context for 4-arg substr

This is the second BBC for this distro in a short row. The other is
covered in https://rt.perl.org/Public/Bug/Display.html?id=132142

To produce the new one I applied Ilmari's patch for AnyEvent-7.14.

Diagnostics now​:

: Can't modify delete in substr at /tmp/loop_over_bdir-15721-
ERyp63/AnyEvent-HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1104, near
""")"

I have a fix for that first error. I will push it as soon as tests finish.

: Type of arg 1 to AnyEvent​::Util​::guard must be block or sub {} (not
reference constructor) at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-
HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1238, near "}"
: syntax error at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-HTTP-2.23-
0/blib/lib/AnyEvent/HTTP.pm line 1433, near "$ENV{http_proxy"
: syntax error at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-HTTP-2.23-
0/blib/lib/AnyEvent/HTTP.pm line 1434, near "}"
: Compilation failed in require at t/00_load.t line 3.
: BEGIN failed--compilation aborted at t/00_load.t line 3.

I hope these are just side-effects of the first error.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2017

From @cpansprout

On Fri, 01 Dec 2017 18​:21​:29 -0800, sprout wrote​:

On Fri, 01 Dec 2017 15​:27​:39 -0800,
andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

: Can't modify delete in substr at /tmp/loop_over_bdir-15721-
ERyp63/AnyEvent-HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1104, near
""")"

I have a fix for that first error. I will push it as soon as tests
finish.

Now pushed as 833d07d.

: Type of arg 1 to AnyEvent​::Util​::guard must be block or sub {} (not
reference constructor) at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-
HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1238, near "}"
: syntax error at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-HTTP-
2.23-
0/blib/lib/AnyEvent/HTTP.pm line 1433, near "$ENV{http_proxy"
: syntax error at /tmp/loop_over_bdir-15721-ERyp63/AnyEvent-HTTP-
2.23-
0/blib/lib/AnyEvent/HTTP.pm line 1434, near "}"
: Compilation failed in require at t/00_load.t line 3.
: BEGIN failed--compilation aborted at t/00_load.t line 3.

I hope these are just side-effects of the first error.

Do you still get these other errors?

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2017

From @andk

On Fri, 01 Dec 2017 18​:48​:45 -0800, "Father Chrysostomos via RT" <perlbug-followup@​perl.org> said​:

I hope these are just side-effects of the first error.

  > Do you still get these other errors?

Beautiful​:

: All tests successful.
: Files=3, Tests=28, 1 wallclock secs ( 0.03 usr 0.02 sys + 0.16 cusr 0.01 csys = 0.22 CPU)
: Result​: PASS
: MLEHMANN/AnyEvent-HTTP-2.23.tar.gz
: /usr/bin/make test -- OK

This ticket can be closed.

Thanks!
--
andreas

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2017

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

@p5pRT p5pRT closed this as completed Dec 2, 2017
@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2017

From zefram@fysh.org

Father Chrysostomos via RT wrote​:

On Fri, 01 Dec 2017 15​:27​:39 -0800, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

: Can't modify delete in substr at /tmp/loop_over_bdir-15721-
ERyp63/AnyEvent-HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1104, near
""")"

I have a fix for that first error. I will push it as soon as tests finish.

The change you've made is to use loose lvalue context for the first arg
of a 4-arg substr. I think that is a bad change and should be reverted.
4-arg substr does not only sometimes mutate its first argument; it
*always* writes to it. Doing so is the primary purpose of 4-arg substr.
It should therefore require a fully-qualified lvalue. It is correct for
it to reject the use of a delete there, consistent with how assignment
to a delete is rejected.

As far as I can see, AnyEvent​::HTTP has no reason to use a mutating
substr on a delete there. It should be using either a mutating substr
on a hash element left in place, or a non-mutating substr on a delete.

I reckon commit 833d07d should be
reverted.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2017

From @cpansprout

On Mon, 04 Dec 2017 09​:37​:56 -0800, zefram@​fysh.org wrote​:

Father Chrysostomos via RT wrote​:

On Fri, 01 Dec 2017 15​:27​:39 -0800,
andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

: Can't modify delete in substr at /tmp/loop_over_bdir-15721-
ERyp63/AnyEvent-HTTP-2.23-0/blib/lib/AnyEvent/HTTP.pm line 1104,
near
""")"

I have a fix for that first error. I will push it as soon as tests
finish.

The change you've made is to use loose lvalue context for the first
arg
of a 4-arg substr. I think that is a bad change and should be
reverted.
4-arg substr does not only sometimes mutate its first argument; it
*always* writes to it. Doing so is the primary purpose of 4-arg
substr.
It should therefore require a fully-qualified lvalue. It is correct
for
it to reject the use of a delete there, consistent with how assignment
to a delete is rejected.

The acceptance or rejection of any particular op as an lvalue is necessarily arbitrary. There is actually no syntactic need for 3=4 to be prohibited, because it will croak at run time anyway. The only reason it is prohibited is that it would be unhelpful to programmers to allow such obviously sloppy code to compile. This arbitrariness actually prevents some useful constructs​:

++tie $X, "foo";

If foo​::TIESCALAR returns an object with overloading that makes ++ meaningful, then that construct could be used, were it not for the compile-time check. I’m not arguing that this case should be made to work. I’m just using it as an example to demonstrate how arbitrary compile-time lvalue checks are. Other examples​:

${\3} = 4; # This could die at compile time.
$x.$y = 7; # SomeClass​::(. could be an lvalue function.
for(3){$_++} # This could die at compile time.

As far as I can see, AnyEvent​::HTTP has no reason to use a mutating
substr on a delete there. It should be using either a mutating substr
on a hash element left in place, or a non-mutating substr on a delete.

It may be that AnyEvent​::HTTP is using four-argument substr unnecessarily. But I tend to see *any* CPAN failure as merely a canary in the coalmine, and not the only failure. The fact that AnyEvent​::HTTP uses it that way is enough to prove that we have broken backward-compatibility when we did not need to. Since the first argument to substr has never ever had strict compile-time lvalue checks applied to it, I think it would be wrong to impose it now, because of the arbitrariness I mentioned above.

I reckon commit 833d07d should be
reverted.

I think backward compatibility should win. The current (restored) behaviour is not clearly a bug.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2017

From zefram@fysh.org

Father Chrysostomos via RT wrote​:

The acceptance or rejection of any particular op as an lvalue is
necessarily arbitrary.

This is true; there is some arbitrariness, and even a few cases where it's
useful to work around a forbidden type of lvalue. But the arbitrariness
is mostly consistent. For it to be inconsistent here seems to be a bug.

The only reason it is prohibited is that it would be unhelpful to
programmers to allow such obviously sloppy code to compile.

It is just as helpful to reject substr on a delete as it is to reject
assignment to a delete.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2017

From @cpansprout

On Mon, 04 Dec 2017 13​:53​:36 -0800, zefram@​fysh.org wrote​:

Father Chrysostomos via RT wrote​:

The acceptance or rejection of any particular op as an lvalue is
necessarily arbitrary.

This is true; there is some arbitrariness, and even a few cases where it's
useful to work around a forbidden type of lvalue. But the arbitrariness
is mostly consistent. For it to be inconsistent here seems to be a bug.

The only reason it is prohibited is that it would be unhelpful to
programmers to allow such obviously sloppy code to compile.

It is just as helpful to reject substr on a delete as it is to reject
assignment to a delete.

And it is also helpful to allow it. Traditionally Perl has retained backward compatibility in such cases.

I do not believe we can reach an agreement on this. It would be helpful if someone else would chip in.

--

Father Chrysostomos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant