Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

More problems with push/keys $scalar #10896

Closed p5pRT closed 13 years ago

p5pRT commented 13 years ago

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

Searchable as RT80626$

p5pRT commented 13 years ago

From @cpansprout

$ perl5.13.7 -le 'push 42\, 1\,2\,3; print "@​42"' Type of arg 1 to push must be array (not constant item) at -e line 1\, near "3;" Execution of -e aborted due to compilation errors.

$ perl5.13.7 -le 'push ${\42}\, 1\,2\,3; print "@​42"' 1 2 3

So\, with strict mode off\, symbolic deferencing is allowed unless it is detected at compile-time.

Let’s try something else​:

$ perl5.13.7 -le 'use strict; push *f\, 1\,2\,3; print "@​​::f"' 1 2 3

$ perl5.13.7 -le 'use strict; use constant g=>*f; push g\, 1\,2\,3; print "@​​::f"' Type of arg 1 to push must be array (not constant item) at -e line 1\, near "3;" Execution of -e aborted due to compilation errors.

So push is sometimes stricter than strict.

The edge cases for keys are obviously even more weird​:

$ perl5.13.7 -le '%foo = qw(a 1 b 1 c 1); $\,=" ";print keys foo =>' Hash %foo missing the % in argument 1 of keys() at -e line 1. c a b

$ perl5.13.7 -le '%foo = qw(a 1 b 1 c 1); $\,=" ";print keys "foo"' Type of argument to keys on reference must be hashref or arrayref at -e line 1.

I thought foo => and "foo" were equivalent.

$ perl5.13.7 -le '%foo = qw(a 1 b 1 c 1); $\,=" ";print keys *foo' Type of argument to keys on reference must be hashref or arrayref at -e line 1.

But *foo is a perfectly valid array/hash reference (even under strict). Why cannot it be resolved like a blessed scalar with @​{} and %{} overloading?

More inconsistencies​:

$ perl5.13.7 -le 'keys %$x = 7' $ perl5.13.7 -le 'keys $x = 7' Can't modify keys on reference in scalar assignment at -e line 1\, near "7;" Execution of -e aborted due to compilation errors. $ perl5.13.7 -le 'keys $x; print $x//"\"' \ $ perl5.13.7 -le 'push $x; print $x//"\"' ARRAY(0x803a00)

Also\, to bring this up again\, what if I have an object that is a blessed array (I do) the contents of which are part of the public API (they are) and the blessing exists for the sake of calling methods on it? What if I want to add %{} overloading to it as an alternate interface? In that case\, I am not changing the API (even by David Golden’s standards)\, as ref() returns the same thing. But that will break any code doing keys $object.

So\, could we please make this simpler? This set of rules for dealing with constants and overloading does not match anything elsewhere in Perl. So it is just another set of edge cases for people to learn.

Oh look\, there is a patch attached hereto.

(This also eliminates the undocumented ‘keys undef’ behaviour and makes keys $foo consistent with keys %$foo in terms of autovivification.)


Flags​:   category=core   severity=high


Site configuration information for perl 5.13.7​:

Configured by sprout at Thu Dec 9 14​:53​:58 PST 2010.

Summary of my perl5 (revision 5 version 13 subversion 7) configuration​:   Snapshot of​: 9e9fdd5de1676d607ddf9b9f10b8df2659af3ded   Platform​:   osname=darwin\, osvers=10.4.0\, archname=darwin-2level   uname='darwin pint.local 10.4.0 darwin kernel version 10.4.0​: fri apr 23 18​:28​:53 pdt 2010; root​:xnu-1504.7.4~1release_i386 i386 '   config_args='-de -Dusedevel -DDEBUGGING'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=undef\, usemultiplicity=undef   useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef   use64bitint=undef\, use64bitall=undef\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='cc'\, ccflags ='-fno-common -DPERL_DARWIN -no-cpp-precomp -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'\,   optimize='-O3 -g'\,   cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'   ccversion=''\, gccversion='4.2.1 (Apple Inc. build 5664)'\, gccosandvers=''   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   ivtype='long'\, ivsize=4\, 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/usr/local/lib'   libpth=/usr/local/lib /usr/lib   libs=-ldbm -ldl -lm -lutil -lc   perllibs=-ldl -lm -lutil -lc   libc=/usr/lib/libc.dylib\, 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/usr/local/lib -fstack-protector'

Locally applied patches​:  


@​INC for perl 5.13.7​:   /usr/local/lib/perl5/site_perl/5.13.7/darwin-2level   /usr/local/lib/perl5/site_perl/5.13.7   /usr/local/lib/perl5/5.13.7/darwin-2level   /usr/local/lib/perl5/5.13.7   /usr/local/lib/perl5/site_perl   .


Environment for perl 5.13.7​:   DYLD_LIBRARY_PATH (unset)   HOME=/Users/sprout   LANG=en_US.UTF-8   LANGUAGE (unset)   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin   PERL_BADLANG (unset)   SHELL=/bin/bash

p5pRT commented 13 years ago

From @cpansprout

Inline Patch ```diff diff --git a/doop.c b/doop.c index 550e6fb..35efba6 100644 --- a/doop.c +++ b/doop.c @@ -1436,9 +1436,8 @@ Perl_do_kv(pTHX) register HE *entry; const I32 gimme = GIMME_V; const I32 dokv = (PL_op->op_type == OP_RV2HV || PL_op->op_type == OP_PADHV); - /* op_type is OP_RKEYS/OP_RVALUES if pp_rkeys delegated to here */ - const I32 dokeys = dokv || (PL_op->op_type == OP_KEYS || PL_op->op_type == OP_RKEYS); - const I32 dovalues = dokv || (PL_op->op_type == OP_VALUES || PL_op->op_type == OP_RVALUES); + const I32 dokeys = dokv || (PL_op->op_type == OP_KEYS); + const I32 dovalues = dokv || (PL_op->op_type == OP_VALUES); if (!hv) { if (PL_op->op_flags & OPf_MOD || LVRET) { /* lvalue */ diff --git a/embed.h b/embed.h index d484a10..e4b83ea 100644 --- a/embed.h +++ b/embed.h @@ -939,7 +939,6 @@ #define ck_method(a) Perl_ck_method(aTHX_ a) #define ck_null(a) Perl_ck_null(aTHX_ a) #define ck_open(a) Perl_ck_open(aTHX_ a) -#define ck_push(a) Perl_ck_push(aTHX_ a) #define ck_readline(a) Perl_ck_readline(aTHX_ a) #define ck_repeat(a) Perl_ck_repeat(aTHX_ a) #define ck_require(a) Perl_ck_require(aTHX_ a) @@ -1334,7 +1333,6 @@ #define pp_rand() Perl_pp_rand(aTHX) #define pp_range() Perl_pp_range(aTHX) #define pp_rcatline() Perl_pp_rcatline(aTHX) -#define pp_reach() Perl_pp_reach(aTHX) #define pp_read() Perl_pp_read(aTHX) #define pp_readdir() Perl_pp_readdir(aTHX) #define pp_readline() Perl_pp_readline(aTHX) @@ -1355,14 +1353,12 @@ #define pp_rewinddir() Perl_pp_rewinddir(aTHX) #define pp_right_shift() Perl_pp_right_shift(aTHX) #define pp_rindex() Perl_pp_rindex(aTHX) -#define pp_rkeys() Perl_pp_rkeys(aTHX) #define pp_rmdir() Perl_pp_rmdir(aTHX) #define pp_rv2av() Perl_pp_rv2av(aTHX) #define pp_rv2cv() Perl_pp_rv2cv(aTHX) #define pp_rv2gv() Perl_pp_rv2gv(aTHX) #define pp_rv2hv() Perl_pp_rv2hv(aTHX) #define pp_rv2sv() Perl_pp_rv2sv(aTHX) -#define pp_rvalues() Perl_pp_rvalues(aTHX) #define pp_sassign() Perl_pp_sassign(aTHX) #define pp_say() Perl_pp_say(aTHX) #define pp_scalar() Perl_pp_scalar(aTHX) diff --git a/ext/Opcode/Opcode.pm b/ext/Opcode/Opcode.pm index 0766940..a9e746f 100644 --- a/ext/Opcode/Opcode.pm +++ b/ext/Opcode/Opcode.pm @@ -311,7 +311,7 @@ invert_opset function. rv2av aassign aelem aelemfast aslice av2arylen rv2hv helem hslice each values keys exists delete aeach akeys avalues - boolkeys reach rvalues rkeys + boolkeys preinc i_preinc predec i_predec postinc i_postinc postdec i_postdec int hex oct abs pow multiply i_multiply divide i_divide diff --git a/op.c b/op.c index 4c3c876..ea75d0e 100644 --- a/op.c +++ b/op.c @@ -310,12 +310,6 @@ Perl_Slab_Free(pTHX_ void *op) #define RETURN_UNLIMITED_NUMBER (PERL_INT_MAX / 2) -#define CHANGE_TYPE(o,type) \ - STMT_START { \ - o->op_type = (OPCODE)type; \ - o->op_ppaddr = PL_ppaddr[type]; \ - } STMT_END - STATIC const char* S_gv_ename(pTHX_ GV *gv) { @@ -7413,47 +7407,67 @@ Perl_ck_fun(pTHX_ OP *o) "Useless use of %s with no values", PL_op_desc[type]); - if (kid->op_type == OP_CONST && - (kid->op_private & OPpCONST_BARE)) - { - OP * const newop = newAVREF(newGVOP(OP_GV, 0, - gv_fetchsv(((SVOP*)kid)->op_sv, GV_ADD, SVt_PVAV) )); - Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED), + if (kid->op_type != OP_RV2AV && kid->op_type != OP_PADAV) { + SV * const kidsv = + kid->op_type == OP_CONST ? ((SVOP*)kid)->op_sv : NULL; + OP * const newsubop = + kidsv && !isGV(kidsv) && !SvROK(kidsv) + ? newGVOP( + OP_GV, 0, gv_fetchsv(kidsv, GV_ADD, SVt_PVAV) + ) + : kid; + OP * const newop = newAVREF(newsubop); + + if (kid->op_type == OP_CONST && + (kid->op_private & OPpCONST_BARE)) + Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED), "Array @%"SVf" missing the @ in argument %"IVdf" of %s()", SVfARG(((SVOP*)kid)->op_sv), (IV)numargs, PL_op_desc[type]); + + if (newsubop == kid) + kid->op_sibling = NULL; + else #ifdef PERL_MAD - op_getmad(kid,newop,'K'); + op_getmad(kid,newop,'K'); #else - op_free(kid); + op_free(kid); #endif kid = newop; kid->op_sibling = sibl; *tokid = kid; } - else if (kid->op_type != OP_RV2AV && kid->op_type != OP_PADAV) - bad_type(numargs, "array", PL_op_desc[type], kid); op_lvalue(kid, type); break; case OA_HVREF: - if (kid->op_type == OP_CONST && - (kid->op_private & OPpCONST_BARE)) - { - OP * const newop = newHVREF(newGVOP(OP_GV, 0, - gv_fetchsv(((SVOP*)kid)->op_sv, GV_ADD, SVt_PVHV) )); - Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED), + if (kid->op_type != OP_RV2HV && kid->op_type != OP_PADHV) { + SV * const kidsv = + kid->op_type == OP_CONST ? ((SVOP*)kid)->op_sv : NULL; + OP * const newsubop = + kidsv && !isGV(kidsv) && !SvROK(kidsv) + ? newGVOP( + OP_GV, 0, gv_fetchsv(kidsv, GV_ADD, SVt_PVHV) + ) + : kid; + OP * const newop = newHVREF(newsubop); + + if (kid->op_type == OP_CONST && + (kid->op_private & OPpCONST_BARE)) + Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED), "Hash %%%"SVf" missing the %% in argument %"IVdf" of %s()", SVfARG(((SVOP*)kid)->op_sv), (IV)numargs, PL_op_desc[type]); + + if (newsubop == kid) + assert(!kid->op_sibling); + else #ifdef PERL_MAD - op_getmad(kid,newop,'K'); + op_getmad(kid,newop,'K'); #else - op_free(kid); + op_free(kid); #endif kid = newop; kid->op_sibling = sibl; *tokid = kid; } - else if (kid->op_type != OP_RV2HV && kid->op_type != OP_PADHV) - bad_type(numargs, "hash", PL_op_desc[type], kid); op_lvalue(kid, type); break; case OA_CVREF: @@ -8256,7 +8270,7 @@ Perl_ck_shift(pTHX_ OP *o) return newUNOP(type, 0, scalar(argop)); #endif } - return scalar(modkids(ck_push(o), type)); + return scalar(modkids(ck_fun(o), type)); } OP * @@ -9122,48 +9136,6 @@ Perl_ck_substr(pTHX_ OP *o) } OP * -Perl_ck_push(pTHX_ OP *o) -{ - dVAR; - OP *kid = o->op_flags & OPf_KIDS ? cLISTOPo->op_first : NULL; - OP *cursor = NULL; - OP *proxy = NULL; - - PERL_ARGS_ASSERT_CK_PUSH; - - /* If 1st kid is pushmark (e.g. push, unshift, splice), we need 2nd kid */ - if (kid) { - cursor = kid->op_type == OP_PUSHMARK ? kid->op_sibling : kid; - } - - /* If not array or array deref, wrap it with an array deref. - * For OP_CONST, we only wrap arrayrefs */ - if (cursor) { - if ( ( cursor->op_type != OP_PADAV - && cursor->op_type != OP_RV2AV - && cursor->op_type != OP_CONST - ) - || - ( cursor->op_type == OP_CONST - && SvROK(cSVOPx_sv(cursor)) - && SvTYPE(SvRV(cSVOPx_sv(cursor))) == SVt_PVAV - ) - ) { - proxy = newAVREF(cursor); - if ( cursor == kid ) { - cLISTOPx(o)->op_first = proxy; - } - else { - cLISTOPx(kid)->op_sibling = proxy; - } - cLISTOPx(proxy)->op_sibling = cLISTOPx(cursor)->op_sibling; - cLISTOPx(cursor)->op_sibling = NULL; - } - } - return ck_fun(o); -} - -OP * Perl_ck_each(pTHX_ OP *o) { dVAR; @@ -9171,30 +9143,16 @@ Perl_ck_each(pTHX_ OP *o) const unsigned orig_type = o->op_type; const unsigned array_type = orig_type == OP_EACH ? OP_AEACH : orig_type == OP_KEYS ? OP_AKEYS : OP_AVALUES; - const unsigned ref_type = orig_type == OP_EACH ? OP_REACH - : orig_type == OP_KEYS ? OP_RKEYS : OP_RVALUES; PERL_ARGS_ASSERT_CK_EACH; if (kid) { - switch (kid->op_type) { - case OP_PADHV: - case OP_RV2HV: - break; - case OP_PADAV: - case OP_RV2AV: - CHANGE_TYPE(o, array_type); - break; - case OP_CONST: - if (kid->op_private == OPpCONST_BARE) - /* we let ck_fun treat as hash */ - break; - default: - CHANGE_TYPE(o, ref_type); + if (kid->op_type == OP_PADAV || kid->op_type == OP_RV2AV) { + o->op_type = (OPCODE)array_type; + o->op_ppaddr = PL_ppaddr[array_type]; } } - /* if treating as a reference, defer additional checks to runtime */ - return o->op_type == ref_type ? o : ck_fun(o); + return ck_fun(o); } /* caller is supposed to assign the return to the diff --git a/opcode.h b/opcode.h index 122c67f..71f46c5 100644 --- a/opcode.h +++ b/opcode.h @@ -394,9 +394,6 @@ EXTCONST char* const PL_op_name[] = { "lock", "once", "custom", - "reach", - "rkeys", - "rvalues", "transr", }; #endif @@ -771,9 +768,6 @@ EXTCONST char* const PL_op_desc[] = { "lock", "once", "unknown custom operator", - "each on reference", - "keys on reference", - "values on reference", "transliteration (tr///)", }; #endif @@ -1162,9 +1156,6 @@ EXT Perl_ppaddr_t PL_ppaddr[] /* or perlvars.h */ Perl_pp_lock, Perl_pp_once, Perl_unimplemented_op, /* Perl_pp_custom */ - Perl_pp_rkeys, /* Perl_pp_reach */ - Perl_pp_rkeys, - Perl_pp_rkeys, /* Perl_pp_rvalues */ Perl_pp_trans, /* Perl_pp_transr */ } #endif @@ -1334,11 +1325,11 @@ EXT Perl_check_t PL_check[] /* or perlvars.h */ Perl_ck_null, /* lslice */ Perl_ck_fun, /* anonlist */ Perl_ck_fun, /* anonhash */ - Perl_ck_push, /* splice */ - Perl_ck_push, /* push */ + Perl_ck_fun, /* splice */ + Perl_ck_fun, /* push */ Perl_ck_shift, /* pop */ Perl_ck_shift, /* shift */ - Perl_ck_push, /* unshift */ + Perl_ck_fun, /* unshift */ Perl_ck_sort, /* sort */ Perl_ck_fun, /* reverse */ Perl_ck_grep, /* grepstart */ @@ -1550,9 +1541,6 @@ EXT Perl_check_t PL_check[] /* or perlvars.h */ Perl_ck_rfun, /* lock */ Perl_ck_null, /* once */ Perl_ck_null, /* custom */ - Perl_ck_each, /* reach */ - Perl_ck_each, /* rkeys */ - Perl_ck_each, /* rvalues */ Perl_ck_match, /* transr */ } #endif @@ -1932,9 +1920,6 @@ EXTCONST U32 PL_opargs[] = { 0x00007b04, /* lock */ 0x00000300, /* once */ 0x00000000, /* custom */ - 0x00001b00, /* reach */ - 0x00001b08, /* rkeys */ - 0x00001b08, /* rvalues */ 0x00001804, /* transr */ }; #endif diff --git a/opnames.h b/opnames.h index 609c6e2..86781a3 100644 --- a/opnames.h +++ b/opnames.h @@ -381,14 +381,11 @@ typedef enum opcode { OP_LOCK = 363, OP_ONCE = 364, OP_CUSTOM = 365, - OP_REACH = 366, - OP_RKEYS = 367, - OP_RVALUES = 368, - OP_TRANSR = 369, + OP_TRANSR = 366, OP_max } opcode; -#define MAXO 370 +#define MAXO 367 #define OP_phoney_INPUT_ONLY -1 #define OP_phoney_OUTPUT_ONLY -2 diff --git a/pod/perldiag.pod b/pod/perldiag.pod index 7250057..71fd181 100644 --- a/pod/perldiag.pod +++ b/pod/perldiag.pod @@ -76,17 +76,6 @@ on the operator (e.g. C) or declare the subroutine to be an object method (see L or L). -=item Ambiguous overloaded argument to %s resolved as %s - -(W ambiguous) You called C, C or C on an object that had -overloading of C<%{}> or C<@{}> or both. In such a case, the object is -dereferenced according to its overloading, not its underlying reference type. -The warning is issued when C<%{}> overloading exists on a blessed arrayref, -when C<@{}> overloading exists on a blessed hashref, or when both overloadings -are defined (in which case C<%{}> is used). You can force the interpretation -of the object by explictly dereferencing it as an array or hash instead of -passing the object itself to C, C or C. - =item Ambiguous range in transliteration operator (F) You wrote something like C which doesn't mean anything at diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod index 6a498bc..095c6aa 100644 --- a/pod/perlfunc.pod +++ b/pod/perlfunc.pod @@ -1455,7 +1455,7 @@ typo. =item each HASH (or HASHREF) X X -=item each ARRAY (or ARRAYREF) +=item each ARRAY X When called in list context, returns a 2-element list consisting of the key @@ -1494,16 +1494,11 @@ but in a different order: print "$key=$value\n"; } -When given a reference to a hash or array, the argument will be +When given a reference to a hash, the argument will be dereferenced automatically. while (($key,$value) = each $hashref) { ... } -If the reference is a blessed object that overrides either C<%{}> or -C<@{}>, the override will be used instead of dereferencing the underlying -variable type. If both overrides are provided, C<%{}> will be the default. -If this is not desired, you must dereference the argument yourself. - See also C, C and C. =item eof FILEHANDLE @@ -2615,7 +2610,7 @@ first argument. Compare L. =item keys HASH (or HASHREF) X X -=item keys ARRAY (or ARRAYREF) +=item keys ARRAY Returns a list consisting of all the keys of the named hash, or the indices of an array. (In scalar context, returns the number of keys or indices.) @@ -2672,16 +2667,11 @@ C in this way (but you needn't worry about doing this by accident, as trying has no effect). C in an lvalue context is a syntax error. -When given a reference to a hash or array, the argument will be +When given a reference to a hash, the argument will be dereferenced automatically. for (keys $hashref) { ... } - for (keys $obj->get_arrayref) { ... } - -If the reference is a blessed object that overrides either C<%{}> or -C<@{}>, the override will be used instead of dereferencing the underlying -variable type. If both overrides are provided, C<%{}> will be the default. -If this is not desired, you must dereference the argument yourself. + for (keys $obj->get_fields) { ... } See also C, C and C. @@ -7385,7 +7375,7 @@ recognized; barewords are considered filenames. =item values HASH (or HASHREF) X -=item values ARRAY (or ARRAYREF) +=item values ARRAY Returns a list consisting of all the values of the named hash, or the values of an array. (In a scalar context, returns the number of values.) @@ -7413,16 +7403,11 @@ modify the contents of the hash: for (values %hash) { s/foo/bar/g } # modifies %hash values for (@hash{keys %hash}) { s/foo/bar/g } # same -When given a reference to a hash or array, the argument will be +When given a reference to a hash, the argument will be dereferenced automatically. for (values $hashref) { ... } - for (values $obj->get_arrayref) { ... } - -If the reference is a blessed object that overrides either C<%{}> or -C<@{}>, the override will be used instead of dereferencing the underlying -variable type. If both overrides are provided, C<%{}> will be the default. -If this is not desired, you must dereference the argument yourself. + for (values $obj->get_fields) { ... } See also C, C, and C. diff --git a/pp.c b/pp.c index 47cf756..00cb9c0 100644 --- a/pp.c +++ b/pp.c @@ -4661,71 +4661,6 @@ PP(pp_aslice) RETURN; } -/* Smart dereferencing for keys, values and each */ -PP(pp_rkeys) -{ - dVAR; - dSP; - dPOPss; - - if (!SvOK(sv)) - RETURN; - - if (SvROK(sv)) { - SvGETMAGIC(sv); - if (SvAMAGIC(sv)) { - /* N.B.: AMG macros return sv if no overloading is found */ - SV *maybe_hv = AMG_CALLun(sv,to_hv); - SV *maybe_av = AMG_CALLun(sv,to_av); - if ( maybe_hv != sv && maybe_av != sv ) { - Perl_ck_warner(aTHX_ packWARN(WARN_AMBIGUOUS), "%s", - Perl_form(aTHX_ "Ambiguous overloaded argument to %s resolved as %%{}", - PL_op_desc[PL_op->op_type] - ) - ); - sv = maybe_hv; - } - else if ( maybe_av != sv ) { - if ( SvTYPE(SvRV(sv)) == SVt_PVHV ) { - /* @{} overload, but underlying reftype is HV */ - Perl_ck_warner(aTHX_ packWARN(WARN_AMBIGUOUS), "%s", - Perl_form(aTHX_ "Ambiguous overloaded argument to %s resolved as @{}", - PL_op_desc[PL_op->op_type] - ) - ); - } - sv = maybe_av; - } - else if ( maybe_hv != sv ) { - if ( SvTYPE(SvRV(sv)) == SVt_PVAV ) { - /* %{} overload, but underlying reftype is AV */ - Perl_ck_warner(aTHX_ packWARN(WARN_AMBIGUOUS), "%s", - Perl_form(aTHX_ "Ambiguous overloaded argument to %s resolved as %%{}", - PL_op_desc[PL_op->op_type] - ) - ); - } - sv = maybe_hv; - } - } - sv = SvRV(sv); - } - - if ( SvTYPE(sv) != SVt_PVHV && SvTYPE(sv) != SVt_PVAV ) { - DIE(aTHX_ "Type of argument to %s must be hashref or arrayref", - PL_op_desc[PL_op->op_type] ); - } - - /* Delegate to correct function for op type */ - PUSHs(sv); - if (PL_op->op_type == OP_RKEYS || PL_op->op_type == OP_RVALUES) { - return (SvTYPE(sv) == SVt_PVHV) ? Perl_do_kv(aTHX) : Perl_pp_akeys(aTHX); - } - else { - return (SvTYPE(sv) == SVt_PVHV) ? Perl_pp_each(aTHX) : Perl_pp_aeach(aTHX); - } -} - PP(pp_aeach) { dVAR; @@ -4771,7 +4706,7 @@ PP(pp_akeys) EXTEND(SP, n + 1); - if (PL_op->op_type == OP_AKEYS || PL_op->op_type == OP_RKEYS) { + if (PL_op->op_type == OP_AKEYS) { n += i; for (; i <= n; i++) { mPUSHi(i); diff --git a/pp.sym b/pp.sym index 11e8f78..594a752 100644 --- a/pp.sym +++ b/pp.sym @@ -30,7 +30,6 @@ Perl_ck_match Perl_ck_method Perl_ck_null Perl_ck_open -Perl_ck_push Perl_ck_readline Perl_ck_repeat Perl_ck_require @@ -410,9 +409,6 @@ Perl_pp_getlogin Perl_pp_syscall Perl_pp_lock Perl_pp_once -Perl_pp_reach -Perl_pp_rkeys -Perl_pp_rvalues Perl_pp_transr # ex: set ro: diff --git a/proto.h b/proto.h index a05f2b9..091bf50 100644 --- a/proto.h +++ b/proto.h @@ -424,12 +424,6 @@ PERL_CALLCONV OP * Perl_ck_open(pTHX_ OP *o) #define PERL_ARGS_ASSERT_CK_OPEN \ assert(o) -PERL_CALLCONV OP * Perl_ck_push(pTHX_ OP *o) - __attribute__warn_unused_result__ - __attribute__nonnull__(pTHX_1); -#define PERL_ARGS_ASSERT_CK_PUSH \ - assert(o) - PERL_CALLCONV OP * Perl_ck_readline(pTHX_ OP *o) __attribute__warn_unused_result__ __attribute__nonnull__(pTHX_1); @@ -3117,7 +3111,6 @@ PERL_CALLCONV OP * Perl_pp_quotemeta(pTHX); PERL_CALLCONV OP * Perl_pp_rand(pTHX); PERL_CALLCONV OP * Perl_pp_range(pTHX); PERL_CALLCONV OP * Perl_pp_rcatline(pTHX); -PERL_CALLCONV OP * Perl_pp_reach(pTHX); PERL_CALLCONV OP * Perl_pp_read(pTHX); PERL_CALLCONV OP * Perl_pp_readdir(pTHX); PERL_CALLCONV OP * Perl_pp_readline(pTHX); @@ -3138,14 +3131,12 @@ PERL_CALLCONV OP * Perl_pp_reverse(pTHX); PERL_CALLCONV OP * Perl_pp_rewinddir(pTHX); PERL_CALLCONV OP * Perl_pp_right_shift(pTHX); PERL_CALLCONV OP * Perl_pp_rindex(pTHX); -PERL_CALLCONV OP * Perl_pp_rkeys(pTHX); PERL_CALLCONV OP * Perl_pp_rmdir(pTHX); PERL_CALLCONV OP * Perl_pp_rv2av(pTHX); PERL_CALLCONV OP * Perl_pp_rv2cv(pTHX); PERL_CALLCONV OP * Perl_pp_rv2gv(pTHX); PERL_CALLCONV OP * Perl_pp_rv2hv(pTHX); PERL_CALLCONV OP * Perl_pp_rv2sv(pTHX); -PERL_CALLCONV OP * Perl_pp_rvalues(pTHX); PERL_CALLCONV OP * Perl_pp_sassign(pTHX); PERL_CALLCONV OP * Perl_pp_say(pTHX); PERL_CALLCONV OP * Perl_pp_scalar(pTHX); diff --git a/regen/opcode.pl b/regen/opcode.pl index bd3d55a..834934e 100755 --- a/regen/opcode.pl +++ b/regen/opcode.pl @@ -106,7 +106,6 @@ my @raw_alias = ( Perl_pp_bit_or => ['bit_xor'], Perl_pp_rv2av => ['rv2hv'], Perl_pp_akeys => ['avalues'], - Perl_pp_rkeys => [qw(rvalues reach)], Perl_pp_trans => [qw(trans transr)], ); @@ -807,11 +806,11 @@ lslice list slice ck_null 2 H L L anonlist anonymous list ([]) ck_fun ms@ L anonhash anonymous hash ({}) ck_fun ms@ L -splice splice ck_push m@ A S? S? L -push push ck_push imsT@ A L +splice splice ck_fun m@ A S? S? L +push push ck_fun imsT@ A L pop pop ck_shift s% A? shift shift ck_shift s% A? -unshift unshift ck_push imsT@ A L +unshift unshift ck_fun imsT@ A L sort sort ck_sort dm@ C? L reverse reverse ck_fun mt@ L @@ -1099,10 +1098,5 @@ once once ck_null | custom unknown custom operator ck_null 0 -# For smart dereference for each/keys/values -reach each on reference ck_each % S -rkeys keys on reference ck_each t% S -rvalues values on reference ck_each t% S - # y///r transr transliteration (tr///) ck_match is" S diff --git a/t/op/push.t b/t/op/push.t index 2804d5b..61666ee 100644 --- a/t/op/push.t +++ b/t/op/push.t @@ -14,7 +14,7 @@ -4, 4 5 6 7, 0 1 2 3 EOF -print "1..", 13 + 2*@tests, "\n"; +print "1..", 14 + 2*@tests, "\n"; die "blech" unless @tests; @x = (1,2,3); @@ -52,15 +52,19 @@ use constant CONST_ARRAYREF => [qw/a b c/]; push CONST_ARRAYREF(), qw/d e f/; if (join(':',@{CONST_ARRAYREF()}) eq 'a:b:c:d:e:f') {print "ok 9\n";} else {print "not ok 9\n";} -# test implicit dereference errors +# test implicit symbolic dereference eval "push 42, 0, 1, 2, 3"; -if ( $@ && $@ =~ /must be array/ ) {print "ok 10\n"} else {print "not ok 10 # \$\@ = $@\n"} +if (join(":", @42) eq "0:1:2:3") {print "ok 10\n"} +else {print "not ok 10\n"} +eval "use strict; push 'foo', 0, 1, 2, 3"; +if ( $@ && $@ =~ /requires explicit package/ ) {print "ok 11\n"} +else {print "not ok 11 # \$\@ = $@\n"} $hashref = { }; eval { push $hashref, 0, 1, 2, 3 }; -if ( $@ && $@ =~ /Not an ARRAY reference/ ) {print "ok 11\n"} else {print "not ok 11 # \$\@ = $@\n"} +if ( $@ && $@ =~ /Not an ARRAY reference/ ) {print "ok 12\n"} else {print "not ok 12 # \$\@ = $@\n"} -$test = 12; +$test = 13; # test context { diff --git a/t/op/smartkve.t b/t/op/smartkve.t index 4cb19f5..9f6e70d 100644 --- a/t/op/smartkve.t +++ b/t/op/smartkve.t @@ -79,13 +79,13 @@ keys CONST_HASH(); pass('Void: keys CONST_HASH();'); keys hash_sub(); pass('Void: keys hash_sub();'); keys hash_sub; pass('Void: keys hash_sub;'); keys $obj->hash; pass('Void: keys $obj->hash;'); -keys $array; pass('Void: keys $array;'); -keys $data->{array}; pass('Void: keys $data->{array};'); -keys CONST_ARRAY; pass('Void: keys CONST_ARRAY;'); -keys CONST_ARRAY(); pass('Void: keys CONST_ARRAY();'); -keys array_sub; pass('Void: keys array_sub;'); -keys array_sub(); pass('Void: keys array_sub();'); -keys $obj->array; pass('Void: keys $obj->array;'); +ok(!eval 'keys $array ;1', 'Void: keys $array;'); +ok(!eval 'keys $data->{array};1', 'Void: keys $data->{array};'); +ok(!eval 'keys CONST_ARRAY ;1', 'Void: keys CONST_ARRAY;'); +ok(!eval 'keys CONST_ARRAY() ;1', 'Void: keys CONST_ARRAY();'); +ok(!eval 'keys array_sub ;1', 'Void: keys array_sub;'); +ok(!eval 'keys array_sub() ;1', 'Void: keys array_sub();'); +ok(!eval 'keys $obj->array ;1', 'Void: keys $obj->array;'); # Keys -- scalar @@ -96,13 +96,13 @@ is(keys CONST_HASH() ,3, 'Scalar: keys CONST_HASH()'); is(keys hash_sub ,3, 'Scalar: keys hash_sub'); is(keys hash_sub() ,3, 'Scalar: keys hash_sub()'); is(keys $obj->hash ,3, 'Scalar: keys $obj->hash'); -is(keys $array ,3, 'Scalar: keys $array'); -is(keys $data->{array} ,3, 'Scalar: keys $data->{array}'); -is(keys CONST_ARRAY ,3, 'Scalar: keys CONST_ARRAY'); -is(keys CONST_ARRAY() ,3, 'Scalar: keys CONST_ARRAY()'); -is(keys array_sub ,3, 'Scalar: keys array_sub'); -is(keys array_sub() ,3, 'Scalar: keys array_sub()'); -is(keys $obj->array ,3, 'Scalar: keys $obj->array'); +is(eval 'keys $array' ,undef, 'Scalar: keys $array'); +is(eval 'keys $data->{array}',undef, 'Scalar: keys $data->{array}'); +is(eval 'keys CONST_ARRAY' ,undef, 'Scalar: keys CONST_ARRAY'); +is(eval 'keys CONST_ARRAY()' ,undef, 'Scalar: keys CONST_ARRAY()'); +is(eval 'keys array_sub' ,undef, 'Scalar: keys array_sub'); +is(eval 'keys array_sub()' ,undef, 'Scalar: keys array_sub()'); +is(eval 'keys $obj->array' ,undef, 'Scalar: keys $obj->array'); # Keys -- list @@ -116,34 +116,34 @@ is(j(keys CONST_HASH()) ,$h_expect, 'List: keys CONST_HASH()'); is(j(keys hash_sub) ,$h_expect, 'List: keys hash_sub'); is(j(keys hash_sub()) ,$h_expect, 'List: keys hash_sub()'); is(j(keys $obj->hash) ,$h_expect, 'List: keys $obj->hash'); -is(j(keys $array) ,$a_expect, 'List: keys $array'); -is(j(keys $data->{array}) ,$a_expect, 'List: keys $data->{array}'); -is(j(keys CONST_ARRAY) ,$a_expect, 'List: keys CONST_ARRAY'); -is(j(keys CONST_ARRAY()) ,$a_expect, 'List: keys CONST_ARRAY()'); -is(j(keys array_sub) ,$a_expect, 'List: keys array_sub'); -is(j(keys array_sub()) ,$a_expect, 'List: keys array_sub()'); -is(j(keys $obj->array) ,$a_expect, 'List: keys $obj->array'); +is(eval 'j(keys $array)' , undef , 'List: keys $array'); +is(eval 'j(keys $data->{array})', undef , 'List: keys $data->{array}'); +is(eval 'j(keys CONST_ARRAY)' , undef , 'List: keys CONST_ARRAY'); +is(eval 'j(keys CONST_ARRAY())' , undef , 'List: keys CONST_ARRAY()'); +is(eval 'j(keys array_sub)' , undef , 'List: keys array_sub'); +is(eval 'j(keys array_sub())' , undef , 'List: keys array_sub()'); +is(eval 'j(keys $obj->array)' , undef , 'List: keys $obj->array'); # Keys -- undef undef $empty; -is(j(keys undef), '', 'Undef: keys undef is empty list'); +is(eval 'j(keys undef)', undef, 'Undef: keys undef is error'); is(j(keys $empty), '', 'Undef: keys $empty is empty list'); -is($empty, undef, 'Undef: $empty is not vivified'); +is(ref $empty, 'HASH', 'Undef: $empty is vivified'); # Keys -- vivification is(j(keys $empty->{hash}), '', 'Vivify: keys $empty->{hash}'); ok(defined $empty , 'Vivify: $empty is HASHREF'); -ok(!defined $empty->{hash} , 'Vivify: $empty->{hash} is undef'); +is(ref $empty->{hash}, 'HASH','Vivify: $empty->{hash} is undef'); # Keys -- errors -eval "keys 3"; -ok($@ =~ qr/Type of argument to keys on reference must be hashref or arrayref/, - 'Errors: keys CONSTANT throws error' +eval "keys 'foo'"; +ok($@ =~ qr/requires explicit package/, + 'Errors: keys "foo" throws error' ); eval "keys qr/foo/"; -ok($@ =~ qr/Type of argument to keys on reference must be hashref or arrayref/, +ok($@ =~ qr/Not a HASH reference/, 'Errors: keys qr/foo/ throws error' ); @@ -161,13 +161,13 @@ values CONST_HASH(); pass('Void: values CONST_HASH();'); values hash_sub(); pass('Void: values hash_sub();'); values hash_sub; pass('Void: values hash_sub;'); values $obj->hash; pass('Void: values $obj->hash;'); -values $array; pass('Void: values $array;'); -values $data->{array}; pass('Void: values $data->{array};'); -values CONST_ARRAY; pass('Void: values CONST_ARRAY;'); -values CONST_ARRAY(); pass('Void: values CONST_ARRAY();'); -values array_sub; pass('Void: values array_sub;'); -values array_sub(); pass('Void: values array_sub();'); -values $obj->array; pass('Void: values $obj->array;'); +ok(!eval 'values $array; 1', 'Void: values $array;'); +ok(!eval 'values $data->{array}; 1', 'Void: values $data->{array};'); +ok(!eval 'values CONST_ARRAY; 1', 'Void: values CONST_ARRAY;'); +ok(!eval 'values CONST_ARRAY(); 1', 'Void: values CONST_ARRAY();'); +ok(!eval 'values array_sub; 1', 'Void: values array_sub;'); +ok(!eval 'values array_sub(); 1', 'Void: values array_sub();'); +ok(!eval 'values $obj->array; 1', 'Void: values $obj->array;'); # Values -- scalar @@ -178,13 +178,13 @@ is(values CONST_HASH() ,3, 'Scalar: values CONST_HASH()'); is(values hash_sub ,3, 'Scalar: values hash_sub'); is(values hash_sub() ,3, 'Scalar: values hash_sub()'); is(values $obj->hash ,3, 'Scalar: values $obj->hash'); -is(values $array ,3, 'Scalar: values $array'); -is(values $data->{array} ,3, 'Scalar: values $data->{array}'); -is(values CONST_ARRAY ,3, 'Scalar: values CONST_ARRAY'); -is(values CONST_ARRAY() ,3, 'Scalar: values CONST_ARRAY()'); -is(values array_sub ,3, 'Scalar: values array_sub'); -is(values array_sub() ,3, 'Scalar: values array_sub()'); -is(values $obj->array ,3, 'Scalar: values $obj->array'); +is(eval 'values $array' ,undef, 'Scalar: values $array'); +is(eval 'values $data->{array}',undef, 'Scalar: values $data->{array}'); +is(eval 'values CONST_ARRAY' ,undef, 'Scalar: values CONST_ARRAY'); +is(eval 'values CONST_ARRAY()' ,undef, 'Scalar: values CONST_ARRAY()'); +is(eval 'values array_sub' ,undef, 'Scalar: values array_sub'); +is(eval 'values array_sub()' ,undef, 'Scalar: values array_sub()'); +is(eval 'values $obj->array' ,undef, 'Scalar: values $obj->array'); # Values -- list @@ -198,34 +198,34 @@ is(j(values CONST_HASH()) ,$h_expect, 'List: values CONST_HASH()'); is(j(values hash_sub) ,$h_expect, 'List: values hash_sub'); is(j(values hash_sub()) ,$h_expect, 'List: values hash_sub()'); is(j(values $obj->hash) ,$h_expect, 'List: values $obj->hash'); -is(j(values $array) ,$a_expect, 'List: values $array'); -is(j(values $data->{array}) ,$a_expect, 'List: values $data->{array}'); -is(j(values CONST_ARRAY) ,$a_expect, 'List: values CONST_ARRAY'); -is(j(values CONST_ARRAY()) ,$a_expect, 'List: values CONST_ARRAY()'); -is(j(values array_sub) ,$a_expect, 'List: values array_sub'); -is(j(values array_sub()) ,$a_expect, 'List: values array_sub()'); -is(j(values $obj->array) ,$a_expect, 'List: values $obj->array'); +is(eval 'j(values $array)' , undef , 'List: values $array'); +is(eval 'j(values $data->{array})', undef , 'List: values $data->{array}'); +is(eval 'j(values CONST_ARRAY)' , undef , 'List: values CONST_ARRAY'); +is(eval 'j(values CONST_ARRAY())' , undef , 'List: values CONST_ARRAY()'); +is(eval 'j(values array_sub)' , undef , 'List: values array_sub'); +is(eval 'j(values array_sub())' , undef , 'List: values array_sub()'); +is(eval 'j(values $obj->array)' , undef , 'List: values $obj->array'); # Values -- undef undef $empty; -is(j(values undef), '', 'Undef: values undef is empty list'); +is(eval 'j(values undef)', undef, 'Undef: values undef is an error'); is(j(values $empty), '', 'Undef: values $empty is empty list'); -is($empty, undef, 'Undef: $empty is not vivified'); +is(ref $empty, 'HASH', 'Undef: $empty is vivified'); # Values -- vivification is(j(values $empty->{hash}), '', 'Vivify: values $empty->{hash}'); ok(defined $empty , 'Vivify: $empty is HASHREF'); -ok(!defined $empty->{hash} , 'Vivify: $empty->{hash} is undef'); +ok(defined $empty->{hash} , 'Vivify: $empty->{hash} is not undef'); # Values -- errors -eval "values 3"; -ok($@ =~ qr/Type of argument to values on reference must be hashref or arrayref/, - 'Errors: values CONSTANT throws error' +eval "values 'fee'"; +ok($@ =~ qr/requires explicit package/, + 'Errors: values "foo" throws error' ); eval "values qr/foo/"; -ok($@ =~ qr/Type of argument to values on reference must be hashref or arrayref/, +ok($@ =~ qr/Not a HASH reference/, 'Errors: values qr/foo/ throws error' ); @@ -243,13 +243,13 @@ each CONST_HASH(); pass('Void: each CONST_HASH()'); each hash_sub(); pass('Void: each hash_sub()'); each hash_sub; pass('Void: each hash_sub'); each $obj->hash; pass('Void: each $obj->hash'); -each $array; pass('Void: each $array'); -each $data->{array}; pass('Void: each $data->{array}'); -each CONST_ARRAY; pass('Void: each CONST_ARRAY'); -each CONST_ARRAY(); pass('Void: each CONST_ARRAY()'); -each array_sub; pass('Void: each array_sub'); -each array_sub(); pass('Void: each array_sub()'); -each $obj->array; pass('Void: each $obj->array'); +ok(!eval 'each $array; 1', 'Void: each $array'); +ok(!eval 'each $data->{array}; 1', 'Void: each $data->{array}'); +ok(!eval 'each CONST_ARRAY; 1', 'Void: each CONST_ARRAY;'); +ok(!eval 'each CONST_ARRAY(); 1', 'Void: each CONST_ARRAY();'); +ok(!eval 'each array_sub; 1', 'Void: each array_sub'); +ok(!eval 'each array_sub(); 1', 'Void: each array_sub()'); +ok(!eval 'each $obj->array; 1', 'Void: each $obj->array'); # Reset iterators @@ -260,13 +260,6 @@ keys CONST_HASH(); keys hash_sub(); keys hash_sub; keys $obj->hash; -keys $array; -keys $data->{array}; -keys CONST_ARRAY; -keys CONST_ARRAY(); -keys array_sub; -keys array_sub(); -keys $obj->array; # Each -- scalar @@ -277,13 +270,13 @@ keys $obj->array; @tmp=(); while(defined( $k = each hash_sub())){push @tmp,$k}; is(j(@tmp),j(keys hash_sub()), 'Scalar: each hash_sub()'); @tmp=(); while(defined( $k = each hash_sub)){push @tmp,$k}; is(j(@tmp),j(keys hash_sub), 'Scalar: each hash_sub'); @tmp=(); while(defined( $k = each $obj->hash)){push @tmp,$k}; is(j(@tmp),j(keys $obj->hash), 'Scalar: each $obj->hash'); -@tmp=(); while(defined( $k = each $array)){push @tmp,$k}; is(j(@tmp),j(keys $array), 'Scalar: each $array'); -@tmp=(); while(defined( $k = each $data->{array})){push @tmp,$k}; is(j(@tmp),j(keys $data->{array}), 'Scalar: each $data->{array}'); -@tmp=(); while(defined( $k = each CONST_ARRAY)){push @tmp,$k}; is(j(@tmp),j(keys CONST_ARRAY), 'Scalar: each CONST_ARRAY'); -@tmp=(); while(defined( $k = each CONST_ARRAY())){push @tmp,$k}; is(j(@tmp),j(keys CONST_ARRAY()), 'Scalar: each CONST_ARRAY()'); -@tmp=(); while(defined( $k = each array_sub)){push @tmp,$k}; is(j(@tmp),j(keys array_sub), 'Scalar: each array_sub'); -@tmp=(); while(defined( $k = each array_sub())){push @tmp,$k}; is(j(@tmp),j(keys array_sub()), 'Scalar: each array_sub()'); -@tmp=(); while(defined( $k = each $obj->array)){push @tmp,$k}; is(j(@tmp),j(keys $obj->array), 'Scalar: each $obj->array'); +is(eval 'each $array' ,undef, 'Scalar: each $array'); +is(eval 'each $data->{array}',undef, 'Scalar: each $data->{array}'); +is(eval 'each CONST_ARRAY' ,undef, 'Scalar: each CONST_ARRAY'); +is(eval 'each CONST_ARRAY()' ,undef, 'Scalar: each CONST_ARRAY()'); +is(eval 'each array_sub' ,undef, 'Scalar: each array_sub'); +is(eval 'each array_sub()' ,undef, 'Scalar: each array_sub()'); +is(eval 'each $obj->array' ,undef, 'Scalar: each $obj->array'); # Each -- list @@ -294,34 +287,34 @@ keys $obj->array; @tmp=@tmp2=(); while(($k,$v) = each hash_sub()){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys hash_sub(), values hash_sub()), 'List: each hash_sub()'); @tmp=@tmp2=(); while(($k,$v) = each hash_sub){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys hash_sub, values hash_sub), 'List: each hash_sub'); @tmp=@tmp2=(); while(($k,$v) = each $obj->hash){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys $obj->hash, values $obj->hash), 'List: each $obj->hash'); -@tmp=@tmp2=(); while(($k,$v) = each $array){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys $array, values $array), 'List: each $array'); -@tmp=@tmp2=(); while(($k,$v) = each $data->{array}){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys $data->{array}, values $data->{array}), 'List: each $data->{array}'); -@tmp=@tmp2=(); while(($k,$v) = each CONST_ARRAY){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys CONST_ARRAY, values CONST_ARRAY), 'List: each CONST_ARRAY'); -@tmp=@tmp2=(); while(($k,$v) = each CONST_ARRAY()){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys CONST_ARRAY(), values CONST_ARRAY()), 'List: each CONST_ARRAY()'); -@tmp=@tmp2=(); while(($k,$v) = each array_sub){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys array_sub, values array_sub), 'List: each array_sub'); -@tmp=@tmp2=(); while(($k,$v) = each array_sub()){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys array_sub(), values array_sub()), 'List: each array_sub()'); -@tmp=@tmp2=(); while(($k,$v) = each $obj->array){push @tmp,$k; push @tmp2,$v}; is(j(@tmp,@tmp2),j(keys $obj->array, values $obj->array), 'List: each $obj->array'); +is(eval 'j(each $array)' , undef , 'List: each $array'); +is(eval 'j(each $data->{array})', undef , 'List: each $data->{array}'); +is(eval 'j(each CONST_ARRAY)' , undef , 'List: each CONST_ARRAY'); +is(eval 'j(each CONST_ARRAY())' , undef , 'List: each CONST_ARRAY()'); +is(eval 'j(each array_sub)' , undef , 'List: each array_sub'); +is(eval 'j(each array_sub())' , undef , 'List: each array_sub()'); +is(eval 'j(each $obj->array)' , undef , 'List: each $obj->array'); # Each -- undef undef $empty; -is(j(@{[each undef]}), '', 'Undef: each undef is empty list'); +is(eval 'each undef;1', undef, 'Undef: each undef is an error'); is(j(@{[each $empty]}), '', 'Undef: each $empty is empty list'); -is($empty, undef, 'Undef: $empty is not vivified'); +is(ref $empty, 'HASH','Undef: $empty is vivified'); # Values -- vivification is(j(@{[each $empty->{hash}]}), '', 'Vivify: each $empty->{hash} is empty list'); ok(defined $empty , 'Vivify: $empty is HASHREF'); -ok(!defined $empty->{hash} , 'Vivify: $empty->{hash} is undef'); +ok(defined $empty->{hash} , 'Vivify: $empty->{hash} is hash ref'); # Values -- errors -eval "each 3"; -ok($@ =~ qr/Type of argument to each on reference must be hashref or arrayref/, - 'Errors: each CONSTANT throws error' +eval "each 'foo'"; +ok($@ =~ qr/requires explicit package/, + 'Errors: each "foo" throws error' ); eval "each qr/foo/"; -ok($@ =~ qr/Type of argument to each on reference must be hashref or arrayref/, +ok($@ =~ qr/Not a HASH reference/, 'Errors: each qr/foo/ throws error' ); @@ -337,25 +330,22 @@ my $over_b = Foo::Overload::Both->new; my $over_h_a = Foo::Overload::HashOnArray->new; my $over_a_h = Foo::Overload::ArrayOnHash->new; -my $re_warn_array = qr/Ambiguous overloaded argument to keys on reference resolved as \@\{\}/; -my $re_warn_hash = qr/Ambiguous overloaded argument to keys on reference resolved as \%\{\}/; - { my $warn = ''; local $SIG{__WARN__} = sub { $warn = shift }; - is(j(keys $over_a), j(keys @$array), "Overload: array dereference"); + is(eval 'keys $over_a;1', undef, "Overload: array dereference"); is($warn, '', "no warning issued"); $warn = ''; is(j(keys $over_h), j(keys %$hash), "Overload: hash dereference"); is($warn, '', "no warning issued"); $warn = ''; is(j(keys $over_b), j(keys %$hash), "Overload: ambiguous dereference (both) resolves to hash"); - like($warn, $re_warn_hash, "warning correct"); $warn = ''; + is($warn, '', 'no warning issued for %/@{} object'); $warn = ''; is(j(keys $over_h_a), j(keys %$hash), "Overload: ambiguous dereference resolves to hash"); - like($warn, $re_warn_hash, "warning correct"); $warn = ''; + is($warn, '', "no warning for array with %{}"); $warn = ''; - is(j(keys $over_a_h), j(keys @$array), "Overload: ambiguous dereference resolves to array"); - like($warn, $re_warn_array, "warning correct"); $warn = ''; + is(j(keys $over_a_h), 'foo', 'Overload: ambiguous dereference does not resolve to @{}'); + is($warn, '', 'no warning for hash with @{}'); $warn = ''; } ```
p5pRT commented 13 years ago

From @xdg

[from the tail end of the report]

Oh look\, there is a patch attached hereto.

That's a very disingenuous (not to mention sarcastic) way to submit a "revert a feature" patch.

I understand that you don't like aspects of this feature. However\, the opportunity to comment on the addition of this feature was a while ago and the debates were quite long and there was plenty of opportunity for input before the decision was taken to add it.

You cite a handful of edge cases. I'd be happy to collaborate with you on fixing the edge cases\, but I don't find your current approach to be constructive.

Regards\, David

p5pRT commented 13 years ago

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

p5pRT commented 13 years ago

From @cpansprout

On Dec 12\, 2010\, at 2​:02 PM\, David Golden via RT wrote​:

[from the tail end of the report]

Oh look\, there is a patch attached hereto.

That's a very disingenuous (not to mention sarcastic) way to submit a "revert a feature" patch.

I was afraid it might be taken that way. I apologise sincerely if I have hurt your feelings.

I understand that you don't like aspects of this feature. However\, the opportunity to comment on the addition of this feature was a while ago and the debates were quite long and there was plenty of opportunity for input before the decision was taken to add it.

There were only four days between the submission of your patch and its application.

I do not want to get dragged into an argument. So I’ve asked Jesse to decide which version we will have.

p5pRT commented 13 years ago

From @demerphq

On 12 December 2010 21​:21\, Father Chrysostomos \perlbug\-followup@&#8203;perl\.org wrote​:

# New Ticket Created by  Father Chrysostomos # Please include the string​:  [perl #80626] # in the subject line of all future correspondence about this issue. # \<URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=80626 >

$ perl5.13.7 -le 'push 42\, 1\,2\,3; print "@​42"' Type of arg 1 to push must be array (not constant item) at -e line 1\, near "3;" Execution of -e aborted due to compilation errors.

$ perl5.13.7 -le 'push ${\42}\, 1\,2\,3; print "@​42"' 1 2 3

So\, with strict mode off\, symbolic deferencing is allowed unless it is detected at compile-time.

Let’s try something else​:

$ perl5.13.7 -le 'use strict; push *f\, 1\,2\,3; print "@​​::f"' 1 2 3

$ perl5.13.7 -le 'use strict; use constant g=>*f; push g\, 1\,2\,3; print "@​​::f"' Type of arg 1 to push must be array (not constant item) at -e line 1\, near "3;" Execution of -e aborted due to compilation errors.

So push is sometimes stricter than strict.

The edge cases for keys are obviously even more weird​:

$ perl5.13.7 -le '%foo = qw(a 1 b 1 c 1); $\,=" ";print keys foo =>' Hash %foo missing the % in argument 1 of keys() at -e line 1. c a b

$ perl5.13.7 -le '%foo = qw(a 1 b 1 c 1); $\,=" ";print keys "foo"' Type of argument to keys on reference must be hashref or arrayref at -e line 1.

I thought foo => and "foo" were equivalent.

$ perl5.13.7 -le '%foo = qw(a 1 b 1 c 1); $\,=" ";print keys *foo' Type of argument to keys on reference must be hashref or arrayref at -e line 1.

But *foo is a perfectly valid array/hash reference (even under strict). Why cannot it be resolved like a blessed scalar with @​{} and %{} overloading?

More inconsistencies​:

$ perl5.13.7 -le 'keys %$x = 7' $ perl5.13.7 -le 'keys $x = 7' Can't modify keys on reference in scalar assignment at -e line 1\, near "7;" Execution of -e aborted due to compilation errors. $ perl5.13.7 -le 'keys $x; print $x//"\"' \ $ perl5.13.7 -le 'push $x; print $x//"\"' ARRAY(0x803a00)

Also\, to bring this up again\, what if I have an object that is a blessed array (I do) the contents of which are part of the public API (they are) and the blessing exists for the sake of calling methods on it? What if I want to add %{} overloading to it as an alternate interface? In that case\, I am not changing the API (even by David Golden’s standards)\, as ref() returns the same thing. But that will break any code doing keys $object.

So\, could we please make this simpler? This set of rules for dealing with constants and overloading does not match anything elsewhere in Perl. So it is just another set of edge cases for people to learn.

On one hand I think a lot of your comments are important edge cases that we need to address. In particular keys $hash= 7; jumped out at me.

On the other hand\, I dont buy the simple rules argument. I think Davids rules DWIM pretty reasonably. People that expect a specific interpretation can specify which behaviour they expect.

I feel that it would be pity if we lost this stuff because of an edge case where only a minimal number of "deeper" perl hackers would go anyway. I cannot remember the last time I saw robust production code which used reference overloading.

Is there some "third way" where you can have your desired and admittedly more consistent and robust semantics without such a drastic change to his patch? A pragma maybe?

I know you don't want to argue\, and wanted Jesse to make a ruling\, so lets say he says "The overload DWIM sticks\, but the edge cases have to be resolved" what would suggest is the right way to get them resolved?

How would you feel about writing a patch to add tests for all the crazy edge cases you have found so that David can use them to eliminate as many edge cases as possible.

Which would leave the only open question "what to do about overloading" and can be decided by Jesse when he has more time.

cheers\, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 13 years ago

From zefram@fysh.org

demerphq wrote​:

Is there some "third way" where you can have your desired and admittedly more consistent and robust semantics without such a drastic change to his patch? A pragma maybe?

The least-crazy way to keep keys($ref) is for it to always mean keys(%$ref). It's only the rarely-used array-viewed-like-a-hash operations that cause hassle here.

-zefram

p5pRT commented 13 years ago

From @xdg

On Mon\, Dec 13\, 2010 at 6​:11 PM\, Zefram \zefram@&#8203;fysh\.org wrote​:

demerphq wrote​:

Is there some "third way" where you can have your desired and admittedly more consistent and robust semantics without such a drastic change to his patch? A pragma maybe?

The least-crazy way to keep keys($ref) is for it to always mean keys(%$ref).  It's only the rarely-used array-viewed-like-a-hash operations that cause hassle here.

In the case where $ref is an unblessed arrayref\, I think it should still work like @​$ref. However\, in an ambiguous case where $ref is blessed and both @​{} and %{} are possible (due to underlying reftype or due to overloading)\, always defaulting to %{} (with a warning) is the other very reasonable approach. (I see that or the current behavior as equally arbitrary\, so defer to whatever seems to make the most sense to people.)

-- David

p5pRT commented 13 years ago

From zefram@fysh.org

David Golden wrote​:

In the case where $ref is an unblessed arrayref\, I think it should still work like @​$ref.

That's where the craziness starts. In keys(%$ref) I might have been relying on it dying if my caller passed me something other than a hash ref. If I can't rely on keys($ref) having the same behaviour then this is something extra that I have to bear in mind and check before using keys($ref). Which means I can't let myself get into the habit of casually using keys($ref) in place of keys(%$ref). Which rather destroys the point of the shorthand.

-zefram

p5pRT commented 13 years ago

From @abigail

On Tue\, Dec 14\, 2010 at 08​:44​:08AM +0000\, Zefram wrote​:

David Golden wrote​:

In the case where $ref is an unblessed arrayref\, I think it should still work like @​$ref.

That's where the craziness starts. In keys(%$ref) I might have been relying on it dying if my caller passed me something other than a hash ref. If I can't rely on keys($ref) having the same behaviour then this is something extra that I have to bear in mind and check before using keys($ref). Which means I can't let myself get into the habit of casually using keys($ref) in place of keys(%$ref). Which rather destroys the point of the shorthand.

So\, you keep using 'keys(%$ref)'\, and the proposed behaviour of 'keys($ref)' won't get in your way.

I find "but the old way dies\, and code may rely on that" a pretty stiffling argument. It basically means\, nothing can ever change - one cannot introduce a construct that's invalid in older perls\, cause there may be some code out there that relies on it currently dying.

The savings of a single keystroke isn't something that I find really exciting\, but I do appreciate the proposal\, and I can see that there are people who prefer to have less sigils. And it's a matter of more DWIM\, which I always welcome.

Abigail

p5pRT commented 13 years ago

From @xdg

On Mon\, Dec 13\, 2010 at 5​:43 PM\, demerphq \demerphq@&#8203;gmail\.com wrote​:

Which would leave the only open question "what to do about overloading" and can be decided by Jesse when he has more time.

[This is a moderately pedantic post -- those with limited time (Hi\, Jesse!) might want to jump to "CONCLUSIONS" at the bottom]

I want to speak to the overloaded objects ambiguities concern and lay out a spectrum of possible behaviors. I'll use "keys $ref" for these examples\, though it applies equally to values and each.

Here are several potential approaches​:

(0) Do not allow "keys $ref" at all. It would be a syntax error just as in 5.12.

(1) Allow "keys $ref"\, but always interpret it as "keys %{$ref}".

(2) Allow "keys $ref"\, but only for unblessed references. Dereference based on reftype. Die if $ref is an object.

(3) Allow "keys $ref"\, but only for unblessed references or objects without %{} and @​{} overloading. Dereference based on reftype. Die if $ref is an object with dereference overloading.

(4) Allow "keys $ref"\, but only for unblessed references\, objects without overloading\, objects where the reftype matches dereference overloading or objects where the reftype is neither HASH nor ARRAY and where there is only one of %{} and @​{}. Die if $ref is a blessed hash with @​{} overloading; a blessed array with %{} overloading; a blessed anything with both @​{} and %{}; or a blessed non-container without one of @​{} or %{} overloading. Dereference based on overloading if present or reftype otherwise.

All of the cases 1 - 4 above have no ambiguity in interpretation. The reference that gets dereferenced only appears to be of one reftype whether naturally or synthetically via overloading. The next case and its variations introduce the need to resolve ambiguity​:

(5) Allow "keys $ref" for any reference or object\, regardless of overloading. If there is no overloading\, or if overloading matches the reftype\, or if only one of %{} and @​{} overloading exists for a non-container reftype object\, then dereference accordingly (unambiguous). If there is ambiguity\, warn about the ambiguity and see (5a) through (5e) for resolution​:

(5a) For ambiguity\, dereference with %{} always

(5b) For ambiguity\, dereference according to the reftype for ARRAY and HASH; for non-container reftypes\, dereference based on overloading\, but favor @​{} if there are both

(5c) For ambiguity\, dereference according to the reftype for ARRAY and HASH; for non-container reftypes\, dereference based on overloading\, but favor %{} if there are both

(5d) For ambiguity\, dereference based on overloading if there is only one of @​{} or %{} and as @​{} if there are both overloadings

(5e) For ambiguity\, dereference based on overloading if there is only one of @​{} or %{} and as %{} if there are both overloadings

For reference\, the patch that is now in blead equates to (5e) above. I think all of (5a) to (5e) are relatively arbitrary choices\, which is why I think they should issue warnings. I tend to favor (5e)\, (5a) and (5c) -- in that order -- as the most sensible options among them\, which are all variations of "favor hashes" to various degrees.

To step back a bit\, I see an interesting sharp line between (2) and (3) above. Option (2) treats objects as opaque whereas options (3) and beyond break encapsulation. That could be an interesting place to draw the line because it's not unreasonable to require people breaking encapsulation to be explicit about it.

Between (3) and (4) is a step that maximizes the feature whenever there is only one way to dereference. However\, I acknowledge the risk of the sort of surprise API breakage that concerns Father C. Adding a feature (some sort of overloaded dereferencing) to a class could cause previously working code to just die.

Moving from (4) to (5) is just a less harsh approach than (4)\, in that Perl attempts to do something reasonable (for some definition of "reasonable") in ambiguous cases and issues a warning about it. Assuming that the warnings can be made fatal at the users option\, I think it's more perlish to warn instead of die and let the stricter behavior be an option for the user to enable if desired. Thus\, I think (5) is clearly preferable to (4).

CONCLUSION

There is a lot of code that just manipulates simple data structures entirely within the boundaries of a programmer's control where no ambiguity is possible. I would be disappointed to see "keys $ref" go away for those common cases because of the possibility of surprise in what I expect are much rarer cases where overloading is being used to make objects have polymorphic behavior.

Given the cases I describe above\, I see four of them as preferred options (which I'll restate for those who jumped to the conclusion)​:

(2) Defend encapsulation​: have "keys $ref" work only for unblessed references and die otherwise. No ambiguity is possible.

(5a) Always guess hash​: Have "keys $ref" work for blessed or unblessed references; in ambiguous cases\, warn and dereference with %{} always.

(5c) Guess by reftype​: Have "keys $ref" work for blessed or unblessed references; in ambiguous cases\, warn and dereference according to the reftype for ARRAY and HASH; for non-container reftypes\, dereference based on overloading\, but favor %{} if there are both

(5e) Guess by overload​: Have "keys $ref" work for blessed or unblessed references; in ambiguous cases\, warn and dereference based on overloading if there is only one of @​{} or %{} and as %{} if there are both overloadings

I chose (5e) for the original patch because it seemed the most liberal yet consistent (i.e. not based on whether something had a container reftype or not)\, but I would be happy with any of these four behaviors as the final result.

-- David

p5pRT commented 13 years ago

From @rjbs

* David Golden \xdaveg@&#8203;gmail\.com [2010-12-15T09​:24​:22]

[ ... ]

Thanks for this post.

(2) Defend encapsulation​: have "keys $ref" work only for unblessed references and die otherwise. No ambiguity is possible.

I think this is the best way forward\, of those in your conclusion. It didn't work before\, it doesn't work now\, but there's still benefit for simple containers. In the future\, overloading could provide disambiguation​: you may return an object if it marked with the overloading that says "acts like an array for push/pop/shift/unshift\," for example. Or\, alternately\, that says\, "in ^-context\, act like this kind of reference always."

The problem case that FC mentioned still exists​: You used to return an arrayref\, and people relied on calling (say) keys on it\, but now you want to return a blessed arrayref. With protected encapsulation\, without the overloading existing\, this will fail.

Although I am excited by "push $aref\, @​values" I think I am back where I spent much of my time watching this feature​: in favor of (0)\, you can't push onto a reference. The alternate proposal then was better syntax for dereference\, which has the added benefit of working in many\, many other places\, and of being entirely unambiguous.

Is has the significant failing of not yet having been implemented.

I think I still find these much better in the long run​:

  push Array $foo->bar\, @​items;   # or   push $foo->bar->@​\, @​items;

...and that if it means we spend another year having to wrap things in @​{...}\, then that's a price I'm willing to pay.

-- rjbs

p5pRT commented 13 years ago

From @ikegami

On Wed\, Dec 15\, 2010 at 9​:24 AM\, David Golden \xdaveg@&#8203;gmail\.com wrote​:

I want to speak to the overloaded objects ambiguities concern and lay out a spectrum of possible behaviors. I'll use "keys $ref" for these examples\, though it applies equally to values and each.

Here are several potential approaches​:

(0) Do not allow "keys $ref" at all. It would be a syntax error just as in 5.12.

(1) Allow "keys $ref"\, but always interpret it as "keys %{$ref}".

Hi\,

I'm against adding more polymorphism based on the current representation of the value. It goes against Perl's design\, and it's has been proven a source of problems time and time again. For example\, The Unicode Bug\, pseudo-hashes\, globs vs scalars with globs\, bit operators and even @​INC.

The effects are even predictable. How often to you think "keys $ref" will be used with the intention of preventing overloading? Yet every single option except (0) and (1) does just that. It was implied that programmers would know to avoid the unqualified version for code that could receive overloaded objects and would avoid it in those circumstances\, but that's simply not true.

- Eric Brine

p5pRT commented 13 years ago

From @jandubois

On Wed\, 15 Dec 2010\, David Golden wrote​:

Here are several potential approaches​:

(0) Do not allow "keys $ref" at all. It would be a syntax error just as in 5.12.

(1) Allow "keys $ref"\, but always interpret it as "keys %{$ref}".

(2) Allow "keys $ref"\, but only for unblessed references. Dereference based on reftype. Die if $ref is an object.

I'm in favor of (1). It is a really simple rule​: "keys/values/each work on hashes\, arrays\, and hash references"\, and it is obvious how it interacts with overloading.

So it doesn't work with array references\, but then\, it never used to do so before\, and in some ways having keys/values/each work on arrays is of questionable value to begin with (IMHO).

Cheers\, -Jan

p5pRT commented 13 years ago

From @abigail

On Wed\, Dec 15\, 2010 at 09​:24​:22AM -0500\, David Golden wrote​:

On Mon\, Dec 13\, 2010 at 5​:43 PM\, demerphq \demerphq@&#8203;gmail\.com wrote​:

Which would leave the only open question "what to do about overloading" and can be decided by Jesse when he has more time.

[This is a moderately pedantic post -- those with limited time (Hi\, Jesse!) might want to jump to "CONCLUSIONS" at the bottom]

I want to speak to the overloaded objects ambiguities concern and lay out a spectrum of possible behaviors. I'll use "keys $ref" for these examples\, though it applies equally to values and each.

Here are several potential approaches​:

(0) Do not allow "keys $ref" at all. It would be a syntax error just as in 5.12.

(1) Allow "keys $ref"\, but always interpret it as "keys %{$ref}".

This would be my option. Because a) I think this will be what most programmers want it do\, and\, more importantly\, b) you can describe that behaviour in a single line. 4) and 5) add\, IMO\, too much complexity (not in the sense of implementation\, but of the language) for too little gain. And that's probably true for 2) and 3) as well.

Abigail

p5pRT commented 13 years ago

From @tonycoz

On Wed\, Dec 15\, 2010 at 09​:24​:22AM -0500\, David Golden wrote​: > Here are several potential approaches​:

(0) Do not allow "keys $ref" at all. It would be a syntax error just as in 5.12.

(1) Allow "keys $ref"\, but always interpret it as "keys %{$ref}".

This one.

But my bias is against keys etc on arrays anyway.

Tony

p5pRT commented 13 years ago

From @cpansprout

On Mon Dec 13 14​:43​:36 2010\, demerphq wrote​:

How would you feel about writing a patch to add tests for all the crazy edge cases you have found so that David can use them to eliminate as many edge cases as possible.

I am willing to do that\, but I’ve been very busy lately and it looks as though I will not have a chance for a week and a half. The edge cases for keys will be determined by whether the current overloading behaviour will stay\, and I still have to think through all the edge cases if it does.

p5pRT commented 13 years ago

From [Unknown Contact. See original ticket]

On Mon Dec 13 14​:43​:36 2010\, demerphq wrote​:

How would you feel about writing a patch to add tests for all the crazy edge cases you have found so that David can use them to eliminate as many edge cases as possible.

I am willing to do that\, but I’ve been very busy lately and it looks as though I will not have a chance for a week and a half. The edge cases for keys will be determined by whether the current overloading behaviour will stay\, and I still have to think through all the edge cases if it does.

p5pRT commented 13 years ago

From @iabyn

On Wed\, Dec 15\, 2010 at 09​:24​:22AM -0500\, David Golden wrote​:

I want to speak to the overloaded objects ambiguities concern and lay out a spectrum of possible behaviors. I'll use "keys $ref" for these examples\, though it applies equally to values and each. [big snip]

This thread has been silent for 3 months\, and we're now one day away from the notional 5.14.0 code freeze\, so I'm wondering what we should do.

There seem to be two issues​:

a) whether to always treat $r as a *hash* ref in 'keys $r' etc   (as opposed to doing more complex stuff to determine whether to   execute it as 'keys @​$r' or 'keys %$r'). There seemed to be a rough   consensus on this approach.

b) FC identified a number of edge cases\, where the thing after keys etc   wasn't a simple scalar variable\, but a glob\, or a '=>'-quoted string   etc. There doesn't seem to be much open for discussion there\, its more   just a case of fixing them.

Now\, can you (DG) and/or us (p5p) fix (a) and/or (b) before the code-freeze\, and if not\, should this feature be removed for now\, and re-addressed post-5.14??

I think I'd reluctantly vote for removal.

-- "But Sidley Park is already a picture\, and a most amiable picture too. The slopes are green and gentle. The trees are companionably grouped at intervals that show them to advantage. The rill is a serpentine ribbon unwound from the lake peaceably contained by meadows on which the right amount of sheep are tastefully arranged." -- Lady Croom\, "Arcadia"

p5pRT commented 13 years ago

From @xdg

On Sat\, Mar 19\, 2011 at 6​:57 PM\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

Now\, can you (DG) and/or us (p5p) fix (a) and/or (b) before the code-freeze\, and if not\, should this feature be removed for now\, and re-addressed post-5.14??

I think I'd reluctantly vote for removal.

We're past the freeze for "user visible changes"\, which (IMO) would include removal of a feature.

The DWIM behavior decision is ultimately Jesse's and a number of options were laid out and an opinions presented (on and off list). If Jesse isn't ready to make a decision\, maybe we document the feature as "experimental" for 5.14\, which gives him wiggle room to decide later.

The edge case fixes were blocking on TODO tests for them. Father C was hoping to get to them when time permitted. Demerphq's assessment seemed to be that they might only impact "deeper" perl hackers in any case. I think those should just be fixed in 5.15 and (optionally) backported to 5.14

-- David

p5pRT commented 13 years ago

From ambrus@math.bme.hu

On Tue\, Dec 14\, 2010 at 12​:11 AM\, Zefram \zefram@&#8203;fysh\.org wrote​:

The least-crazy way to keep keys($ref) is for it to always mean keys(%$ref).  It's only the rarely-used array-viewed-like-a-hash operations that cause hassle here.

-zefram

Agreed. Please let this be the behaviour. No implicit dereferencing arrays with keys or values.

On Sun\, Mar 20\, 2011 at 1​:44 AM\, David Golden \xdaveg@&#8203;gmail\.com wrote​:

We're past the freeze for "user visible changes"\, which (IMO) would include removal of a feature.

The DWIM behavior decision is ultimately Jesse's and a number of options were laid out and an opinions presented (on and off list). If Jesse isn't ready to make a decision\, maybe we document the feature as "experimental" for 5.14\, which gives him wiggle room to decide later.

I'd be okay with that.

Ambrus

p5pRT commented 13 years ago

From @obra

On Sat 19.Mar'11 at 20​:44​:37 -0400\, David Golden wrote​:

On Sat\, Mar 19\, 2011 at 6​:57 PM\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

Now\, can you (DG) and/or us (p5p) fix (a) and/or (b) before the code-freeze\, and if not\, should this feature be removed for now\, and re-addressed post-5.14??

I think I'd reluctantly vote for removal.

We're past the freeze for "user visible changes"\, which (IMO) would include removal of a feature.

The DWIM behavior decision is ultimately Jesse's and a number of options were laid out and an opinions presented (on and off list). If Jesse isn't ready to make a decision\, maybe we document the feature as "experimental" for 5.14\, which gives him wiggle room to decide later.

I suppose that marking it as experimental does give us the wiggle room to deal with it after 5.14\, but I worry that it's a "behavior" rather than a feature. I'd sort of been waiting to see the testing of edge cases get sorted out. How plausible would it be to hide this feature behind a "use feature" directive for 5.14?

-j

The edge case fixes were blocking on TODO tests for them. Father C was hoping to get to them when time permitted. Demerphq's assessment seemed to be that they might only impact "deeper" perl hackers in any case. I think those should just be fixed in 5.15 and (optionally) backported to 5.14

-- David

p5pRT commented 13 years ago

From @cpansprout

On Mar 20\, 2011\, at 4​:30 PM\, Jesse via RT wrote​:

On Sat 19.Mar'11 at 20​:44​:37 -0400\, David Golden wrote​:

On Sat\, Mar 19\, 2011 at 6​:57 PM\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

Now\, can you (DG) and/or us (p5p) fix (a) and/or (b) before the code-freeze\, and if not\, should this feature be removed for now\, and re-addressed post-5.14??

I think I'd reluctantly vote for removal.

We're past the freeze for "user visible changes"\, which (IMO) would include removal of a feature.

The DWIM behavior decision is ultimately Jesse's and a number of options were laid out and an opinions presented (on and off list). If Jesse isn't ready to make a decision\, maybe we document the feature as "experimental" for 5.14\, which gives him wiggle room to decide later.

I suppose that marking it as experimental does give us the wiggle room to deal with it after 5.14\, but I worry that it's a "behavior" rather than a feature.

I'd sort of been waiting to see the testing of edge cases get sorted out.

I’m afraid I had forgotten about the edge cases. Now that I think about it though\, we have a chicken-and-egg problem. There is no problem with push\, splice\, etc.\, as the edge cases are all currently errors and can be resolved later without backward compatibility concerns. The edges cases for keys\, each and values\, however\, entirely depend upon your decision with regard to array refs and objects.

How plausible would it be to hide this feature behind a "use feature" directive for 5.14?

That would be possible\, but wouldn’t it be strange to put it there? What would the same ‘feature’ feature do in 5.16?

I think marking that aspect of those functions as experimental in the docs (and removing the ‘or ARRAYREF’ from the =item directives in perlfunc) would be acceptable.

-j

The edge case fixes were blocking on TODO tests for them. Father C was hoping to get to them when time permitted. Demerphq's assessment seemed to be that they might only impact "deeper" perl hackers in any case. I think those should just be fixed in 5.15 and (optionally) backported to 5.14

-- David

p5pRT commented 13 years ago

From @cpansprout

On Mon Mar 21 20​:40​:19 2011\, sprout wrote​:

On Mar 20\, 2011\, at 4​:30 PM\, Jesse via RT wrote​:

I'd sort of been waiting to see the testing of edge cases get sorted out.

I’m afraid I had forgotten about the edge cases. Now that I think about it though\, we have a chicken-and-egg problem. There is no problem with push\, splice\, etc.\, as the edge cases are all currently errors and can be resolved later without backward compatibility concerns. The edges cases for keys\, each and values\, however\, entirely depend upon your decision with regard to array refs and objects.

Attached is a first go at a test script. Edit the JC constant to choose between the different behaviours tested for.

BTW\, I noticed that ck_push is duplicating some logic from ck_fun. That was why in my first patch to this ticket I deleted ck_push and added some code to the array section of ck_fun.

p5pRT commented 13 years ago

From @cpansprout

#!/usr/bin/perl

# For most of these tests\, I���m actually trying to make the behaviour # comparable to what comes before ->[...] and ->{...}. Note that # "foo"->{...} is permitted even under strict refs (strict vars\, not refs\, # catches it)\, as the name is known at compile time. Hence "INC"->{...} and # 42->[...] are always permitted\, as they are globals.

# I���m not entirely sure about the edge cases for keys for 5a-5e.

# This script as-is should not be incorporated into the perl core\, but # rather tests from it should be incorporated into other test scripts\, # whichever are appropriate.

# What the tests test for are based on Jesse���s Choice​: sub JC (){ '1' } # (please) # which is one of David Golden���s options​:   # (0) Do not allow keys "$ref" at all.   # (1) keys $ref always means keys %$ref   # (2) keys $ref only on unblessed refs   # (3) keys $ref only on non-deref-overloaded refs   # (4) keys $ref for hashes and arrays\, but not arrays with %{}\,   # hashes with @​{}\, or other objects with both @​{} and %{}   # or neither   # (5) keys $ref on any reference or object\, resolving ambiguities   # like this​:   # (5a) use %{}   # (5b) based on underlying type for real arrays and hashes\,   # based on overloading otherwise\, favouring @​{}   # (5c) based on underlying type for real arrays and hashes\,   # based on overloading otherwise\, favouring %{}   # (5d) dereference based on overloading\, when present\, favouring   # @​{} when both overloading types are present   # (5e) dereference based on overloading\, when present\, favouring   # %{} when both overloading types are present

BEGIN { die "Why are you even running this?"if!JC } BEGIN { die "Specify the suboption\, please" if JC eq 5 } BEGIN { die "Invalid choice" if JC !~ /^(?​:[1-4]|5[a-e])\z/}

use Test​::More; plan tests => 19; use strict;

# push with various non-ref scalars eval 'push 42\, 1\,2\,3'; is "@​42"\, "1 2 3"\, 'push 42'; ok !eval 'push ${\42}\, 1\,2\,3; 1' && $@​ =~ /strict refs/\,   'push $expr is subject to strict refs'; ok !eval 'push "foo"\, 1\,2\,3; 1' && $@​ =~ /explicit package name/\,   'push "string" is subject to strict vars';

# push with globs eval 'push *bar\, 1\,2\,3'; is "@​​::bar"\, "1 2 3"\, 'push *bar'; use constant bazglob => *baz; eval 'push bazglob\, 1\,2\,3'; is "@​​::baz"\, "1 2 3"\, 'push glob_constant';

# keys with various non-ref scalars @​42 = (1\,2\,3); %42 = qw(a 1 b 1 c 1); my @​keys = eval 'sort keys 42'; is "@​keys"\,   JC =~ /[234]/ ? "" : JC =~ /1|5[ace]/ ? "a b c" : "1 2 3"\,   'keys 42 when both @​42 and %42 exist'; ok JC =~ /[234]/ ? $@​ : !$@​\, '(no) error after keys 42'; our %hash = %42; our @​array = @​42; @​keys = eval 'sort keys "hash"'; is "@​keys"\,   JC =~ /[234]/ ? "" : "a b c"\,   'keys "hash" when only %hash exists'; ok JC =~ /[234]/ ? $@​ : !$@​\, '(no) error after keys "hash"'; @​keys = eval 'sort keys "array"'; is "@​keys"\,   JC =~ /[1-4]|5a/ ? "" : "1 2 3"\,   'keys "array" when only @​array exists'; ok JC =~ /[234]/ ? $@​ : !$@​\, '(no) error after keys "array"';

# keys with globs @​keys = eval 'sort keys *42'; is "@​keys"\,   JC =~ /[234]/ ? "" : JC =~ /1|5[ace]/ ? "a b c" : "1 2 3"\,   'keys *42 when both @​42 and %42 exist'; ok JC =~ /[234]/ ? $@​ : !$@​\, '(no) error after keys *42'; @​keys = eval 'sort keys *hash'; is "@​keys"\,   JC =~ /[234]/ ? "" : "a b c"\,   'keys *hash when only %hash exists'; ok JC =~ /[234]/ ? $@​ : !$@​\, '(no) error after keys *hash'; @​keys = eval 'sort keys *array'; is "@​keys"\,   JC =~ /[1-4]|5a/ ? "" : "1 2 3"\,   'keys *array when only @​array exists'; ok JC =~ /[234]/ ? $@​ : !$@​\, '(no) error after keys *array';

# lvalue keys my $aref = []; my $href = {}; ok !eval 'keys $aref = 8; 1'\, 'keys $aref'; ok eval 'keys $href = 8; 1'\, 'keys $href';

p5pRT commented 13 years ago

From [Unknown Contact. See original ticket]

On Mon Mar 21 20​:40​:19 2011\, sprout wrote​:

On Mar 20\, 2011\, at 4​:30 PM\, Jesse via RT wrote​:

I'd sort of been waiting to see the testing of edge cases get sorted out.

I’m afraid I had forgotten about the edge cases. Now that I think about it though\, we have a chicken-and-egg problem. There is no problem with push\, splice\, etc.\, as the edge cases are all currently errors and can be resolved later without backward compatibility concerns. The edges cases for keys\, each and values\, however\, entirely depend upon your decision with regard to array refs and objects.

Attached is a first go at a test script. Edit the JC constant to choose between the different behaviours tested for.

BTW\, I noticed that ck_push is duplicating some logic from ck_fun. That was why in my first patch to this ticket I deleted ck_push and added some code to the array section of ck_fun.

p5pRT commented 13 years ago

From @iabyn

On Mon\, Mar 21\, 2011 at 08​:39​:42PM -0700\, Father Chrysostomos wrote​:

How plausible would it be to hide this feature behind a "use feature" directive for 5.14?

That would be possible\, but wouldn’t it be strange to put it there? What would the same ‘feature’ feature do in 5.16?

I agree. 'use feature' is inappropriate here.

I think marking that aspect of those functions as experimental in the docs (and removing the ‘or ARRAYREF’ from the =item directives in perlfunc) would be acceptable.

Agreed. I think the docs should make clear that any use apart from on an *unblessed* array or hash ref is undefined and highly subject to change.

Note also that traditionally we don't change such behaviours during maint cycles (the smartmatch in 5.10.1 being the big ugly exception).

I'm happy to make the doc changes if Jesse gives the nod.

-- Monto Blanco... scorchio!

p5pRT commented 13 years ago

From @cpansprout

On Mar 30\, 2011\, at 7​:40 AM\, Dave Mitchell wrote​:

On Mon\, Mar 21\, 2011 at 08​:39​:42PM -0700\, Father Chrysostomos wrote​:

How plausible would it be to hide this feature behind a "use feature" directive for 5.14?

That would be possible\, but wouldn’t it be strange to put it there? What would the same ‘feature’ feature do in 5.16?

I agree. 'use feature' is inappropriate here.

I think marking that aspect of those functions as experimental in the docs (and removing the ‘or ARRAYREF’ from the =item directives in perlfunc) would be acceptable.

Agreed. I think the docs should make clear that any use apart from on an *unblessed* array or hash ref is undefined and highly subject to change.

For push\, shift\, etc.\, the only controversial cases are currently errors.

For keys\, each and values\, even unblessed array refs are still controversial. So I think the easiest approach for now is to mark keys/each/values with *any* scalar as being experimental and subject to change.

Attached is a diff with my recommendation as to how the docs should be changed.

Note also that traditionally we don't change such behaviours during maint cycles (the smartmatch in 5.10.1 being the big ugly exception).

I'm happy to make the doc changes if Jesse gives the nod.

p5pRT commented 13 years ago

From @cpansprout

Inline Patch ```diff diff --git a/pod/perldelta.pod b/pod/perldelta.pod index 646d1bc..8624143 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -253,6 +253,10 @@ C on an entry of C<%+> or C<%->. =head3 Array and hash container functions accept references +B For C, C and C this feature is considered +experimental, as the exact behaviour may change in a future version of +Perl. + All built-in functions that operate directly on array or hash containers now also accept hard references to arrays or hashes: diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod index c2a68a5..9ce7bea 100644 --- a/pod/perlfunc.pod +++ b/pod/perlfunc.pod @@ -1467,10 +1467,10 @@ convert a core file into an executable. That's why you should now invoke it as C, if you don't want to be warned against a possible typo. -=item each HASH (or HASHREF) +=item each HASH X X -=item each ARRAY (or ARRAYREF) +=item each ARRAY X When called in list context, returns a 2-element list consisting of the key @@ -1509,8 +1509,10 @@ but in a different order: print "$key=$value\n"; } -When given a reference to a hash or array, the argument will be -dereferenced automatically. +Starting with Perl 5.14, C can take a reference to a hash or array. +The argument will be dereferenced automatically. This aspect of C +is considered highly experimental. The exact behaviour may change in a +future version of Perl. while (($key,$value) = each $hashref) { ... } @@ -2641,10 +2643,10 @@ separated by the value of EXPR, and returns that new string. Example: Beware that unlike C, C doesn't take a pattern as its first argument. Compare L. -=item keys HASH (or HASHREF) +=item keys HASH X X -=item keys ARRAY (or ARRAYREF) +=item keys ARRAY Returns a list consisting of all the keys of the named hash, or the indices of an array. (In scalar context, returns the number of keys or indices.) @@ -2701,8 +2703,10 @@ C in this way (but you needn't worry about doing this by accident, as trying has no effect). C in an lvalue context is a syntax error. -When given a reference to a hash or array, the argument will be -dereferenced automatically. +Starting with Perl 5.14, C can take a reference to a hash or array. +The argument will be dereferenced automatically. This aspect of C +is considered highly experimental. The exact behaviour may change in a +future version of Perl. for (keys $hashref) { ... } for (keys $obj->get_arrayref) { ... } @@ -7435,10 +7439,10 @@ files. On systems that don't support futimes(2), passing filehandles raises an exception. Filehandles must be passed as globs or glob references to be recognized; barewords are considered filenames. -=item values HASH (or HASHREF) +=item values HASH X -=item values ARRAY (or ARRAYREF) +=item values ARRAY Returns a list consisting of all the values of the named hash, or the values of an array. (In a scalar context, returns the number of values.) @@ -7466,8 +7470,10 @@ modify the contents of the hash: for (values %hash) { s/foo/bar/g } # modifies %hash values for (@hash{keys %hash}) { s/foo/bar/g } # same -When given a reference to a hash or array, the argument will be -dereferenced automatically. +Starting with Perl 5.14, C can take a reference to a hash or array. +The argument will be dereferenced automatically. This aspect of C +is considered highly experimental. The exact behaviour may change in a +future version of Perl. for (values $hashref) { ... } for (values $obj->get_arrayref) { ... } ```
p5pRT commented 13 years ago

From @obra

After poring over the test scripts and various recommendations\, requests\, suggestions and concerns\, I believe that the right course of action at this point is what\, I believe was referred to as "proposal 2" at some point in the past​:

* keys\, each and values should only magically dereference unblessed   hashrefs and arrayrefs.

* keys\, each and values should behave as they did in 5.12 for blessed   references

* For 5.14\, we will mark this behavior as experimental\, with an eye   toward removing the experimental marker late in the 5.15 series   based on positive user feedback.

I'm really sorry that I've been so avoidant on this one.

-Jesse

p5pRT commented 13 years ago

From @obra

As happens with some regularity\, I made a pronouncement that was too vague and didn't cover all the cases I was being asked about.

Let me try again​:

  The following should magically dereference unblessed arrayrefs​:   push\, pop\, shift\, unshift

  The following should magically dereference unblessed hashrefs or arrayrefs​:   keys\, values\, each

Everything else should work like it did in 5.12.

Now\, given that pronouncement\, I _believe_ we have some bugs to fix and some extra functionalty to remove.

-j

p5pRT commented 13 years ago

From @abigail

On Tue\, Apr 12\, 2011 at 06​:50​:03AM -0400\, Jesse Vincent wrote​:

As happens with some regularity\, I made a pronouncement that was too vague and didn't cover all the cases I was being asked about.

Let me try again​:

The following should magically dereference unblessed arrayrefs​: push\, pop\, shift\, unshift

No splice?

The following should magically dereference unblessed hashrefs or arrayrefs​: keys\, values\, each

Everything else should work like it did in 5.12.

Now\, given that pronouncement\, I _believe_ we have some bugs to fix and some extra functionalty to remove.

Abigail

p5pRT commented 13 years ago

From @obra

On Tue\, Apr 12\, 2011 at 12​:55​:00PM +0200\, Abigail wrote​:

On Tue\, Apr 12\, 2011 at 06​:50​:03AM -0400\, Jesse Vincent wrote​:

As happens with some regularity\, I made a pronouncement that was too vague and didn't cover all the cases I was being asked about.

Let me try again​:

The following should magically dereference unblessed arrayrefs​: push\, pop\, shift\, unshift

No splice?

*sigh* The first version of this message had just said "keys and friends" Can I get away with falling back on "the customary set of functions that operate on hashes and arrays"?

p5pRT commented 13 years ago

From @xdg

On Tue\, Apr 12\, 2011 at 7​:02 AM\, Jesse Vincent \jesse@&#8203;fsck\.com wrote​:

On Tue\, Apr 12\, 2011 at 12​:55​:00PM +0200\, Abigail wrote​:

On Tue\, Apr 12\, 2011 at 06​:50​:03AM -0400\, Jesse Vincent wrote​:

As happens with some regularity\, I made a pronouncement that was too vague and didn't cover all the cases I was being asked about.

Let me try again​:

  The following should magically dereference unblessed arrayrefs​:     push\, pop\, shift\, unshift

No splice?

*sigh* The first version of this message had just said "keys and friends"   Can I get away with falling back on "the customary set of functions that operate on hashes and arrays"?

Splice was not made magical like push/pop/etc.

-- David

p5pRT commented 13 years ago

From @xdg

On Tue\, Apr 12\, 2011 at 7​:41 AM\, David Golden \xdaveg@&#8203;gmail\.com wrote​:

On Tue\, Apr 12\, 2011 at 7​:02 AM\, Jesse Vincent \jesse@&#8203;fsck\.com wrote​:

On Tue\, Apr 12\, 2011 at 12​:55​:00PM +0200\, Abigail wrote​:

On Tue\, Apr 12\, 2011 at 06​:50​:03AM -0400\, Jesse Vincent wrote​:

As happens with some regularity\, I made a pronouncement that was too vague and didn't cover all the cases I was being asked about.

Let me try again​:

  The following should magically dereference unblessed arrayrefs​:     push\, pop\, shift\, unshift

No splice?

*sigh* The first version of this message had just said "keys and friends"   Can I get away with falling back on "the customary set of functions that operate on hashes and arrays"?

Splice was not made magical like push/pop/etc.

Sorry -- yes it was and it should be made to work only on unblessed references like push/pop/shift/unshift.

Apologies for the confusion. I'm in week 3 of my sleep deprivation experiment and my memory occasionally misfires. :-)

-- David

p5pRT commented 13 years ago

From @cpansprout

On Apr 12\, 2011\, at 6​:25 AM\, David Golden via RT wrote​:

On Tue\, Apr 12\, 2011 at 7​:41 AM\, David Golden \xdaveg@&#8203;gmail\.com wrote​:

On Tue\, Apr 12\, 2011 at 7​:02 AM\, Jesse Vincent \jesse@&#8203;fsck\.com wrote​:

On Tue\, Apr 12\, 2011 at 12​:55​:00PM +0200\, Abigail wrote​:

On Tue\, Apr 12\, 2011 at 06​:50​:03AM -0400\, Jesse Vincent wrote​:

As happens with some regularity\, I made a pronouncement that was too vague and didn't cover all the cases I was being asked about.

Let me try again​:

The following should magically dereference unblessed arrayrefs​: push\, pop\, shift\, unshift

No splice?

*sigh* The first version of this message had just said "keys and friends" Can I get away with falling back on "the customary set of functions that operate on hashes and arrays"?

Splice was not made magical like push/pop/etc.

Sorry -- yes it was and it should be made to work only on unblessed references like push/pop/shift/unshift.

Apologies for the confusion. I'm in week 3 of my sleep deprivation experiment and my memory occasionally misfires. :-)

Jesse​: What about keys %hashref as an lvalue?

David​: Are you going to do this\, or shall I go ahead?

p5pRT commented 13 years ago

From @xdg

On Sun\, Apr 17\, 2011 at 8​:08 PM\, Father Chrysostomos \sprout@&#8203;cpan\.org wrote​:

Jesse​: What about keys %hashref as an lvalue?

David​: Are you going to do this\, or shall I go ahead?

Father C​: I've got a 3 week old baby and family in town\, so I've hardly touched my computer. If you or someone could make the necessary changes for Jesses\, I'd greatly appreciate it.

Regards\, David

p5pRT commented 13 years ago

From @obra

On Sun\, Apr 17\, 2011 at 05​:08​:38PM -0700\, Father Chrysostomos wrote​:

Jesse​: What about keys %hashref as an lvalue?

On an unblessed hashref\, my ideal would be for it to work as close to how keys would work on a hash. I suppose that yes\, the lvalue behavior should work the same way.

Thanks for the catch.

--

p5pRT commented 13 years ago

From @cpansprout

On Apr 17\, 2011\, at 6​:36 PM\, David Golden wrote​:

On Sun\, Apr 17\, 2011 at 8​:08 PM\, Father Chrysostomos \sprout@&#8203;cpan\.org wrote​:

Jesse​: What about keys %hashref as an lvalue?

David​: Are you going to do this\, or shall I go ahead?

Father C​: I've got a 3 week old baby and family in town\, so I've hardly touched my computer. If you or someone could make the necessary changes for Jesses\, I'd greatly appreciate it.

OK\, I will.

p5pRT commented 13 years ago

From @obra

sprout's series of commits just now gets us to the place we hoped to be for 5.14.0. I'm resolving this ticket.

p5pRT commented 13 years ago

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