Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

warn broken when operand is overloaded #13641

Closed p5pRT closed 6 years ago

p5pRT commented 10 years ago

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

Searchable as RT121372$

p5pRT commented 10 years ago

From schmorp@schmorp.de

Created by schmorp@schmorp.de

"warn" is documented to append the file/line number text when the warning doesn't end in a newline\, which is obviously useful for warning messages.

howver\, it doesn't do so when the last argument is overloaded to stringify\, apparently since 5.14.x (5.12.5. still correctly appends line number info\, 5.14.4 and newer perl versions do not).

one practical difference is that warning messages that end with such objects (for example\, Math​::GMP "numbers") will be reported with the wrong line number when warn hooks are used (which is a common situation).

here is a test script​:

  package X;

  use overload '""' => sub { 1 };

  our $one = bless [];

  $SIG{__WARN__} = sub { warn $_[0] }; # line 7

  warn $one; # line 9

perl 5.12.5 output​: 1 at /tmp/x line 9.

perl 5.18.1 output​: 1 at /tmp/x line 7.

i.e.\, the line number is wrong in 5.18.1.

this *could* be construed as a documentation bug (as apparently\, there was some attempt to make warn more like die\, for no apparently useful reason)\, however\, I'd say passing objects around with warn is less useful than having correct line numbers when logging.

Perl Info ``` Flags: category=core severity=low Site configuration information for perl 5.18.1: Configured by Marc Lehmann at Mon Oct 21 09:27:08 CEST 2013. Summary of my perl5 (revision 5 version 18 subversion 1) configuration: Platform: osname=linux, osvers=3.10-3-amd64, archname=x86_64-linux uname='linux cerebro 3.10-3-amd64 #1 smp debian 3.10.11-1 (2013-09-10) x86_64 gnulinux ' config_args='-Duselargefiles -Duse64bitint -Dusemymalloc=n -Dstatic_ext=Fcntl -Dcc=ccache gcc -Dccflags=-DPERL_DISABLE_PMC -DPERL_ARENA_SIZE=16376 -USITEARCH_EXP -USITELIB_EXP -UARCHLIB_EXP -D_GNU_SOURCE -I/opt/include -ggdb -gdwarf-2 -g3 -Doptimize=-O6 -fno-asynchronous-unwind-tables -fno-strict-aliasing -Dcccdlflags=-fPIC -Dldflags=-L/opt/perl/lib -L/opt/lib -Dlibs=-ldl -lm -lcrypt -Dprefix=/opt/perl -Dprivlib=/opt/perl/lib/perl5 -Darchlib=/opt/perl/lib/perl5 -Uusevendorprefix -Dsiteprefix=/opt/perl -Dsitelib=/opt/perl/lib/perl5 -Dsitearch=/opt/perl/lib/perl5 -Dsitebin=/opt/perl/bin -Dman1dir=/opt/perl/man/man1 -Dman3dir=/opt/perl/man/man3 -Dsiteman1dir=/opt/perl/man/man1 -Dsiteman3dir=/opt/perl/man/man3 -Dman1ext=1 -Dman3ext=3 -Dpager=/usr/bin/less -Uafs -Uusesfio -Uusenm -Uuseshrplib -Ud_dosuid -Dusethreads=undef -Duse5005threads=undef -Duseithreads=undef -Dusemultiplicity=undef -Demail=perl-binary@plan9.de -Dcf_email=perl-binary@plan9.de -Dcf_by=Marc Lehmann -Dlocincpth=/opt/perl/include /opt/include -Dmyhostname=localhost -Dmultiarch=undef -Dbin=/opt/perl/bin -Dxxxusedevel -DxxxDEBUGGING -Dxxxuse_debugging_perl -Dxxxuse_debugmalloc -dEs' 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=undef usemymalloc=n, bincompat5005=undef Compiler: cc='ccache gcc', ccflags ='-DPERL_DISABLE_PMC -DPERL_ARENA_SIZE=16376 -USITEARCH_EXP -USITELIB_EXP -UARCHLIB_EXP -D_GNU_SOURCE -I/opt/include -ggdb -gdwarf-2 -g3 -fno-strict-aliasing -pipe -I/opt/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O6 -fno-asynchronous-unwind-tables -fno-strict-aliasing', cppflags='-DPERL_DISABLE_PMC -DPERL_ARENA_SIZE=16376 -USITEARCH_EXP -USITELIB_EXP -UARCHLIB_EXP -D_GNU_SOURCE -I/opt/include -ggdb -gdwarf-2 -g3 -fno-strict-aliasing -pipe -I/opt/include' ccversion='', gccversion='4.7.2', 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='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='ccache gcc', ldflags ='-L/opt/perl/lib -L/opt/lib -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-ldl -lm -lcrypt perllibs=-ldl -lm -lcrypt libc=, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.17' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O6 -fno-asynchronous-unwind-tables -fno-strict-aliasing -L/opt/perl/lib -L/opt/lib -L/usr/local/lib' Locally applied patches: @INC for perl 5.18.1: /opt/perl/lib/perl5 /opt/perl/lib/perl5 . Environment for perl 5.18.1: HOME=/root LANG (unset) LANGUAGE (unset) LC_CTYPE=en_US.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PERL5_CPANPLUS_CONFIG=/root/.cpanplus/config PERLDB_OPTS=ornaments=0 PERL_ANYEVENT_DBI_TESTS=1 PERL_ANYEVENT_LOOP_TESTS=1 PERL_ANYEVENT_NET_TESTS=1 PERL_BADLANG (unset) PERL_UNICODE=E SHELL=/bin/bash ```
p5pRT commented 10 years ago

From zefram@fysh.org

schmorp@​schmorp.de wrote​:

howver\, it doesn't do so when the last argument is overloaded to stringify\, apparently since 5.14.x (5.12.5. still correctly appends line number info\, 5.14.4 and newer perl versions do not).

It was a deliberate change to preserve blessed objects rather than collapse them to strings. The location information is only appended if the input is sufficiently simple that nothing is lost by doing so. The documentation ought to be clearer about it.

-zefram

p5pRT commented 10 years ago

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

p5pRT commented 10 years ago

From schmorp@schmorp.de

On Tue\, Mar 04\, 2014 at 03​:13​:32AM -0800\, Zefram via RT \perlbug\-followup@​perl\.org wrote​:

It was a deliberate change to preserve blessed objects rather than collapse them to strings.

Thats a completely useless change of course(*). OTOH\, the disadvantage of losing line number infos for warnings is big. Since it is undocumented behaviour (and useless)\, I would strongly suggest treating it as a regression and implementing it as documented.

The documentation ought to be clearer about it.

"clearer"? The documentation is absolutely clear that line number info is appended in this case\, there is nothing unclear about it.

(*) you can just define your own warn function that accepts objects if you need this feature (all logging frameworks support this). this cannot be done with die\, for which this feature is genuinely useful (and existed before this change).

--   The choice of a Deliantra\, the free code+content MORPG   -----==- _GNU_ http​://www.deliantra.net   ----==-- _ generation   ---==---(_)__ __ ____ __ Marc Lehmann   --==---/ / _ \/ // /\ \/ / schmorp@​schmorp.de   -=====/_/_//_/\_\,_/ /_/\_\

p5pRT commented 10 years ago

From schmorp@schmorp.de

On Tue\, Mar 04\, 2014 at 03​:13​:32AM -0800\, Zefram via RT \perlbug\-followup@​perl\.org wrote​:

It was a deliberate change to preserve blessed objects rather than collapse them to strings.

Indeed\, I just realised (in another debugging session) that line number information is lost for almost anything\, including naked references\, not just objects or overloaded objects.

Again\, I can only urge you to undo this needless and pointless change - it makes "warn" close to useless for general debugging.

--   The choice of a Deliantra\, the free code+content MORPG   -----==- _GNU_ http​://www.deliantra.net   ----==-- _ generation   ---==---(_)__ __ ____ __ Marc Lehmann   --==---/ / _ \/ // /\ \/ / schmorp@​schmorp.de   -=====/_/_//_/\_\,_/ /_/\_\

p5pRT commented 10 years ago

From zefram@fysh.org

Marc Lehmann wrote​:

Again\, I can only urge you to undo this needless and pointless change - it makes "warn" close to useless for general debugging.

You're free to stringify a reference yourself before passing it to warn\, if you actually want a warning message to consist of the stringified form of the reference and get the line number appended. This arrangement gives you the choice about how a reference is to be treated.

-zefram

p5pRT commented 10 years ago

From schmorp@schmorp.de

On Thu\, Mar 06\, 2014 at 02​:35​:46AM -0800\, Zefram via RT \perlbug\-followup@​perl\.org wrote​:

You're free to stringify a reference yourself before passing it to warn\, if you actually want a warning message to consist of the stringified form of the reference and get the line number appended. This arrangement gives you the choice about how a reference is to be treated.

Which is the same choice the former arrangement gave me\, of course. I have no problem with that.

The former arrangement\, however\, is a) backwards compatible and b) works as documented in all versions of perl - the current arrangement fails both\, and adds no additional benefit (unlike the much older change to die for example\, which gives important benefits).

So the fact remains that its a pointless and unneeded change\, only designed to break backwards compatibility and making using warn for warning messages harder\, while not giving any benefits.

That we even have this discussion is shocking\, to be honest. p5p *needs* to stop breaking backards compatibility for no reason\, and it *needs* to stop this bullshit arguing about things that are clearly\, and without any doubt\, bugs (in this case either in the docs or in the code).

The fact that you can't even admit that this is a bug completely disqualifies you from having an informed opinion about this.

--   The choice of a Deliantra\, the free code+content MORPG   -----==- _GNU_ http​://www.deliantra.net   ----==-- _ generation   ---==---(_)__ __ ____ __ Marc Lehmann   --==---/ / _ \/ // /\ \/ / schmorp@​schmorp.de   -=====/_/_//_/\_\,_/ /_/\_\

p5pRT commented 6 years ago

From zefram@fysh.org

I've corrected and clarified the documentation in commit 52e58e76455a7593547f41dc02823f4b5f83185c. This ticket can be closed.

-zefram

p5pRT commented 6 years ago

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