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

Smart match goes wrong with array slice - precedence problem? #10580

Closed
p5pRT opened this issue Aug 27, 2010 · 6 comments
Closed

Smart match goes wrong with array slice - precedence problem? #10580

p5pRT opened this issue Aug 27, 2010 · 6 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Aug 27, 2010

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

Searchable as RT77468$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 27, 2010

From @epa

Created by @epa

  use 5.010;
  my @​a = qw(a y0 z);
  my @​b = qw(x y1 z);
  my @​trimmed = @​a[0 .. $#b];
  say "first lhs​: @​a[0 .. $#b]";
  say "second lhs​: @​trimmed";
  say 'first yes' if (@​a[0 .. $#b] ~~ @​b);
  say 'second yes' if (@​trimmed ~~ @​b);

Oddly, this prints

first lhs​: a y0 z
second lhs​: a y0 z
first yes

I expected it not to print 'first yes', since the LHS of the smart
match operation is an array that has ('a', 'y0', 'z'), which is
different from the RHS array ('a', 'y1', 'z').

Or at the very least, I expected the 'first yes' and 'second yes'
to either both appear or neither appear, since the expressions
are equivalent.

If I put brackets round the 'first yes' line, as

  say 'first yes' if ((@​a[0 .. $#b]) ~~ @​b);

then the result is as I expect - but I can't see why the precedence
isn't correct as is. If this is 'not a bug', then perhaps a warning
for this ambiguous construct is merited.

Perl Info

Flags:
    category=core
    severity=high

This perlbug was built using Perl 5.10.0 in the Fedora build system.
It is being executed now by Perl 5.10.0 - Tue Dec  1 10:20:45 EST 2009.

Site configuration information for perl 5.10.0:

Configured by Red Hat, Inc. at Tue Dec  1 10:20:45 EST 2009.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
  Platform:
    osname=linux, osvers=2.6.18-164.2.1.el5, archname=x86_64-linux-thread-multi
    uname='linux x86-5.fedora.phx.redhat.com 2.6.18-164.2.1.el5 #1 smp mon sep 21 04:37:42 edt 2009 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Accflags=-DPERL_USE_SAFE_PUTENV -Dversion=5.10.0 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dprivlib=/usr/lib/perl5/5.10.0 -Dsitelib=/usr/local/lib/perl5/site_perl/5.10.0 -Dvendorlib=/usr/lib/perl5/vendor_perl/5.10.0 -Darchlib=/usr/lib64/perl5/5.10.0/x86_64-linux-thread-multi -Dsitearch=/usr/local/lib64/perl5/site_perl/5.10.0/x86_64-linux-thread-multi -Dvendorarch=/usr/lib64/perl5/vendor_perl/5.10.0/x86_64-linux-thread-multi -Dinc_version_list=none -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dotherlibdirs=/usr/lib/perl5/site_perl'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DPERL_USE_SAFE_PUTENV -DDEBUGGING -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
    optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DPERL_USE_SAFE_PUTENV -DDEBUGGING -fno-strict-aliasing -pipe -I/usr/local/include -I/usr/include/gdbm'
    ccversion='', gccversion='4.4.2 20091027 (Red Hat 4.4.2-7)', 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='gcc', ldflags =''
    libpth=/usr/local/lib64 /lib64 /usr/lib64
    libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.11'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/lib64/perl5/5.10.0/x86_64-linux-thread-multi/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic'

Locally applied patches:
    


@INC for perl 5.10.0:
    /home/eda/lib/perl5/5.10.0
    /home/eda/lib/perl5/site_perl/5.10.0
    /usr/local/lib64/perl5/site_perl/5.10.0/x86_64-linux-thread-multi
    /usr/local/lib/perl5/site_perl/5.10.0
    /usr/lib64/perl5/vendor_perl/5.10.0/x86_64-linux-thread-multi
    /usr/lib/perl5/vendor_perl/5.10.0
    /usr/lib/perl5/vendor_perl
    /usr/lib64/perl5/5.10.0/x86_64-linux-thread-multi
    /usr/lib/perl5/5.10.0
    /usr/lib/perl5/site_perl/5.10.0
    /usr/lib/perl5/site_perl
    .


Environment for perl 5.10.0:
    HOME=/home/eda
    LANG=en_GB.UTF-8
    LANGUAGE (unset)
    LC_COLLATE=C
    LC_CTYPE=en_GB.UTF-8
    LC_MESSAGES=en_GB.UTF-8
    LC_MONETARY=en_GB.UTF-8
    LC_NUMERIC=en_GB.UTF-8
    LC_TIME=en_GB.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/eda/bin:/home/eda/bin:/home/eda/bin:/usr/lib64/qt-3.3/bin:/usr/kerberos/sbin:/usr/kerberos/bin:/usr/lib64/ccache:/usr/local/bin:/bin:/usr/bin:/sbin:/usr/sbin:/sbin:/usr/sbin:/sbin:/usr/sbin
    PERL5LIB=/home/eda/lib/perl5/5.10.0:/home/eda/lib/perl5/site_perl/5.10.0
    PERL_BADLANG (unset)
    SHELL=/bin/bash

-- 
Ed Avis <eda@waniasset.com>

______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email 
______________________________________________________________________

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 28, 2010

From @dgl

This reminded me of some odd behaviour I had never had time to investigate.

It's not really a precedence problem, the suggested workaround of adding parens
doesn't exactly work​:

  (1,2,3) ~~ [1..3] # true
  (3) ~~ [1..3] # true
  (3,3,1) ~~ [1..3] # false

This is because this compares the last item of the LHS with the array and 3 ~~
[1..3] is true.

Looking at how smartmatch works underneath it actually takes a reference to the
array at op generation time (by calling ref_array_or_hash in op.c, B​::Deparse
will show you). This function doesn't seem to take array slices into account.

Patch attached. Although I'm not sure I like the approach of teaching
ref_array_or_hash to return an anonlist for slices, although it does seem to
work.

David

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 28, 2010

From @dgl

0001-Fix-RT-77468-Smart-matching-on-slices.patch
From bd4ff422615858ef8c733f2bdfbd942d00feabad Mon Sep 17 00:00:00 2001
From: David Leadbeater <dgl@dgl.cx>
Date: Sat, 28 Aug 2010 22:39:58 +0100
Subject: [PATCH] Fix RT #77468: Smart matching on slices

ref_array_or_hash did not take aslice or hslice OPs into account; wrap
them in an anonlist so that smart matching has a reference as it
expects.
---
 op.c              |   12 ++++++++++++
 t/op/smartmatch.t |   26 +++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/op.c b/op.c
index 433face..a0fae09 100644
--- a/op.c
+++ b/op.c
@@ -5543,6 +5543,18 @@ S_ref_array_or_hash(pTHX_ OP *cond)
 	return newUNOP(OP_REFGEN,
 	    0, mod(cond, OP_REFGEN));
 
+    else if(cond
+    && (cond->op_type == OP_ASLICE
+    ||  cond->op_type == OP_HSLICE)) {
+
+	/* anonlist now needs a list from this op, was previously used in
+	 * scalar context */
+	cond->op_flags |= ~(OPf_WANT_SCALAR | OPf_REF);
+	cond->op_flags |= OPf_WANT_LIST;
+
+	return newANONLIST(mod(cond, OP_ANONLIST));
+    }
+
     else
 	return cond;
 }
diff --git a/t/op/smartmatch.t b/t/op/smartmatch.t
index f8a073b..f14e91c 100644
--- a/t/op/smartmatch.t
+++ b/t/op/smartmatch.t
@@ -73,7 +73,7 @@ my %keyandmore = map { $_ => 0 } @keyandmore;
 my %fooormore = map { $_ => 0 } @fooormore;
 
 # Load and run the tests
-plan tests => 335;
+plan tests => 351;
 
 while (<DATA>) {
   SKIP: {
@@ -484,6 +484,30 @@ __DATA__
 	@nums		{  1, '', 12, '' }
 !	@nums		{ 11, '', 12, '' }
 
+# array slices
+	@nums[0..-1]	[]
+	@nums[0..0]	[1]
+!	@nums[0..1]	[0..2]
+	@nums[0..4]	[1..5]
+
+!	undef		@nums[0..-1]
+	1		@nums[0..0]
+	2		@nums[0..1]
+!	@nums[0..1]	2
+
+	@nums[0..1]	@nums[0..1]
+
+# hash slices
+	@keyandmore{qw(not)}		[undef]
+	@keyandmore{qw(key)}		[0]
+
+	undef				@keyandmore{qw(not)}
+	0				@keyandmore{qw(key and more)}
+!	2				@keyandmore{qw(key and)}
+
+	@fooormore{qw(foo)}		@keyandmore{qw(key)}
+	@fooormore{qw(foo or more)}	@keyandmore{qw(key and more)}
+
 # UNDEF
 !	3		undef
 !	1		undef
-- 
1.6.3.3

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 28, 2010

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 7, 2010

From @rgarcia

On 28 August 2010 23​:49, David Leadbeater <dgl@​dgl.cx> wrote​:

This reminded me of some odd behaviour I had never had time to investigate.

It's not really a precedence problem, the suggested workaround of adding parens
doesn't exactly work​:

 (1,2,3) ~~ [1..3] # true
 (3) ~~ [1..3] # true
 (3,3,1) ~~ [1..3] # false

This is because this compares the last item of the LHS with the array and 3 ~~
[1..3] is true.

Looking at how smartmatch works underneath it actually takes a reference to the
array at op generation time (by calling ref_array_or_hash in op.c, B​::Deparse
will show you). This function doesn't seem to take array slices into account.

Patch attached. Although I'm not sure I like the approach of teaching
ref_array_or_hash to return an anonlist for slices, although it does seem to
work.

Thanks, applied to bleadperl as 329a333.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 7, 2010

@rgs - 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.