Skip to content

Defined-or operator assigns key in hash too early #14023

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

Closed
p5pRT opened this issue Aug 12, 2014 · 12 comments
Closed

Defined-or operator assigns key in hash too early #14023

p5pRT opened this issue Aug 12, 2014 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 12, 2014

Migrated from rt.perl.org#122512 (status was 'rejected')

Searchable as RT122512$

@p5pRT
Copy link
Author

p5pRT commented Aug 12, 2014

From [email protected]

If I use the defined-or operator in the manner shown in the example, the value 1 gets assigned to $hello{key}, instead of the expected value 0​:

  $ perl -E 'my %hello; $hello{key} //= keys %hello; say $hello{key}'
  1

This is especially surprising, considering that the equivalent using the ternary operator does not behave this way, but behaves as expected​:

  $ perl -E 'my %hello; $hello{key} = defined($hello{key}) ? $hello{key} : keys %hello; say $hello{key}'
  0



Flags​:
  category=core
  severity=medium


Site configuration information for perl 5.20.0​:

Configured by davidl at Thu Aug 7 09​:55​:02 BST 2014.

Summary of my perl5 (revision 5 version 20 subversion 0) configuration​:
 
  Platform​:
  osname=linux, osvers=3.13.0-32-generic, archname=x86_64-linux
  uname='linux gonin 3.13.0-32-generic #57-ubuntu smp tue jul 15 03​:51​:08 utc 2014 x86_64 x86_64 x86_64 gnulinux '
  config_args='-de -Dprefix=/home/davidl/perl5/perlbrew/perls/perl-5.20.0 -Aeval​:scriptdir=/home/davidl/perl5/perlbrew/perls/perl-5.20.0/bin'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2',
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.8.2', 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='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.8/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.19'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'


@​INC for perl 5.20.0​:
  /home/davidl/perl5/lib/perl5
  /home/davidl/perl5/perlbrew/perls/perl-5.20.0/lib/site_perl/5.20.0/x86_64-linux
  /home/davidl/perl5/perlbrew/perls/perl-5.20.0/lib/site_perl/5.20.0
  /home/davidl/perl5/perlbrew/perls/perl-5.20.0/lib/5.20.0/x86_64-linux
  /home/davidl/perl5/perlbrew/perls/perl-5.20.0/lib/5.20.0
  .


Environment for perl 5.20.0​:
  HOME=/home/davidl
  LANG=en_GB.UTF-8
  LANGUAGE=en_GB​:en
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/home/davidl/perl5/perlbrew/bin​:/home/davidl/perl5/perlbrew/perls/perl-5.20.0/bin​:/home/davidl/perl5/bin​:/home/davidl/.local/bin​:/usr/local/sbin​:/usr/local/bin​:/usr/sbin​:/usr/bin​:/sbin​:/bin​:/usr/games​:/usr/local/games
  PERL5LIB=/home/davidl/perl5/lib/perl5
  PERLBREW_BASHRC_VERSION=0.66
  PERLBREW_HOME=/home/davidl/.perlbrew
  PERLBREW_MANPATH=/home/davidl/perl5/perlbrew/perls/perl-5.20.0/man
  PERLBREW_PATH=/home/davidl/perl5/perlbrew/bin​:/home/davidl/perl5/perlbrew/perls/perl-5.20.0/bin
  PERLBREW_PERL=perl-5.20.0

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2014

From @cpansprout

On Tue Aug 12 08​:56​:11 2014, Flimm wrote​:

If I use the defined-or operator in the manner shown in the example,
the value 1 gets assigned to $hello{key}, instead of the expected
value 0​:

$ perl -E 'my %hello; $hello{key} //= keys %hello; say $hello{key}'
1

This is especially surprising, considering that the equivalent using
the ternary operator does not behave this way, but behaves as
expected​:

$ perl -E 'my %hello; $hello{key} = defined($hello{key}) ? $hello{key}
: keys %hello; say $hello{key}'
0

I’m not sure this is a bug. The equivalent using the ternary operator is not absolutely equivalent, because it evaluates the $hello{key} twice, once in rvalue context (not vivifying the element), and once in lvalue context (vivifying). //= only evalutes its lhs once, and in lvalue context at that, so it vivifies. I don’t see any other way this could work.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2014

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

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2014

From [email protected]

I’m not sure this is a bug. The equivalent using the ternary operator
is not absolutely equivalent, because it evaluates the $hello{key}
twice, once in rvalue context (not vivifying the element), and once in
lvalue context (vivifying). //= only evalutes its lhs once, and in
lvalue context at that, so it vivifies. I don’t see any other way
this could work.

It could work the way that I expect it :)

Simply evaluating $hello{key} should not cause the key "key" to exist in the hash %hello, that's not how autovivifaction is documented to work, or works in practise​:

  $ perl -E 'my %hello; say $hello{key}; say scalar(keys %hello);'

  0

Autovivification should only create new references to hashes or arrays, according to perlref, which is clearly not what is created here, $hello{key} is never a reference.

I don't see any documentation that says that autovivifaction works differently when it's the left-hand-side of a defined-or operator. It certainly doesn't behave that way when it's the left-hand side of the assignment operator​:

  $ perl -E 'my %hello; $hello{key} = keys %hello; say $hello{key}'
  0

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2014

From @ap

* David D. Lowe via RT <perlbug-followup@​perl.org> [2014-08-13 11​:35]​:

I’m not sure this is a bug. The equivalent using the ternary
operator is not absolutely equivalent, because it evaluates the
$hello{key} twice, once in rvalue context (not vivifying the
element), and once in lvalue context (vivifying). //= only evalutes
its lhs once, and in lvalue context at that, so it vivifies. I don’t
see any other way this could work.

It could work the way that I expect it :)

Simply evaluating $hello{key} should not cause the key "key" to exist
in the hash %hello, that's not how autovivifaction is documented to
work, or works in practise​:

$ perl \-E 'my %hello; say $hello\{key\}; say scalar\(keys %hello\);'
0

What relevance does your example have here? There is no autovivification
taking place.

Autovivification should only create new references to hashes or
arrays, according to perlref, which is clearly not what is created
here, $hello{key} is never a reference.

I don't see any documentation that says that autovivifaction works
differently when it's the left-hand-side of a defined-or operator. It
certainly doesn't behave that way when it's the left-hand side of the
assignment operator​:

$ perl \-E 'my %hello; $hello\{key\} = keys %hello; say $hello\{key\}'
0

Obviously. That doesn’t need to evaluate the LHS to know whether to
execute the RHS, it can and does just execute the RHS first. Any time
you force it into lvalue context first, you’ll necessarily get the same
behaviour as with defined-or​:

  $ perl -E 'my %hello; $_ = keys %hello for $hello{key}; say $hello{key}'
  1

And while it might seem obvious that $hello{key} //= EXPR should behave
like you ask for, the LHS of defined-or can be anything, including

  $hash{ pop @​entry } //= EXPR
  @​array[ rand 30 ] //= EXPR

where you can’t just evaluate the LHS expression twice and expect it to
work.

And because you *might* need to store a value, and you need an lvalue
for that, and you can only evaluate the expression once, then you *must*
evaluate it in lvalue context even before you know whether you need an
lvalue. This is simply inescapable.

The only way around this fact would be a deep change to the way that
lvalue context interacts with hashes, so that it wouldn’t just allocate
the key in order to refer to the value, but instead create an “intent”
to store a value in that key, which could go away unfulfilled. Then the
inescapable necessity of lvalue context would not preclude the behaviour
you ask for. But when you go to reason this out, you run into the fact
that several such intents for the same key can then coexist at the same
time (esp in Perl where lvalues can be passed around without losing
lvalueness), and that quickly leads you to various identity problems and
temporal paradoxes (because now one thing (the same hash key) can hide
behind multiple different intents) – all far more heady stuff than the
“surprise” you are complaining of.

The way it works now is really the only way it can sanely work.

That way may not be satisfying for some simple “obvious” cases where it
seems like it should clearly be able to do something smarter, such as
in your example. But in actual fact you would need to find and special-
case every one of these simple cases.

Regards,
--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2014

From @epa

This is consistent with some other assignment-modification operators​:

% perl -E 'my %hello; $hello{key} ||= keys %hello; say $hello{key}'
1

It is not specific to defined-or.

@p5pRT
Copy link
Author

p5pRT commented Sep 14, 2014

From @jkeenan

On Wed Aug 13 03​:26​:43 2014, aristotle wrote​:

* David D. Lowe via RT <perlbug-followup@​perl.org> [2014-08-13 11​:35]​:

I’m not sure this is a bug. The equivalent using the ternary
operator is not absolutely equivalent, because it evaluates the
$hello{key} twice, once in rvalue context (not vivifying the
element), and once in lvalue context (vivifying). //= only evalutes
its lhs once, and in lvalue context at that, so it vivifies. I don’t
see any other way this could work.

It could work the way that I expect it :)

Simply evaluating $hello{key} should not cause the key "key" to exist
in the hash %hello, that's not how autovivifaction is documented to
work, or works in practise​:

$ perl \-E 'my %hello; say $hello\{key\}; say scalar\(keys %hello\);'
0

What relevance does your example have here? There is no autovivification
taking place.

Autovivification should only create new references to hashes or
arrays, according to perlref, which is clearly not what is created
here, $hello{key} is never a reference.

I don't see any documentation that says that autovivifaction works
differently when it's the left-hand-side of a defined-or operator. It
certainly doesn't behave that way when it's the left-hand side of the
assignment operator​:

$ perl \-E 'my %hello; $hello\{key\} = keys %hello; say $hello\{key\}'
0

Obviously. That doesn’t need to evaluate the LHS to know whether to
execute the RHS, it can and does just execute the RHS first. Any time
you force it into lvalue context first, you’ll necessarily get the same
behaviour as with defined-or​:

$ perl \-E 'my %hello; $\_ = keys %hello for $hello\{key\}; say $hello\{key\}'
1

And while it might seem obvious that $hello{key} //= EXPR should behave
like you ask for, the LHS of defined-or can be anything, including

$hash\{ pop @&#8203;entry \} //= EXPR
@&#8203;array\[ rand 30 \] //= EXPR

where you can’t just evaluate the LHS expression twice and expect it to
work.

And because you *might* need to store a value, and you need an lvalue
for that, and you can only evaluate the expression once, then you *must*
evaluate it in lvalue context even before you know whether you need an
lvalue. This is simply inescapable.

The only way around this fact would be a deep change to the way that
lvalue context interacts with hashes, so that it wouldn’t just allocate
the key in order to refer to the value, but instead create an “intent”
to store a value in that key, which could go away unfulfilled. Then the
inescapable necessity of lvalue context would not preclude the behaviour
you ask for. But when you go to reason this out, you run into the fact
that several such intents for the same key can then coexist at the same
time (esp in Perl where lvalues can be passed around without losing
lvalueness), and that quickly leads you to various identity problems and
temporal paradoxes (because now one thing (the same hash key) can hide
behind multiple different intents) – all far more heady stuff than the
“surprise” you are complaining of.

The way it works now is really the only way it can sanely work.

That way may not be satisfying for some simple “obvious” cases where it
seems like it should clearly be able to do something smarter, such as
in your example. But in actual fact you would need to find and special-
case every one of these simple cases.

Regards,

Reviewing this discussion, my sense is that we're saying this is not a bug. If that's correct, I'd like to close the ticket.

Any disagreement with that?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 15, 2014

From @ikegami

Two observations​:

1. The operand evaluation order for C<< $x //= $y >> is not documented, so
the behaviour does not contradict any documentation.

2. A hash element as a parameter in a sub call is replaced with a magical
scalar in order to postpone the creation of the hash element until the
corresponding element of $_ is modified. I don't think it's warranted here.

perl -MDevel​::Peek -E"sub { say 0+keys(%h); Dump($_[0]) }->( $h{x} );"
0
SV = PVLV(0x4f2824) at 0xa79e6c
  REFCNT = 1
  FLAGS = (GMG,SMG)
  IV = 0
  NV = 0
  PV = 0
  MAGIC = 0x4c0864
  MG_VIRTUAL = &PL_vtbl_defelem
  MG_TYPE = PERL_MAGIC_defelem(y)
  MG_FLAGS = 0x02
  REFCOUNTED
  MG_OBJ = 0xa79fbc
  SV = PV(0xa7abb4) at 0xa79fbc
  REFCNT = 1
  FLAGS = (POK,pPOK)
  PV = 0xa7bdac "x"\0
  CUR = 1
  LEN = 12
  TYPE = y
  TARGOFF = 0
  TARGLEN = 1
  TARG = 0x4f1654
  FLAGS = 0
  SV = PVHV(0x4aa874) at 0x4f1654
  REFCNT = 2
  FLAGS = (OOK,SHAREKEYS)
  ARRAY = 0x4b92ac
  KEYS = 0
  FILL = 0
  MAX = 7
  RITER = -1
  EITER = 0x0
  RAND = 0x64547bb7

@p5pRT
Copy link
Author

p5pRT commented Sep 15, 2014

From [email protected]

On Mon, Sep 15, 2014 at 5​:26 AM, Eric Brine <ikegami@​adaelis.com> wrote​:

1. The operand evaluation order for C<< $x //= $y >> is not documented, so
the behaviour does not contradict any documentation.

  Doesn't the documentation imply it, though?

  The defined-or operator is documented to be short-circuiting, so the LHO
must be (fully) evaluated before the RHO.

  And the assignment operators are documented to be "without duplicating
any side effects that dereferencing the lvalue might trigger", so the LHO
must be (fully) evaluated just once.

  How could that possibly work in any other order?

Eirik

@p5pRT
Copy link
Author

p5pRT commented Sep 15, 2014

From @ikegami

On Mon, Sep 15, 2014 at 6​:43 AM, Eirik Berg Hanssen <
Eirik-Berg.Hanssen@​allverden.no> wrote​:

How could that possibly work in any other order?

The second observation I posted shows how it's not only possible, but it's
actually done elsewhere in Perl.

@p5pRT
Copy link
Author

p5pRT commented Sep 15, 2014

From [email protected]

On Mon, Sep 15, 2014 at 1​:59 PM, Eric Brine <ikegami@​adaelis.com> wrote​:

On Mon, Sep 15, 2014 at 6​:43 AM, Eirik Berg Hanssen <
Eirik-Berg.Hanssen@​allverden.no> wrote​:

How could that possibly work in any other order?

The second observation I posted shows how it's not only possible, but it's
actually done elsewhere in Perl.

  It shows how one could get around the problem, but it doesn't change the
evaluation order, does it?

  Well, okay, I guess it delays the hash lookup when the element does not
exist. Which it could safely do in this case, as non-existence is
sufficient for short-circuiting purposes ... hmm ...

my %j;
sub { $_[0] //= keys %j }->($j{bar});
say $j{bar};
__END__
0

  :-P

  Okay, I yield. :)

Eirik

@p5pRT
Copy link
Author

p5pRT commented Oct 22, 2014

@cpansprout - Status changed from 'open' to 'rejected'

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

No branches or pull requests

1 participant