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

META_REDUCE_RIGHT doesn't check for 1-arg case like META_REDUCE_LEFT #1705

Open
zoffixznet opened this issue Apr 7, 2018 · 2 comments
Open
Labels
metaop Meta operators

Comments

@zoffixznet
Copy link
Contributor

From IRC: https://irclog.perlgeek.de/perl6/2018-04-07#i_16017645

16:15 |   | m: sub infix:<@> ($a, $b) is assoc<left> { [$a, $b] }; say [@] 10
-- | -- | --
16:15 | camelia | rakudo-moar edbbc4426: OUTPUT: «10␤»
16:15 | cfa | m: sub infix:<@> ($a, $b) is assoc<right> { [$a, $b] }; say [@] 10
16:15 | camelia | rakudo-moar edbbc4426: OUTPUT: «Too few positionals passed; expected 2 arguments but got 1␤  in sub infix:<@> at <tmp> line 1␤  in block <unit> at <tmp> line 1␤␤»
16:16 | Zoffix | heh
16:16 | cfa | i'd expect either both to work or both to fail given the lack of supplied identity
16:16 |   | i'm probably missing something obvious
16:16 | Zoffix | Like this being a bug? :P


16:31 | TimToady | well, METAOP_REDUCE_LEFT checks for the 1-arg case, while METAOP_REDUCE_RIGHT doesn't
-- | -- | --
16:32 |   | there's not actually much call for the built-in right-associative operators in real code, so nobody ever noticed that before
16:32 | Zoffix | mhm
16:33 | TimToady | m: say infix:<**>(42)
16:33 | camelia | rakudo-moar edbbc4426: OUTPUT: «42␤»
16:33 |   | wamba joined #perl6
16:33 | TimToady | at least the support is there in that case
16:35 | Zoffix | Basically this https://github.com/rakudo/rakudo/blob/master/src/core/metaops.pm6#L196-L198 needs to be stuck here: https://github.com/rakudo/rakudo/blob/master/src/core/metaops.pm6#L393 (and maybe triangle cases need it too) if anyone wants to do it

Also does METAOP_REDUCE_CHAIN need something too? There's no 1-arg case for &infix:<(elem)> yet the 1-arg case returns True. Seems to make sense, but that supposed to be like that?

<Zoffix__> m: say &infix:<(elem)>.candidates».signature
<camelia> rakudo-moar edbbc4426: OUTPUT: «((Str:D $a, Map:D $b --> Bool) ($a, Map:D $b --> Bool) (Int:D $a, Range:D $b --> Bool) ($a, Iterable:D $b --> Bool) ($a, Iterator:D $b --> Bool) ($a, QuantHash:D $b --> Bool) ($, Failure:D $b) (Failure:D $a, $) ($a, $b))␤»
<Zoffix__> m: say [(elem)] 2334
<camelia> rakudo-moar edbbc4426: OUTPUT: «True␤»
@lucasbuchala lucasbuchala added the metaop Meta operators label Mar 15, 2019
@ab5tract
Copy link
Collaborator

Honestly this feels more like the METAOP_REDUCE_LEFT behavior is wrong:

> r 'sub infix:<@> ($a, $b) is assoc<left> { [$a*50, $b] }; say [@] 10'
10

Is it really more polite to just pretend that @ has been called, even when it clearly hasn't?

ab5tract added a commit that referenced this issue May 21, 2024
This aligns the behavior of `META_REDUCE_RIGHT` to that of
`META_REDUCE_LEFT` when handling 1-arg cases:

    > sub infix:<@> ($a, $b) is assoc<left> { [$a, $b] }
    > say [@] 10
    10

    > sub infix:<@> ($a, $b) is assoc<right> { [$a, $b] }
    > say [@] 10
    10

This addresses R#1705 (#1705).
ab5tract added a commit that referenced this issue May 21, 2024
This aligns the behavior of `META_REDUCE_RIGHT` to that of
`META_REDUCE_LEFT` when handling 1-arg cases:

    > sub infix:<@> ($a, $b) is assoc<left> { [$a, $b] }
    > say [@] 10
    10

    > sub infix:<@> ($a, $b) is assoc<right> { [$a, $b] }
    > say [@] 10
    10

This addresses R#1705 (#1705).
@niner
Copy link
Collaborator

niner commented May 24, 2024

I'm with @ab5tract on this one. I think METAOP_REDUCE_LEFT's magical handling is counter productive here. If you remove it, all the sensible cases still work, i.e. [+] 3 will still be 3 same as [**] 3 is 3 even now without METAOP_REDUCE_RIGHT doing anything special for the 1-value-case. The reason why these still work is that both operators have multi candidates with a single argument, i.e. they handle identity.

Any new operator should do the same. Guessing that identity will always be just the input value would lead to WAT situations like the examples posted in this issue demonstrate. Adding a multi infix:<@> ($a) is assoc<right> { [$a] } doesn't hurt and makes it abundantly clear, not just to the compiler but more importantly to the user as there's a special multi candidate hopefully carrying its own documentation.

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

No branches or pull requests

4 participants