Raku / old-issue-tracker

Tickets from RT
https://github.com/Raku/old-issue-tracker/issues
2 stars 1 forks source link

`but role { ... }` interacts strangely with outer lexicals, and doesn't warn about it #6139

Open p6rt opened 7 years ago

p6rt commented 7 years ago

Migrated from rt.perl.org#130965 (status was 'open')

Searchable as RT130965$

p6rt commented 7 years ago

From @smls

  sub a ($a) {   return True but role {   method foo { $a }   }   }

  my $x = a 42;   say $x.foo; # 42

  my $y = a "Hello";   say $y.foo; # Hello   say $x.foo; # Hello

I have a suspicion that this is not a bug but rather a consequence of classes/roles not being proper closures.

But the current behavior is rather LTA in cases like this. Could this possibly be made to emit a warning?

p6rt commented 7 years ago

From @lizmat

Yes, this is because classes / roles are not proper closures. And yes, I’ve been bitten by this as well.

To me, making them proper closures, feels like the correct solution to me. But I’ll settle for a warning / exceptione :-)

On 9 Mar 2017, at 16​:35, Sam S. (via RT) \perl6\-bugs\-followup@​perl\.org wrote​:

# New Ticket Created by Sam S. # Please include the string​: [perl #​130965] # in the subject line of all future correspondence about this issue. # \<URL​: https://rt-archive.perl.org/perl6/Ticket/Display.html?id=130965 >

sub a ($a) { return True but role { method foo { $a } } }

my $x = a 42; say $x.foo; # 42

my $y = a "Hello"; say $y.foo; # Hello say $x.foo; # Hello

I have a suspicion that this is not a bug but rather a consequence of classes/roles not being proper closures.

But the current behavior is rather LTA in cases like this. Could this possibly be made to emit a warning?

p6rt commented 7 years ago

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

p6rt commented 7 years ago

From @jnthn

On Thu, 09 Mar 2017 07​:39​:39 -0800, elizabeth wrote​:

Yes, this is because classes / roles are not proper closures. And yes, I’ve been bitten by this as well.

It boils down to roles and classes being compile-time constructs, while closures are decidedly runtime ones. You can force a role to be re-evaluated each time by making it parametric, and passing the state as a role parameter. Note, however, that roles are interned based on the object identity of their positional parameters. This means the pattern, used on values, has a high chance of causing a memory "leak".

Note that the `but RoleName($value)` syntax that mixes in the role and, if it has a single attribute, assigns $value to it, is there to make cases like this a bit more convenient.

To me, making them proper closures, feels like the correct solution to me. For this to happen we'd need​:

* To clone the methods, attribute initialization closures, etc. * To therefore have a cloned method table, Attribute objects, etc. to point to these clones * Which implies a cloned meta-object * Which in turn forces a new type table (which raises a bunch of questions about what .WHAT evaluates to) * And the new type, formed per closed over set of values, will then miss on every single bit of our dynamic optimization infrastructure that keys on type (method caches, multi caches, specializations, etc.)

Declaring lexical classes and roles isn't an entirely unusual practice; without some careful analysis of when we need to do the cloning, we'd end up giving programs using this pattern a huge performance regression. All to give other programs a feature that will be hard to optimize, and so will then get a (deserved) reputation for being slow and thus probably not used all that much anyway.

But I’ll settle for a warning / exceptione :-)

I think these options are worth consideration. I can't think of a false positive off hand.

/jnthn

p6rt commented 7 years ago

From @timo

On 09/03/17 20​:38, jnthn@​jnthn.net via RT wrote​:

On Thu, 09 Mar 2017 07​:39​:39 -0800, elizabeth wrote​:

But I’ll settle for a warning / exceptione :-) I think these options are worth consideration. I can't think of a false positive off hand.

class Test { method tester { say "testest" } }

# warning​: you're not allowed to close over variable &say!

is there a sane heuristic for this? "does it have a & sigil" isn't sane, IMO, and neither is "does it come from the core setting?".

p6rt commented 6 years ago

From j.david.lowe@apple.com

This short program behaves dies on my system, apparently because the mixed-in attribute is somehow "leaking" between the two threads (???)​:

``` #!/usr/bin/env perl6

await((^2).map​: {   start {   for (^10) -> $i {   my $x = Str.new but role { has $.test = $i };   die "$i != " ~ $x.test unless $x.test == $i;   }   } }); ```

produces output like​:

``` $ ./bug Tried to get the result of a broken Promise   in block \ at ./bug line 3

Original exception​:   1 != 2   in block at ./bug line 7

$ ./bug Tried to get the result of a broken Promise   in block \ at ./bug line 3

Original exception​:   3 != 4   in block at ./bug line 7 ```

It's possible that what I'm doing in this code isn't *intended* to be thread-safe, but if that's the case, I don't understand why (e.g. looking at the code, the two threads appear to be completely isolated.)

More information​:

``` $ perl6 --version This is Rakudo version 2017.07 built on MoarVM version 2017.07 implementing Perl 6.c. ```

p6rt commented 6 years ago

From @smls

@​David

I think this is not a thread safety issue, but rather a result of `role`s (intentionally) not being closures.

I.e. a duplicate of this RT​: This is a duplicate of https://rt-archive.perl.org/perl6/Ticket/Display.html?id=130965

p6rt commented 6 years ago

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

p6rt commented 6 years ago

From j.david.lowe@apple.com

Agreed. In that case, the threads are just racing to observe a mutating value while it's in the "expected" state.

I will add my voice to the chorus calling for either making roles into closures, or producing a compile-time warning or error. It's tempting to use roles to "monkey patch", and would be good to be warned if it isn't a safe thing to do.

I'm fine with closing this as a duplicate.

On Aug 29, 2017, at 7​:32 AM, Sam S. via RT \perl6\-bugs\-followup@&#8203;perl\.org wrote​:

@​David

I think this is not a thread safety issue, but rather a result of `role`s (intentionally) not being closures.

I.e. a duplicate of this RT​: This is a duplicate of https://rt-archive.perl.org/perl6/Ticket/Display.html?id=130965