Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lock.protect({}) fails, but with surprising message #1644

Closed
robertlemmen opened this issue Mar 23, 2018 · 13 comments
Closed

Lock.protect({}) fails, but with surprising message #1644

robertlemmen opened this issue Mar 23, 2018 · 13 comments

Comments

@robertlemmen
Copy link

I had some code using Lock.protect():

  my $l = Lock.new; $l.protect({ say "woo"; });

Commenting out some code during development left me with an "empty block":

  my $l = Lock.new; $l.protect({});

Which resulted in an error message like this:

  Attempt to unlock mutex by thread not holding it
  in block <unit> at <unknown file> line 1

I agree that this is an error, because what I intended to be an empty block is in fact a hash literal. However the error message is a little bit less than awesome. and surprising, given:

  my sub t(&c) { c(); }; t({});

results in:

  Type check failed in binding to parameter '&c'; expected Callable but got Hash (${})
    in sub t at <unknown file> line 1
    in block <unit> at <unknown file> line 1

Not sure what could be special about Lock.protect...

@lizmat
Copy link
Contributor

lizmat commented Mar 23, 2018

$ perl6 -e 'class A { method a(A:D: &a) { LEAVE die } }; A.new.a({})'
Died

Apparently the LEAVE phaser is executed even though the method itself never got passed parameter binding

@AlexDaniel
Copy link
Contributor

FWIW, I hate this. I keep getting into this trap. I guess the only way to have an empty block is to do {;}, or maybe even {…} if you're sure that it's not going to be executed, but I'll never remember that. The whole idea of using {} for hashes is a p5-ism I think, and IIRC there is already some slight discouragement for {} in favor of %().

Maybe we should tweak the language instead? Not necessarily now, but 6.d or 6.e…

@zoffixznet
Copy link
Contributor

zoffixznet commented Mar 26, 2018

Apparently the LEAVE phaser is executed even though the method itself never got passed parameter binding

That's as-designed. The binding is attempted inside the sub's block so LEAVE gets run when we leave the block after binding failure (this is documented in traps too)

using {} for hashes is a p5-ism I think,

Also, Ruby, Python, and JS/JSON (objects are very similar to hashes).

Maybe we should tweak the language instead? Not necessarily now, but 6.d or 6.e…

👎 just based on the sheer amount of { } use for hashes, including :foo{ ... } colonpair shorthand, which would have to go or new syntax be invented to support colonpair hash shortcuts. That's just too much user impact for very little gain (I had no trouble learning which syntax makes what, and I don't think I'm that special).

Also %() is less convenient to type, compared to {} (requires use of two hands instead of just one, at least on my keyboard layout) and doesn't offer auto-statement-end semantics.

@vendethiel
Copy link
Contributor

@zoffixznet I agree with your post, though there might be an argument to be made that in ruby/python/js it’s not actually an issue, since the lambda syntax is ->{}, lambda: and ()=>{}. (I don’t know of any language which uses the same syntax for hashes and lambdas?)

@zoffixznet
Copy link
Contributor

-> {} is a lambda in Perl 6 as well. As is {;}. It's not the same syntax for lambdas and hashes and the rules are well-defined.

The argument might've been made when the language was being designed. Right now, making such a significant change would be just pissing off—and losing—users.

@lizmat
Copy link
Contributor

lizmat commented Mar 26, 2018 via email

@jnthn
Copy link
Member

jnthn commented Mar 26, 2018

Attempt to unlock mutex by thread not holding it
in block at line 1

We can do better than this by setting a flag saying we did indeed acquire the lock, and only releasing it if we actually did. It's actually a potentially more dangerous bug when one considers lock recursion, since then it could fail silently in the immediate, and then at a distance later. I'll reopen this issue now, and fix it up.

Apparently the LEAVE phaser is executed even though the method itself never got passed parameter binding

Yes, and this is one place where there's no really right answer, given that one can do things like:

sub process(IO::Handle $h = open $*DEAULT-PATH) {
    LEAVE $h.close
}

And then it'd be a trap in the other direction.

The argument might've been made when the language was being designed.

It was. In fact, it's a good default assumption that - having had 15 years of design time - nearly all aspects of the Perl 6 design received no small amount of discussion before settling on the current state. Given how many ways in which Perl 6 differs from Perl 5, it's also a reasonable default assumption that those designed choices made the same way are intentionally that way, also after considering the options.

@jnthn jnthn reopened this Mar 26, 2018
@zoffixznet
Copy link
Contributor

zoffixznet commented Mar 26, 2018

Attempt to unlock mutex by thread not holding it
in block at line 1

We can do better than this

FWIW, after db7361a the error for OP's version of the code is Cannot resolve caller protect(Lock: Hash); none of these signatures match:␤ (Lock:D $: &code, *%_)

@jnthn
Copy link
Member

jnthn commented Mar 26, 2018

FWIW, after db7361a the error for OP's version of the code is Cannot resolve caller protect(Lock: Hash); none of these signatures match:␤ (Lock:D $: &code, *%_)

Yeah, that's a decent way to do it too; the text linking the commit is pretty tiny and I missed it. I'm doing a similar fix for Lock::Async.protect also.

@jnthn
Copy link
Member

jnthn commented Mar 26, 2018

Fixed for Lock::Async.protect to in 299dc6fc55.

@jnthn jnthn closed this as completed Mar 26, 2018
@AlexDaniel
Copy link
Contributor

The argument might've been made when the language was being designed.

It was. In fact, it's a good default assumption that - having had 15 years of design time - nearly all aspects of the Perl 6 design received no small amount of discussion before settling on the current state. Given how many ways in which Perl 6 differs from Perl 5, it's also a reasonable default assumption that those designed choices made the same way are intentionally that way, also after considering the options.

Sorry but this thread makes no sense to me.

  • Even if something was decided one way or another, doesn't mean it's right. We now have more information, and arguably we have slightly different goals now. Some things are there because of perl5, but the weight of this kind of reasoning is diminishing over time. Does it mean that we'll eventually rethink some fundamental design choices? According to this thread – no, because we're religiously married to choices that we made in the past. In reality – maybe! (you never know).
  • So you can still make arguments about language design. Some of the suggested changes are worth pursuing in the long run, some not. And that's the question.

I think there's little use in naysaying, and instead we can just let the interested parties to think and discuss what would be a better way to do something. If it's different from what we actually have in perl 6, it can then be discussed if something can be changed (and if it's worth changing at all).


Anyway. I'm probably stealing this ticket and driving it full off topic, but another way of thinking about the { } issue is in terms of detrapping the language. So let's say we keep { } for hashes because @zoffixznet's reasoning is compelling (it really is), but this doesn't make the trap go away. What if, for example, we give a warning for {}? Something like { } is ambiguous, use {;} to mean an empty Block or %() to mean an empty Hash. Looking at the ecosystem, both {} and %() are used:

With this info I really don't know if it's worth changing (more like not). But… who knows? ;)

@AlexDaniel
Copy link
Contributor

Links:

@AlexDaniel
Copy link
Contributor

Another one:

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

6 participants