I recently stumbled over a case where a given ... when construct gave me
false positives.
Here is the test case:
use strict;
use warnings;
use 5.010;
use Test::More tests => 2;
for my $thing ('3', ' 3') {
my $test = sprintf '%d', $thing;
my $result = '';
given ($thing) {
when ('3') { $result .= "[3]"; continue; }
when (' 3') { $result .= "[ 3]"; continue; }
}
is ($result, "[$thing]", "Test [$thing]");
}
The test fails because '3' and ' 3' smart-match each other.
According to the documentation, this is expected, because $thing (i.e. the
left side of the smart match) is a number and both '3' and ' 3' (the
right-hand side) can be converted to a number, therefore it uses a numeric
comparison. So far so good...
However, the fact that $thing is a number only comes about because I used it
in a completely unrelated sprintf before. If I comment out the sprintf-line,
then the test suddenly is ok.
I consider it very strange that the a decision whether or not to use a
numeric comparison based on whether or not I use sprintf before (and sprintf
doesn't modify $thing, it only modifies $test).
I understand that it is very difficult (or even impossible) to determine
whether the variable on left-hand side of the smart-match ($thing) is a
number or a text,
But for the right-hand side of the smart-match, I would suggest that if the
right-hand side is a literal ( --- not a variable! --- ) text or number,
then it should be trivial to determine whether the type is text or number,
and the resulting smart-match should execute the comparison based on that
type of the right-hand side.
Does this make sense ?
Implementing this change would require changes in the documentation as well
as in perl itself. -- Unfortunately I haven't got the skills to make changes
in perl itself, I could only update the documentation, if that is what I am
asked to do.
*******************************
C:\>perl -V
Summary of my perl5 (revision 5 version 10 subversion 1) configuration:
Platform:
osname=MSWin32, osvers=5.2, archname=MSWin32-x64-multi-thread
uname=''
config_args='undef'
hint=recommended, useposix=true, d_sigaction=undef
useithreads=define, usemultiplicity=define
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=define, use64bitall=undef, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cl', ccflags ='-nologo -GF -W3 -MD -Zi -DNDEBUG -Ox -GL -Wp64
-fp:precise -DWIN32 -D_CONSOLE -DNO_STRICT -DHAVE_DES_FCRYPT -DWIN64 -
DCONSERVATIVE -DUSE_SITECUSTOMIZE -DPRIVLIB_LAST_IN_INC
-DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO
-DPERL_MSVCRT_READFIX',
optimize='-MD -Zi -DNDEBUG -Ox -GL -Wp64 -fp:precise',
cppflags='-DWIN32'
ccversion='14.00.40310.41', gccversion='', gccosandvers=''
intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678
d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8
ivtype='__int64', ivsize=8, nvtype='double', nvsize=8, Off_t='__int64',
lseeksize=8
alignbytes=8, prototype=define
Linker and Libraries:
ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -ltcg
-libpath:"C:\Perl64\lib\CORE" -machine:AMD64'
libpth=\lib
libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi
32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib
odbccp32.lib bufferoverflowU.lib msvcrt.lib
perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib ne
tapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib
odbccp32.lib bufferoverflowU.lib msvcrt.lib
libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl510.lib
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug
-opt:ref,icf -ltcg -libpath:"C:\Perl64\lib\CORE" -machine:AMD64'
Characteristics of this binary (from libperl):
Compile-time options: MULTIPLICITY PERL_DONT_CREATE_GVSV
PERL_IMPLICIT_CONTEXT PERL_IMPLICIT_SYS
PERL_MALLOC_WRAP PL_OP_SLAB_ALLOC USE_64_BIT_INT
USE_ITHREADS USE_LARGE_FILES USE_PERLIO
USE_SITECUSTOMIZE
Locally applied patches:
ActivePerl Build 1006 [291086]
32728 64-bit fix for Time::Local
Built under MSWin32
Compiled at Aug 24 2009 13:45:20
@INC:
C:/Perl64/site/lib
C:/Perl64/lib
.
Yes. The sprintf converts $thing to an integer, making it a dual-type variable.
You have the same kind of bug with the bitwise operators like &, that
can operate on strings or on integers, when the argument is both.
> I consider it very strange that the a decision whether or not to use a
> numeric comparison based on whether or not I use sprintf before (and sprintf
> doesn't modify $thing, it only modifies $test).
>
> I understand that it is very difficult (or even impossible) to determine
> whether the variable on left-hand side of the smart-match ($thing) is a
> number or a text,
>
> But for the right-hand side of the smart-match, I would suggest that if the
> right-hand side is a literal ( --- not a variable! --- ) text or number,
> then it should be trivial to determine whether the type is text or number,
> and the resulting smart-match should execute the comparison based on that
> type of the right-hand side.
>
> Does this make sense ?
This makes sense, although that won't be totally trivial to implement.
I think the optree should be flagged with info about the const-ness of
RHS.
I agree, but if we sacrifice some backwards compatibility, I believe
we can improve things overall, without flagging the const-ness in the
optree.
This is the existing rule ( taken from
http://perldoc.perl.org/perlsyn.html#Smart-matching-in-detail )
29. Any Num numeric equality $a == $b
30. Num numish[4] numeric equality $a == $b
31. undef Any undefined !defined($b)
32. Any Any string equality $a eq $b
Here is my proposal to change the rules -- in rule 29. replace the RHS
"Num" by a RHS "non-dual-type-Num" and remove rule 30. completely.
So the rules will be:
29. Any non-dual-type-Num numeric equality $a == $b
30. # to be removed
31. undef Any undefined !defined($b)
32. Any Any string equality $a eq $b
My idea is based on the assumption that constants can not be
dual-typed. On the other hand, we break existing smart matching where
the RHS is a numeric variable:
use strict;
use warnings;
use 5.010;
use Test::More tests => 1;
my $var = 3; # here, $var is a non-dual-type number
my $test = sprintf '%s', $var; # now, $var is dual-type !!!
my $result = '';
given (' 3') {
when ($var) { $result .= 'match'; }
}
is ($result, 'match', 'Test match');
This test succeeds with the existing rules, but under my new proposed
rules, the test will fail.
I think that with the benefits of a "given...when..." construct that
works as expected, we gain more than we lose. And all this without
flagging the const-ness in the optree.
You're begging the question. It works as expected if everyone expects that,
which I doubt they will. The simplest case is something like:
when ($config->{max}) { ... }
If you've loaded $config from a file, all its numeric values are PVIV at best,
and PV if they haven't been considered numerically yet. Do we really want
the average Perl programmer to add the inevitable debugging-print statements
and wonder "gosh, given is 5 and $config->{max} is 5 but they don't match,
what's up?"
I don't.
--
rjbs
> My idea is based on the assumption that constants can not be
> dual-typed.
Bad assumption.
use Devel::Peek;
for (1..2) {
for ('5') {
Dump $_;
$_+0;
}
}
SV = PV(0x236424) at 0x182a2bc
REFCNT = 2
FLAGS = (PADTMP,POK,READONLY,pPOK)
PV = 0x182edc4 "5"\0
CUR = 1
LEN = 4
SV = PVIV(0x182005c) at 0x182a2bc
REFCNT = 2
FLAGS = (PADTMP,IOK,POK,READONLY,pIOK,pPOK)
IV = 5
PV = 0x182edc4 "5"\0
CUR = 1
LEN = 4
On Sat, Nov 28, 2009 at 9:06 AM, Ricardo Signes
<perl...@rjbs.manxome.org>wrote:
> Do we really want
> the average Perl programmer to add the inevitable debugging-print
> statements
> and wonder "gosh, given is 5 and $config->{max} is 5 but they don't match,
> what's up?"
>
I agree. Perl's design promotes and encurages the interchangeably of IV 5
and PV 5. Don't make this another source of headaches like the bitwise ops.
I agree, but if we sacrifice some backwards compatibility, I believe we
can improve things overall, without flagging the const-ness in the optree.
This is the existing rule ( taken from
http://perldoc.perl.org/perlsyn.html#Smart-matching-in-detail )
29. Any Num numeric equality $a == $b
30. Num numish[4] numeric equality $a == $b
31. undef Any undefined !defined($b)
32. Any Any string equality $a eq $b
Here is my proposal to change the rules -- in rule 29. replace the RHS
"Num" by a RHS "non-dualtype Num" and remove rule 30. completely.
So the rules will be:
29. Any non-dualtype Num numeric equality $a == $b
30. # to be removed
31. undef Any undefined !defined($b)
32. Any Any string equality $a eq $b
My idea is based on the assumption that constants can not be dual-typed.
On the other hand, we break existing smart matching where the RHS is a
numeric variable:
use strict;
use warnings;
use 5.010;
use Test::More tests => 1;
my $var = 3; # here, $var is a non-dual-type number
my $test = sprintf '%s', $var; # now, $var is dual-type !!!
my $result = '';
given (' 3') {
when ($var) { $result .= 'match'; }
}
is ($result, 'match', 'Test match');
This test succeeds with the existing rules, but under my new proposed
rules, the test will fail.
I think that with the benefits of a "given...when..." construct that
Me neither.
I agree with Ricardo Signes, I haven't thought of the implications. I
herewith withdraw my proposal "without flagging the const-ness".
I think the direction which Rafael Garcia-Suarez pointed to (the optree
should be flagged with info about the const-ness of RHS, although that
won't be totally trivial to implement) would be the way to go.
But as I mentioned earlier, I haven't got the skills to make the
changes, so I can only speak as a Perl user:
If there is anybody out there who has got the skills, the time and the
desire to improve "given...when", then he/she has definitely got the
"+1" from me.
Can I propose a new lexically scoped "use feature", for example "use
feature new_smartmatch" that implements my proposal to change the
rules -- in rule 29. replace the RHS "Num" by a RHS "non-dualtype Num"
and remove rule 30. completely.
With "use feature new_smartmatch", we will be able to safely
smartmatch string conditions, even strings that look like a number,
such as:
$value ~~ ['3', ' 3', '3 '] ...
given ($value) { when (' 3') ...
etc...
For those who want the existing smartmatch rules, they don't "use
feature new_smartmatch" anyway, so nothing changes for them.
Thanks, your solution is perfect. I couldn't see the forest for the
trees :-)