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

Autogenerated signature does not set multi-invocant bit #4350

Closed
p6rt opened this issue Jun 25, 2015 · 5 comments
Closed

Autogenerated signature does not set multi-invocant bit #4350

p6rt opened this issue Jun 25, 2015 · 5 comments

Comments

@p6rt
Copy link

p6rt commented Jun 25, 2015

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

Searchable as RT125486$

@p6rt
Copy link
Author

p6rt commented Jun 25, 2015

From @lizmat

[13​:42​:39] <lizmat> There is a subtle difference in the signature of "sub a { @​_ }" and sub a(*@​_) { }" that was so far unseen​:
[13​:42​:41] <lizmat> $ 6 '(sub a { @​_ }).signature.params[0].multi-invocant.say'
[13​:42​:41] <lizmat> False
[13​:42​:50] <lizmat> $ 6 '(sub a(*@​_) { @​_ }).signature.params[0].multi-invocant.say'
[13​:42​:50] <lizmat> True
[13​:44​:00] <lizmat> jnthn masak​: should @​_ have the multi-invocant bit set or not (in either case?)
[13​:46​:19] <lizmat> alternately​: should slurpies ever have the multi-invocant bit set ?
[13​:47​:11] <masak> I don't really see why it should.
[13​:47​:22] <masak> but I don't know exactly what multi-invocant is.
[13​:47​:58] <+dalek> rakudo/nom​: f43725a | lizmat++ | src/core/Signature.pm​:
[13​:47​:58] <+dalek> rakudo/nom​: Show ';;' in signatures if they need to be
[13​:47​:58] <+dalek> rakudo/nom​:
[13​:47​:58] <+dalek> rakudo/nom​: Fixes #​125482
[13​:47​:58] <+dalek> rakudo/nom​: review​: rakudo/rakudo@f43725a038
[13​:48​:14] atroxaper (~atroxaper@​mail.aurus5.ru) left IRC. (Remote host closed the connection)
[13​:48​:28] <lizmat> jnthn just said that params with that bit set, are included in MMD?
[13​:48​:37] <masak> m​: say (sub a(*@​_) { @​_ }).signature.params[0].WHAT
[13​:48​:37] <+camelia> rakudo-moar 2bafa9​: OUTPUT«(Parameter)␤»
[13​:49​:14] salva (~salva@​213.37.131.197.static.user.ono.com) joined the channel.
[13​:49​:20] <lizmat> hmmm... musty have inferred that
[13​:49​:35] atroxaper (~atroxaper@​mail.aurus5.ru) joined the channel.
[13​:49​:37] atroxaper (~atroxaper@​mail.aurus5.ru) left IRC. (Remote host closed the connection)
[13​:50​:11] <lizmat> anyway, the fix causes 3 spectest failures because of difference in handling auto-generated sigs
[13​:50​:17] <masak> it's odd. I don't see `multi-invocant` defined or referred to anywhere.
[13​:50​:32] <lizmat> it's probably an implementation detail
[13​:50​:50] <lizmat> I just added the multi-invocant method
[13​:51​:01] <lizmat> (if you're looking at the source)
[13​:51​:31] <lizmat> t/spec/S06-signature/unspecified.rakudo.moar (Wstat​: 768 Tests​: 17 Failed​: 3)
[13​:51​:31] <lizmat> Failed tests​: 5, 9, 13
[13​:52​:02] <lizmat> feels to me either the test is wrong (now), or having the flag set on auto-generated sigs is wrong
[13​:52​:36] <lizmat> masak​: should there be a difference between the sig of "sub a { @​_ }" and "sub a(*@​_) { }" ?
[13​:53​:39] <jnthn> lizmat​: I suspect it should probably be set on anything that isn't after a ;;
[13​:54​:14] <lizmat> ?? I thought you just said it should be set on anything *before* ;; ?
[13​:54​:27] <lizmat> ah, uh. duh :-)
[13​:54​:46] <jnthn> I...yeah, negations are not unbad. :)
[13​:55​:12] <lizmat> so the autogenned sig is wrong ?
[13​:55​:18] masak .oO( but what about the excluded middle? what about parameters that are exactly on top of the ;; ? ) :P
[13​:56​:00] <jnthn> lizmat​: Yeah, that's probably just an oversight
[13​:56​:27] <lizmat> should I create a ticket for that and todo the failing tests?
[13​:56​:51] <lizmat> or can you give me a pointer as to where to look to fix this oversight ?
[13​:56​:51] <jnthn> Yeah, please. I think there'll be a good way to fix it generally
[13​:57​:08] <jnthn> Well, probably in World.nqp's create_parameter method
[13​:57​:16] <lizmat> ah, ok, lemme check
[13​:57​:34] <jnthn> I think if the param info hash doesn't have the multi_invocant key we should default it to setting the bit, whereas today we probably default to not doing so
[13​:57​:46] <jnthn> You may be able to eliminate places that actions.nqp sets it as a result of this...
[13​:57​:59] <jnthn> (expect the place that sets it to 0)
[14​:03​:04] <timotimo> do we now get a ;; before every *%_?
[14​:03​:15] <jnthn> shouldn't
[14​:03​:27] <timotimo> it would be kind of nice to have that hint for people who tend to forget about nameds only being used for tie-breaking
[14​:03​:32] <jnthn> Well, we will if the bug isn't fixed...
[14​:03​:36] <timotimo> you know, the thing where the order of subs decides
[14​:04​:55] <lizmat> $ 6 '(sub a(​:$n) { }).signature.params[0].multi-invocant.perl.say'
[14​:04​:55] <lizmat> Bool​::True
[14​:05​:22] <jnthn> The language in the spec suggests that maybe, post-6.0, we will have nameds play into the candidate sort
[14​:05​:29] <timotimo> oh
[14​:05​:31] <timotimo> i see
[14​:05​:38] <lizmat> so any named param seem to have the multi-invocant bit set atm
[14​:05​:48] <jnthn> I'm not sure how viable that is, but I'd rather keep .multi-invocant only ever show up after a ;;
[14​:05​:57] <jnthn> That is, keep it aligned with what the programmer wrote
[14​:06​:28] <masak> +1
[14​:06​:50] <masak> simple rule​: "was there a ;; before this parameter?"
[14​:07​:06] <jnthn> aye
[14​:07​:13] <timotimo> fair enough
[14​:07​:17] <jnthn> I like simple rules
[14​:07​:20] <lizmat> $ 6 '(sub a($a;;​:$n;) { }).signature.perl.say'
[14​:07​:20] <lizmat> :(Any $a;; Any :n($n))
[14​:07​:20] <jnthn> ...and I can not lie
[14​:07​:51] <masak> jnthn​: doesn't preserve the meter. you have to like big rules or something :P
[14​:08​:28] <lizmat> so​: the current rule is​: all params have multi-invocant bit set unless there was a ';;' seen, in which case only the params before the ;; have it
[14​:08​:30] masak .oO( I like excessively bending the meter of original works and I cannot lie )
[14​:09​:03] <masak> lizmat​: or, shorter, all params not after a ;; have it

@p6rt
Copy link
Author

p6rt commented Jun 27, 2015

From @lizmat

Fixed with 1a743f9d756a314143

On 25 Jun 2015, at 14​:23, Elizabeth Mattijsen (via RT) <perl6-bugs-followup@​perl.org> wrote​:

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

[13​:42​:39] <lizmat> There is a subtle difference in the signature of "sub a { @​_ }" and sub a(*@​_) { }" that was so far unseen​:
[13​:42​:41] <lizmat> $ 6 '(sub a { @​_ }).signature.params[0].multi-invocant.say'
[13​:42​:41] <lizmat> False
[13​:42​:50] <lizmat> $ 6 '(sub a(*@​_) { @​_ }).signature.params[0].multi-invocant.say'
[13​:42​:50] <lizmat> True
[13​:44​:00] <lizmat> jnthn masak​: should @​_ have the multi-invocant bit set or not (in either case?)
[13​:46​:19] <lizmat> alternately​: should slurpies ever have the multi-invocant bit set ?
[13​:47​:11] <masak> I don't really see why it should.
[13​:47​:22] <masak> but I don't know exactly what multi-invocant is.
[13​:47​:58] <+dalek> rakudo/nom​: f43725a | lizmat++ | src/core/Signature.pm​:
[13​:47​:58] <+dalek> rakudo/nom​: Show ';;' in signatures if they need to be
[13​:47​:58] <+dalek> rakudo/nom​:
[13​:47​:58] <+dalek> rakudo/nom​: Fixes #​125482
[13​:47​:58] <+dalek> rakudo/nom​: review​: rakudo/rakudo@f43725a038
[13​:48​:14] atroxaper (~atroxaper@​mail.aurus5.ru) left IRC. (Remote host closed the connection)
[13​:48​:28] <lizmat> jnthn just said that params with that bit set, are included in MMD?
[13​:48​:37] <masak> m​: say (sub a(*@​_) { @​_ }).signature.params[0].WHAT
[13​:48​:37] <+camelia> rakudo-moar 2bafa9​: OUTPUT«(Parameter)␤»
[13​:49​:14] salva (~salva@​213.37.131.197.static.user.ono.com) joined the channel.
[13​:49​:20] <lizmat> hmmm... musty have inferred that
[13​:49​:35] atroxaper (~atroxaper@​mail.aurus5.ru) joined the channel.
[13​:49​:37] atroxaper (~atroxaper@​mail.aurus5.ru) left IRC. (Remote host closed the connection)
[13​:50​:11] <lizmat> anyway, the fix causes 3 spectest failures because of difference in handling auto-generated sigs
[13​:50​:17] <masak> it's odd. I don't see `multi-invocant` defined or referred to anywhere.
[13​:50​:32] <lizmat> it's probably an implementation detail
[13​:50​:50] <lizmat> I just added the multi-invocant method
[13​:51​:01] <lizmat> (if you're looking at the source)
[13​:51​:31] <lizmat> t/spec/S06-signature/unspecified.rakudo.moar (Wstat​: 768 Tests​: 17 Failed​: 3)
[13​:51​:31] <lizmat> Failed tests​: 5, 9, 13
[13​:52​:02] <lizmat> feels to me either the test is wrong (now), or having the flag set on auto-generated sigs is wrong
[13​:52​:36] <lizmat> masak​: should there be a difference between the sig of "sub a { @​_ }" and "sub a(*@​_) { }" ?
[13​:53​:39] <jnthn> lizmat​: I suspect it should probably be set on anything that isn't after a ;;
[13​:54​:14] <lizmat> ?? I thought you just said it should be set on anything *before* ;; ?
[13​:54​:27] <lizmat> ah, uh. duh :-)
[13​:54​:46] <jnthn> I...yeah, negations are not unbad. :)
[13​:55​:12] <lizmat> so the autogenned sig is wrong ?
[13​:55​:18] masak .oO( but what about the excluded middle? what about parameters that are exactly on top of the ;; ? ) :P
[13​:56​:00] <jnthn> lizmat​: Yeah, that's probably just an oversight
[13​:56​:27] <lizmat> should I create a ticket for that and todo the failing tests?
[13​:56​:51] <lizmat> or can you give me a pointer as to where to look to fix this oversight ?
[13​:56​:51] <jnthn> Yeah, please. I think there'll be a good way to fix it generally
[13​:57​:08] <jnthn> Well, probably in World.nqp's create_parameter method
[13​:57​:16] <lizmat> ah, ok, lemme check
[13​:57​:34] <jnthn> I think if the param info hash doesn't have the multi_invocant key we should default it to setting the bit, whereas today we probably default to not doing so
[13​:57​:46] <jnthn> You may be able to eliminate places that actions.nqp sets it as a result of this...
[13​:57​:59] <jnthn> (expect the place that sets it to 0)
[14​:03​:04] <timotimo> do we now get a ;; before every *%_?
[14​:03​:15] <jnthn> shouldn't
[14​:03​:27] <timotimo> it would be kind of nice to have that hint for people who tend to forget about nameds only being used for tie-breaking
[14​:03​:32] <jnthn> Well, we will if the bug isn't fixed...
[14​:03​:36] <timotimo> you know, the thing where the order of subs decides
[14​:04​:55] <lizmat> $ 6 '(sub a(​:$n) { }).signature.params[0].multi-invocant.perl.say'
[14​:04​:55] <lizmat> Bool​::True
[14​:05​:22] <jnthn> The language in the spec suggests that maybe, post-6.0, we will have nameds play into the candidate sort
[14​:05​:29] <timotimo> oh
[14​:05​:31] <timotimo> i see
[14​:05​:38] <lizmat> so any named param seem to have the multi-invocant bit set atm
[14​:05​:48] <jnthn> I'm not sure how viable that is, but I'd rather keep .multi-invocant only ever show up after a ;;
[14​:05​:57] <jnthn> That is, keep it aligned with what the programmer wrote
[14​:06​:28] <masak> +1
[14​:06​:50] <masak> simple rule​: "was there a ;; before this parameter?"
[14​:07​:06] <jnthn> aye
[14​:07​:13] <timotimo> fair enough
[14​:07​:17] <jnthn> I like simple rules
[14​:07​:20] <lizmat> $ 6 '(sub a($a;;​:$n;) { }).signature.perl.say'
[14​:07​:20] <lizmat> :(Any $a;; Any :n($n))
[14​:07​:20] <jnthn> ...and I can not lie
[14​:07​:51] <masak> jnthn​: doesn't preserve the meter. you have to like big rules or something :P
[14​:08​:28] <lizmat> so​: the current rule is​: all params have multi-invocant bit set unless there was a ';;' seen, in which case only the params before the ;; have it
[14​:08​:30] masak .oO( I like excessively bending the meter of original works and I cannot lie )
[14​:09​:03] <masak> lizmat​: or, shorter, all params not after a ;; have it

@p6rt
Copy link
Author

p6rt commented Sep 13, 2015

From @usev6

AFAIU there are already tests for the multi-invocant bit in S06-signature/multi-invocant.t (lizmat++).

I mentioned the ticket number in one of the tests and I'm closing this ticket as 'resolved'.

1 similar comment
@p6rt
Copy link
Author

p6rt commented Sep 13, 2015

From @usev6

AFAIU there are already tests for the multi-invocant bit in S06-signature/multi-invocant.t (lizmat++).

I mentioned the ticket number in one of the tests and I'm closing this ticket as 'resolved'.

@p6rt
Copy link
Author

p6rt commented Sep 13, 2015

@usev6 - Status changed from 'new' to 'resolved'

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

No branches or pull requests

1 participant