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

perlcall documentation has no good example with POPs #13314

Open
p5pRT opened this issue Sep 26, 2013 · 12 comments
Open

perlcall documentation has no good example with POPs #13314

p5pRT opened this issue Sep 26, 2013 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 26, 2013

Migrated from rt.perl.org#120017 (status was 'open')

Searchable as RT120017$

@p5pRT
Copy link
Author

p5pRT commented Sep 26, 2013

From @ruz

Created by @ruz

perldoc perlcall lacks good example with POPs. Since poped SV is on
temp stack it will be freed as soon as you call FREETMPS and LEAVE
the block.

I had to grep cpan to understand what to do to avoid freeing. Still
not shure if it's ok to call SvREFCNT_inc(POPs) or if it has to be
newSVsv(POPs). If former then example should use most effective
variant of SvREFCNT_inc*.

Perl Info

Flags:
    category=docs
    severity=low

Site configuration information for perl 5.16.1:

Configured by ruz at Tue Sep 11 03:03:12 MSK 2012.

Summary of my perl5 (revision 5 version 16 subversion 1) configuration:
   
  Platform:
    osname=darwin, osvers=12.1.0, archname=darwin-2level
    uname='darwin macbook-ruslan.local 12.1.0 darwin kernel version 12.1.0: tue aug 14 13:29:55 pdt 2012; root:xnu-2050.9.2~1release_x86_64 x86_64 '
    config_args='-de -Duserelocatableinc -Dprefix=/Users/ruz/perl5/perlbrew/perls/perl-5.16.1 -DDEBUGGING -Aeval:scriptdir=/Users/ruz/perl5/perlbrew/perls/perl-5.16.1/bin'
    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='cc', ccflags ='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/opt/local/include',
    optimize='-O3 -g',
    cppflags='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/opt/local/include'
    ccversion='', gccversion='4.2.1 Compatible Apple Clang 4.0 ((tags/Apple/clang-421.0.60))', 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='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/opt/local/lib'
    libpth=/opt/local/lib /usr/lib
    libs=-lgdbm -ldbm -ldl -lm -lutil -lc
    perllibs=-ldl -lm -lutil -lc
    libc=, so=dylib, useshrplib=false, libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/opt/local/lib -fstack-protector'

Locally applied patches:
    


@INC for perl 5.16.1:
    /Users/ruz/perl5/perlbrew/perls/perl-5.16.1/lib/site_perl/5.16.1/darwin-2level
    /Users/ruz/perl5/perlbrew/perls/perl-5.16.1/lib/site_perl/5.16.1
    /Users/ruz/perl5/perlbrew/perls/perl-5.16.1/lib/5.16.1/darwin-2level
    /Users/ruz/perl5/perlbrew/perls/perl-5.16.1/lib/5.16.1
    .


Environment for perl 5.16.1:
    DYLD_LIBRARY_PATH=/opt/oracle/instantclient-11.2.0.3.0-macosx-x64:
    HOME=/Users/ruz
    LANG=ru_RU.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/Users/ruz/perl5/perlbrew/bin:/Users/ruz/perl5/perlbrew/perls/perl-5.16.1/bin:/Users/ruz/bin:/opt/local/bin:/opt/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/opt/X11/bin
    PERLBREW_BASHRC_VERSION=0.66
    PERLBREW_HOME=/Users/ruz/.perlbrew
    PERLBREW_MANPATH=/Users/ruz/perl5/perlbrew/perls/perl-5.16.1/man
    PERLBREW_PATH=/Users/ruz/perl5/perlbrew/bin:/Users/ruz/perl5/perlbrew/perls/perl-5.16.1/bin
    PERLBREW_PERL=perl-5.16.1
    PERLBREW_ROOT=/Users/ruz/perl5/perlbrew
    PERLBREW_VERSION=0.66
    PERL_BADLANG (unset)
    SHELL=/opt/local/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2013

From @jkeenan

On Thu Sep 26 03​:49​:32 2013, ruslan.zakirov@​gmail.com wrote​:

This is a bug report for perl from Ruslan.Zakirov@​gmail.com,
generated with the help of perlbug 1.39 running under perl 5.16.1.

-----------------------------------------------------------------
[Please describe your issue here]

perldoc perlcall lacks good example with POPs. Since poped SV is on
temp stack it will be freed as soon as you call FREETMPS and LEAVE
the block.

I had to grep cpan to understand what to do to avoid freeing. Still
not shure if it's ok to call SvREFCNT_inc(POPs) or if it has to be
newSVsv(POPs). If former then example should use most effective
variant of SvREFCNT_inc*.

Do you feel you learned enough from that grepping to write a first draft
of such documentation?

I'm sure that others knowledgeable in that area could help get a draft
polished enough to be included in the docs.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2013

From @Leont

On Thu Sep 26 03​:49​:32 2013, ruslan.zakirov@​gmail.com wrote​:

perldoc perlcall lacks good example with POPs. Since poped SV is on
temp stack it will be freed as soon as you call FREETMPS and LEAVE
the block.

It's FREETMPS that is relevant in this case, as mortal values are fried
on that.

I had to grep cpan to understand what to do to avoid freeing. Still
not shure if it's ok to call SvREFCNT_inc(POPs) or if it has to be
newSVsv(POPs). If former then example should use most effective
variant of SvREFCNT_inc*.

POPs should never be used in a macro that can evaluate it multiple
times. You want to pop it into a variable when passing it to any macro
(such as SvREFCNT_inc) and you may or may not have to un-temp it after
doing SvREFCNT_inc. newSVsv would be a more foolproof approach.

That said, it begs the question "why are you doing that in the first
place?". Can't you solve the question by moving the FREETMPS? Or just
not use a SAVETMPS/FREETMPS at all (caveats are explained in perlcall).

Leon

Leon

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2013

From ruz@bestpractical.com

On Fri, Sep 27, 2013 at 2​:34 PM, Leon Timmermans via RT <
perlbug-followup@​perl.org> wrote​:

On Thu Sep 26 03​:49​:32 2013, ruslan.zakirov@​gmail.com wrote​:

perldoc perlcall lacks good example with POPs. Since poped SV is on
temp stack it will be freed as soon as you call FREETMPS and LEAVE
the block.

It's FREETMPS that is relevant in this case, as mortal values are fried
on that.

I knew that mortal are freed on FREETMPS, I just didn't know how to
demortalize
variable. grepping cpan gave me answer and shown that I'm not alone, people
comment REFCNT increment after POPs and don't skip saving/freeing temps.

I had to grep cpan to understand what to do to avoid freeing. Still

not shure if it's ok to call SvREFCNT_inc(POPs) or if it has to be
newSVsv(POPs). If former then example should use most effective
variant of SvREFCNT_inc*.

POPs should never be used in a macro that can evaluate it multiple
times. You want to pop it into a variable when passing it to any macro
(such as SvREFCNT_inc) and you may or may not have to un-temp it after
doing SvREFCNT_inc. newSVsv would be a more foolproof approach.

Now I understand what "but can only be used with expressions without side
effects" in description of SvREFCNT_inc_*simple* means. The following text
would have explained a lot "but can only be used with expressions that can
be evaluate twice without side effects".

That said, it begs the question "why are you doing that in the first

place?". Can't you solve the question by moving the FREETMPS? Or just
not use a SAVETMPS/FREETMPS at all (caveats are explained in perlcall).

I've read the section that "explains" it and the section is heavy reading.
It ends with "Mind you, if you are at all uncertain about what to do, it
doesn't do any harm to tidy up anyway.". All examples are tidying up. It's
recommended way to deal for a beginner and it works in so many examples.
How can I guess that to use perl structure returned from a call_* I had to
consider dropping SAVE/FREE temps pairs? Is it really good idea to drop
them?

Item 5 in "Passing Parameters" gives good explanation on why FREETMPS is
required, mentions that return values are freed by it. It is very easy to
understand. What would have helped me is this pointing at two places​:

1) see the section "Using returned values after call"

2) See the section Using Perl to Dispose of Temporaries for details of an
alternative to using these macros.

All examples in sections starting with "Returning ..." map returned values
to some native variable and never use returned SV, never use returned value
outside of LEAVE. Interesting question that is also not clear from the
documentation​: "What would happen with result of POPpx after freeing
temps?".

I hope above helps to understand my motives behind this request for
improvement.

Creating a quest to send doc patches, not sure when I get there.

Leon

Leon

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

--
Best regards, Ruslan.

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2013

From @ikegami

On Fri, Sep 27, 2013 at 1​:58 PM, Ruslan Zakirov <ruz@​bestpractical.com>wrote​:

Now I understand what "but can only be used with expressions without side
effects" in description of SvREFCNT_inc_*simple* means. The following text
would have explained a lot "but can only be used with expressions that can
be evaluate twice without side effects".

In CS, a function has a side effect if it changes anything external.
http​://en.wikipedia.org/wiki/Side_effect_%28computer_science%29

e.g.
lc # Has no side effects. A true function.
chomp # Has the side effect of changing arguments.
print # Has the side effect of changing the terminal.
POPs # Has the side effect of changing the stack.

"Side effect" does not mean "adverse consequence", not even in medicine.

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2013

From @ruz

On Fri, Sep 27, 2013 at 10​:55 PM, Eric Brine via RT <
perlbug-followup@​perl.org> wrote​:

On Fri, Sep 27, 2013 at 1​:58 PM, Ruslan Zakirov <ruz@​bestpractical.com

wrote​:

Now I understand what "but can only be used with expressions without side
effects" in description of SvREFCNT_inc_*simple* means. The following
text
would have explained a lot "but can only be used with expressions that
can
be evaluate twice without side effects".

In CS, a function has a side effect if it changes anything external.
http​://en.wikipedia.org/wiki/Side_effect_%28computer_science%29

e.g.
lc # Has no side effects. A true function.
chomp # Has the side effect of changing arguments.
print # Has the side effect of changing the terminal.
POPs # Has the side effect of changing the stack.

"Side effect" does not mean "adverse consequence", not even in medicine.

How do you know lc has no side effects? (rhetorical question) I sit there
and thinking for a moment "does POPs have side effects?" I even forgot that
SvREFCNT_inc* are macroses so quickly switched to "why does it matter?".
Then goes "I don't want to think about it, let's use SvREFCNT_inc".

Does above change make description less exact?

Why did you brought up wiki and CS?

Do you think putting more words would hurt documentation by increasing its
size and making it less likely to be read?

The following is waste of docs estate​:


  SvREFCNT_inc
  Increments the reference count of the given SV.

  All of the following SvREFCNT_inc* macros are optimized versions
  of SvREFCNT_inc, and can be replaced with SvREFCNT_inc.

  SV* SvREFCNT_inc(SV* sv)

  SvREFCNT_inc_NN
  Same as SvREFCNT_inc, but can only be used if you know sv is not
  NULL. Since we don't have to check the NULLness, it's faster and
  smaller.

  SV* SvREFCNT_inc_NN(SV* sv)

  SvREFCNT_inc_simple
  Same as SvREFCNT_inc, but can only be used with expressions
  without side effects. Since we don't have to store a temporary
  value, it's faster.

  SV* SvREFCNT_inc_simple(SV* sv)

  SvREFCNT_inc_simple_NN
  Same as SvREFCNT_inc_simple, but can only be used if you know
  sv is not NULL. Since we don't have to check the NULLness, it's
  faster and smaller.

  SV* SvREFCNT_inc_simple_NN(SV* sv)

  SvREFCNT_inc_simple_void
  Same as SvREFCNT_inc_simple, but can only be used if you don't
  need the return value. The macro doesn't need to return a
  meaningful value.

  void SvREFCNT_inc_simple_void(SV* sv)

  SvREFCNT_inc_simple_void_NN
  Same as SvREFCNT_inc, but can only be used if you don't need the
  return value, and you know that sv is not NULL. The macro
doesn't
  need to return a meaningful value, or check for NULLness, so
it's
....


It can be something like the following​:


  SvREFCNT_inc
  Increments the reference count of the given SV.

  All of the following SvREFCNT_inc_* macros are optimized
versions
  of SvREFCNT_inc, and can be replaced with SvREFCNT_inc.

  SV* SvREFCNT_inc(SV* sv)

  SvREFCNT_inc_*
  Optimized variants of SvREFCNT_inc.

  SV* SvREFCNT_inc_NN(SV* sv)
  SV* SvREFCNT_inc_simple(SV* sv)
  SV* SvREFCNT_inc_simple_NN(SV* sv)
  void SvREFCNT_inc_simple_void(SV* sv)
  ...

  Variants with "NN" in the name can only be used if you know sv
is not
  NULL. Variants with "simple" can only be used with expressions
  that can be evaluated twice without side effects. "void" can
only be
  used if you don't need the return value.


It's shorter. Note that first part is incomplete, it goes on and on for
every variant repeating (sometimes lacking).

PS.

Just looked into sv.h and saw the following​:

321 #define SvREFCNT_inc(sv) ··· ···S_SvREFCNT_inc(MUTABLE_SV(sv))
322 #define SvREFCNT_inc_simple(sv) ···SvREFCNT_inc(sv)
323 #define SvREFCNT_inc_NN(sv) ···S_SvREFCNT_inc_NN(MUTABLE_SV(sv))
324 #define SvREFCNT_inc_void(sv) ·· ···S_SvREFCNT_inc_void(MUTABLE_SV(sv))
325
326 /* These guys don't need the curly blocks */
327 #define SvREFCNT_inc_simple_void(sv) ···STMT_START { if (sv)
SvREFCNT(sv)++; } STMT_END
328 #define SvREFCNT_inc_simple_NN(sv) ·(++(SvREFCNT(sv)),MUTABLE_SV(sv))
329 #define SvREFCNT_inc_void_NN(sv) ···(void)(++SvREFCNT(MUTABLE_SV(sv)))
330 #define SvREFCNT_inc_simple_void_NN(sv)
(void)(++SvREFCNT(MUTABLE_SV(sv)))

So since 5.17.4 SvREFCNT_inc_simple is not optimized anymore and actually
can work with expressions that has side effects. I don't see reason why it
can not be​:

-# define SvREFCNT_inc_simple(sv) \
- ((sv) ? (SvREFCNT(sv)++,MUTABLE_SV(sv)) : NULL)

On Fri, Sep 27, 2013 at 1​:58 PM, Ruslan Zakirov <ruz@​bestpractical.com>wrote​:

Now I understand what "but can only be used with expressions without side
effects" in description of SvREFCNT_inc_*simple* means. The following text
would have explained a lot "but can only be used with expressions that can
be evaluate twice without side effects".

In CS, a function has a side effect if it changes anything external.
http​://en.wikipedia.org/wiki/Side_effect_%28computer_science%29

e.g.
lc # Has no side effects. A true function.
chomp # Has the side effect of changing arguments.
print # Has the side effect of changing the terminal.
POPs # Has the side effect of changing the stack.

"Side effect" does not mean "adverse consequence", not even in medicine.

--
Best regards, Ruslan.

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2013

From mail@steffen-mueller.net

By the way, check out "TOPs", the obvious equivalent to POPs without the
pop. Alas, it's not officially API, which is at least a tad weird. Seems
like a tame enough thing to expose alongside POPs. It's a pure macro in
all versions of Perl that I've ever seen, though, so it's technically
usable.

On 09/27/2013 10​:41 PM, Ruslan Zakirov wrote​:

321 #define SvREFCNT_inc(sv) ··· ···S_SvREFCNT_inc(MUTABLE_SV(sv))
322 #define SvREFCNT_inc_simple(sv) ···SvREFCNT_inc(sv)
323 #define SvREFCNT_inc_NN(sv) ···S_SvREFCNT_inc_NN(MUTABLE_SV(sv))
324 #define SvREFCNT_inc_void(sv) ··
···S_SvREFCNT_inc_void(MUTABLE_SV(sv))
325
326 /* These guys don't need the curly blocks */
327 #define SvREFCNT_inc_simple_void(sv) ···STMT_START { if (sv)
SvREFCNT(sv)++; } STMT_END
328 #define SvREFCNT_inc_simple_NN(sv) ·(++(SvREFCNT(sv)),MUTABLE_SV(sv))
329 #define SvREFCNT_inc_void_NN(sv) ···(void)(++SvREFCNT(MUTABLE_SV(sv)))
330 #define SvREFCNT_inc_simple_void_NN(sv)
(void)(++SvREFCNT(MUTABLE_SV(sv)))

So since 5.17.4 SvREFCNT_inc_simple is not optimized anymore and
actually can work with expressions that has side effects. I don't see
reason why it can not be​:

-# define SvREFCNT_inc_simple(sv) \
-((sv) ? (SvREFCNT(sv)++,MUTABLE_SV(sv)) : NULL)

If it's API, then because it might be being used? Of course, then you
might still be able to define it to another macro variant just for
backcompat.

--Steffen

@p5pRT
Copy link
Author

p5pRT commented Sep 28, 2013

From @ikegami

On Fri, Sep 27, 2013 at 4​:41 PM, Ruslan Zakirov <ruslan.zakirov@​gmail.com>wrote​:

Why did you brought up wiki and CS?

I told you what the word means because you suggested a correction that made
no sense because you didn't know the definition of the word you were using.

Do you think putting more words would hurt documentation by increasing its

size and making it less likely to be read?

That wasn't the problem, no.

@p5pRT
Copy link
Author

p5pRT commented Dec 30, 2013

From @bulk88

On Fri Sep 27 03​:34​:02 2013, LeonT wrote​:

On Thu Sep 26 03​:49​:32 2013, ruslan.zakirov@​gmail.com wrote​:

I had to grep cpan to understand what to do to avoid freeing. Still
not shure if it's ok to call SvREFCNT_inc(POPs) or if it has to be
newSVsv(POPs). If former then example should use most effective
variant of SvREFCNT_inc*.

POPs should never be used in a macro that can evaluate it multiple
times. You want to pop it into a variable when passing it to any macro
(such as SvREFCNT_inc) and you may or may not have to un-temp it after
doing SvREFCNT_inc. newSVsv would be a more foolproof approach.

AFAIK, to get an SV returned from a call_* to live past the FREETMPS, a SvREFCNT_inc is the solution. But, I thought of another situation, what if a DESTROY in a blessed SV being freeded in FREETMPS die()s? The SV* that got a ++ to live past the FREETMPS leaked. The newSVsv solution also leaked.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Dec 31, 2013

From @iabyn

On Mon, Dec 30, 2013 at 11​:30​:06AM -0800, bulk88 via RT wrote​:

On Fri Sep 27 03​:34​:02 2013, LeonT wrote​:

On Thu Sep 26 03​:49​:32 2013, ruslan.zakirov@​gmail.com wrote​:

I had to grep cpan to understand what to do to avoid freeing. Still
not shure if it's ok to call SvREFCNT_inc(POPs) or if it has to be
newSVsv(POPs). If former then example should use most effective
variant of SvREFCNT_inc*.

POPs should never be used in a macro that can evaluate it multiple
times. You want to pop it into a variable when passing it to any macro
(such as SvREFCNT_inc) and you may or may not have to un-temp it after
doing SvREFCNT_inc. newSVsv would be a more foolproof approach.

AFAIK, to get an SV returned from a call_* to live past the FREETMPS, a
SvREFCNT_inc is the solution. But, I thought of another situation, what
if a DESTROY in a blessed SV being freeded in FREETMPS die()s? The SV*
that got a ++ to live past the FREETMPS leaked. The newSVsv solution
also leaked.

I don't understand what point you're trying to make.
If the temp SV had its refcount increased, then its destructor won't
get called during FREETMPS. If its refcount wasn't increased, then during
freetmps, it's popped of the tmps stack, its refcount becomes zero, it is
freed, and its destructor is called. In neither scenario do I see a leak.

--
That he said that that that that is is is debatable, is debatable.

@p5pRT
Copy link
Author

p5pRT commented Dec 31, 2013

From @cpansprout

On Mon Dec 30 11​:30​:06 2013, bulk88 wrote​:

But, I thought of another situation,
what if a DESTROY in a blessed SV being freeded in FREETMPS die()s?
The SV* that got a ++ to live past the FREETMPS leaked. The newSVsv
solution also leaked.

That’s not a problem, because errors from DESTROY are always caught.

--

Father Chrysostomos

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

No branches or pull requests

2 participants