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

[patch] wrong examples in perlsub/"Constant Functions" #7183

Closed
p5pRT opened this issue Mar 19, 2004 · 6 comments
Closed

[patch] wrong examples in perlsub/"Constant Functions" #7183

p5pRT opened this issue Mar 19, 2004 · 6 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 19, 2004

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

Searchable as RT27768$

@p5pRT
Copy link
Author

p5pRT commented Mar 19, 2004

From padre@elte.hu

Created by padre@elte.hu

By testing whether some functions are treated as constants by perl or not,
I found out, that some of the examples are not correct.

I believe that the difference (for example between FOO_SET and foo_set in
the patch) is that an expression with a statement modifier is still an
expression, so it can be inlined, but an "if" is a statement, not an
expression. However, since I'm not 100% sure about this one, I left this
note out of the patch, so feel free to add it if you know it for certain.

I checked the functions being inlined or not by redefining them later in
the test code, just as it's written perlsub/"Constant Functions".

The patch follows.

HAND,
  Padre

Inline Patch
--- /usr/share/perl/5.8.3/pod/perlsub.pod	2004-02-15 07:55:04.000000000 +0100
+++ perlsub.pod	2004-03-18 17:05:04.000000000 +0100
@@ -1131,7 +1131,16 @@
     sub FLAG_MASK ()	{ FLAG_FOO | FLAG_BAR }
 
     sub OPT_BAZ ()	{ not (0x1B58 & FLAG_MASK) }
-    sub BAZ_VAL () {
+
+    sub N () { int(OPT_BAZ) / 3 }
+
+    sub FOO_SET () { 1 if FLAG_MASK & FLAG_FOO }
+
+Be aware that these will not be inlined:
+
+    sub foo_set () { if (FLAG_MASK & FLAG_FOO) { 1 } }
+
+    sub baz_val () {
 	if (OPT_BAZ) {
 	    return 23;
 	}
@@ -1140,11 +1149,10 @@
 	}
     }
 
-    sub N () { int(BAZ_VAL) / 3 }
     BEGIN {
 	my $prod = 1;
 	for (1..N) { $prod *= $_ }
-	sub N_FACTORIAL () { $prod }
+	sub n_factorial () { $prod }
     }
 
 If you redefine a subroutine that was eligible for inlining, you'll get
Perl Info

Flags:
    category=docs
    severity=low

Site configuration information for perl v5.8.3:

Configured by Debian Project at Sun Feb 15 17:22:09 EST 2004.

Summary of my perl5 (revision 5.0 version 8 subversion 3) configuration:
  Platform:
    osname=linux, osvers=2.4.22-xfs+ti1211, archname=i386-linux-thread-multi
    uname='linux kosh 2.4.22-xfs+ti1211 #1 sat oct 25 10:11:37 est 2003 i686 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i386-linux -Dprefix=/usr -Dprivlib=/usr/share/perl/5.8 -Darchlib=/usr/lib/perl/5.8 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.8.3 -Dsitearch=/usr/local/lib/perl/5.8.3 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Uusesfio -Uusenm -Duseshrplib -Dlibperl=libperl.so.5.8.3 -Dd_dosuid -des'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=define use5005threads=undef useithreads=define usemultiplicity=define
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O3',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -I/usr/local/include'
    ccversion='', gccversion='3.3.3 20040125 (prerelease) (Debian)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=/lib/libc-2.3.2.so, so=so, useshrplib=true, libperl=libperl.so.5.8.3
    gnulibc_version='2.3.2'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic'
    cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.8.3:
    /etc/perl
    /usr/local/lib/perl/5.8.3
    /usr/local/share/perl/5.8.3
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.8
    /usr/share/perl/5.8
    /usr/local/lib/site_perl
    /usr/local/lib/perl/5.8.0
    /usr/local/share/perl/5.8.0
    .


Environment for perl v5.8.3:
    HOME=/home/padre
    LANG=en_US
    LANGUAGE (unset)
    LC_CTYPE=hu_HU
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/padre/Nokia/Update_Manager/bin:/usr/local/java/j2sdk1.4.1_05/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games:/home/padre/bin:.:/home/padre/suli/fun/clean/bin:/usr/local/apps/MozillaFirebird/:/home/oracle/OraHome/bin:/home/padre/bin:.:/usr/local/java/Nokia/Tools/Nokia_Developers_Suite_for_J2ME/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Mar 22, 2004

From @rgs

padre@​elte.hu (via RT) wrote​:

By testing whether some functions are treated as constants by perl or not,
I found out, that some of the examples are not correct.

I believe that the difference (for example between FOO_SET and foo_set in
the patch) is that an expression with a statement modifier is still an
expression, so it can be inlined, but an "if" is a statement, not an
expression. However, since I'm not 100% sure about this one, I left this
note out of the patch, so feel free to add it if you know it for certain.

That's correct (and I added the note.)
I've applied your patch as change #22557 to the development version
of perl, thanks.

--- /usr/share/perl/5.8.3/pod/perlsub.pod 2004-02-15 07​:55​:04.000000000 +0100
+++ perlsub.pod 2004-03-18 17​:05​:04.000000000 +0100

@​@​ -1140,11 +1149,10 @​@​
}
}

- sub N () { int(BAZ_VAL) / 3 }
BEGIN {
my $prod = 1;
for (1..N) { $prod *= $_ }
- sub N_FACTORIAL () { $prod }
+ sub n_factorial () { $prod }
}

...although I think this last example *should* be inlined, so I'm
tempted to say that the non-inlining of it is a bug. I hereby completely
removed it (for now) from perlsub.

@p5pRT
Copy link
Author

p5pRT commented Mar 22, 2004

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

@p5pRT
Copy link
Author

p5pRT commented Mar 22, 2004

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

@p5pRT p5pRT closed this as completed Mar 22, 2004
@p5pRT
Copy link
Author

p5pRT commented Mar 23, 2004

From perl5-porters@ton.iguana.be

In article <20040322220927.1247aa9b.rgarciasuarez@​_ree._r>,
  Rafael Garcia-Suarez <rgarciasuarez@​free.fr> writes​:

@​@​ -1140,11 +1149,10 @​@​
}
}

- sub N () { int(BAZ_VAL) / 3 }
BEGIN {
my $prod = 1;
for (1..N) { $prod *= $_ }
- sub N_FACTORIAL () { $prod }
+ sub n_factorial () { $prod }
}

...although I think this last example *should* be inlined, so I'm
tempted to say that the non-inlining of it is a bug. I hereby completely
removed it (for now) from perlsub.
Current perl behaviour is proper I think. It's a closure, and at the
moment sub n_factorial is defined $prod is actually still undef. It only
later when the body of the BEGIN runs gets its final value.

Though it might be nice to update the docs with a way to actually get
the wanted effect, e.g.​:

#! /usr/bin/perl -wl
BEGIN {
  my $prod = 1;
  for (1..5) { $prod *= $_ }
  require constant;
  constant->import(n_factorial => $prod);
}
print n_factorial();

@p5pRT
Copy link
Author

p5pRT commented Mar 23, 2004

From @rgs

Ton Hospel wrote in perl.perl5.porters :

Current perl behaviour is proper I think. It's a closure, and at the
moment sub n_factorial is defined $prod is actually still undef. It only
later when the body of the BEGIN runs gets its final value.

Though it might be nice to update the docs with a way to actually get
the wanted effect, e.g.​:

#! /usr/bin/perl -wl
BEGIN {
my $prod = 1;
for (1..5) { $prod *= $_ }
require constant;
constant->import(n_factorial => $prod);
}
print n_factorial();

Doesn't this belong to the constant manpage instead ?

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

1 participant