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

warn broken when operand is overloaded #13641

Closed
p5pRT opened this issue Mar 4, 2014 · 9 comments
Closed

warn broken when operand is overloaded #13641

p5pRT opened this issue Mar 4, 2014 · 9 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Mar 4, 2014

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

Searchable as RT121372$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 4, 2014

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
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 4, 2014

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
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 4, 2014

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 4, 2014

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
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 6, 2014

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
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 6, 2014

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
Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 7, 2014

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
Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 16, 2017

From zefram@fysh.org

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

-zefram

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 27, 2017

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

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.