Raku / old-issue-tracker

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

Reimplement List.min in Perl6 instead of PIR #754

Closed p6rt closed 15 years ago

p6rt commented 15 years ago

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

Searchable as RT63712$

p6rt commented 15 years ago

From @bacek


src/builtins/any-list.pir | 47 --------------------------------------------- src/setting/Any-list.pm | 20 ++++++++++++++++++- 2 files changed, 19 insertions(+), 48 deletions(-)

p6rt commented 15 years ago

From @bacek

059ec2c89dc266165efcee344f248218be567fe4.diff ```diff diff --git a/src/builtins/any-list.pir b/src/builtins/any-list.pir index b87666b..bc5cc9e 100644 --- a/src/builtins/any-list.pir +++ b/src/builtins/any-list.pir @@ -223,53 +223,6 @@ Return a List with the keys of the invocant. .return(res) .end -=item min - -=cut - -.namespace [] -.sub 'min' :multi() - .param pmc values :slurpy - .local pmc by - by = get_hll_global 'infix:cmp' - unless values goto have_by - $P0 = values[0] - $I0 = isa $P0, 'Sub' - unless $I0 goto have_by - by = shift values - have_by: - .tailcall values.'min'(by) -.end - - -.namespace ['Any'] -.sub 'min' :method :multi(_) - .param pmc by :optional - .param int has_by :opt_flag - if has_by goto have_by - by = get_hll_global 'infix:cmp' - have_by: - - .local pmc it, result - $P0 = self.'list'() - it = $P0.'iterator'() - unless it goto fail - result = shift it - loop: - unless it goto done - $P0 = shift it - $I0 = by($P0, result) - unless $I0 < 0 goto loop - result = $P0 - goto loop - fail: - .local num failres - failres = "+Inf" - .return (failres) - done: - .return (result) -.end - .namespace [] .sub 'max' :multi() diff --git a/src/setting/Any-list.pm b/src/setting/Any-list.pm index 5758dce..0aaeccc 100644 --- a/src/setting/Any-list.pm +++ b/src/setting/Any-list.pm @@ -23,7 +23,20 @@ class Any is also { $res = &$expression($res, |@args); } $res; - } + }; + + # RT #63700 - parse failed on &infix: + our Array multi method min( $values: Code $by = sub { $^a cmp $^b } ) { + my @list = $values.list; + return +Inf unless @list.elems; + my $res = @list.shift; + for @list -> $x { + if (&$by($res, $x) > 0) { + $res = $x; + } + } + $res; + }; } our List multi grep(Code $test, *@values) { @@ -34,4 +47,9 @@ multi reduce ( Code $expression ;; *@values ) { @values.reduce($expression); } +our List multi min(*@values) { + my $by = @values[0] ~~ Code ?? shift @values !! sub { $^a cmp $^b }; + @values.min($by); +} + # vim: ft=perl6 ```
p6rt commented 15 years ago

From @moritz

On Sat Mar 07 21​:50​:00 2009, bacek wrote​:

--- src/builtins/any-list.pir | 47 --------------------------------------------- src/setting/Any-list.pm | 20 ++++++++++++++++++- 2 files changed, 19 insertions(+), 48 deletions(-)

Applied as a7214ac28c5c7c47932f1e76a15c8707524f964d, thanks.

Moritz

p6rt commented 15 years ago

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

p6rt commented 15 years ago

@moritz - Status changed from 'open' to 'resolved'

p6rt commented 15 years ago

@pmichaud - Status changed from 'resolved' to 'open'

p6rt commented 15 years ago

From @pmichaud

On Sat, Mar 07, 2009 at 09​:50​:02PM -0800, Vasily Chekalkin wrote​:

+our List multi min(*@​values) { + my $by = @​values[0] ~~ Code ?? shift @​values !! sub { $^a cmp $^b }; + @​values.min($by); +}

This doesn't match the spec -- the $by parameter is required. At any rate, the first argument should not be explicitly checked for being a Code object -- if we do this it should be via a multi signature.

Yes, the PIR code was "cheating" in this respect, but I don't want to blindly copy our cheats from PIR into the setting without subjecting them to another review.

+ # RT #​63700 - parse failed on &infix​:\ + our Array multi method min( $values​: Code $by = sub { $^a cmp $^b } ) { + my @​list = $values.list; + return +Inf unless @​list.elems; + my $res = @​list.shift; + for @​list -> $x { + if (&$by($res, $x) > 0) { + $res = $x; + } + } + $res; + }; }

Why are C\ and C\ specced as returning C\ and C\? Shouldn't they just return the single element that is the minimum or maximum from the list?

Pm

p6rt commented 15 years ago

From @moritz

Patrick R. Michaud wrote​:

On Sat, Mar 07, 2009 at 09​:50​:02PM -0800, Vasily Chekalkin wrote​:

+our List multi min(*@​values) { + my $by = @​values[0] ~~ Code ?? shift @​values !! sub { $^a cmp $^b }; + @​values.min($by); +}

This doesn't match the spec -- the $by parameter is required. At any rate, the first argument should not be explicitly checked for being a Code object -- if we do this it should be via a multi signature.

I've fixed that, and corrected the spec tests accordingly.

Yes, the PIR code was "cheating" in this respect, but I don't want to blindly copy our cheats from PIR into the setting without subjecting them to another review.

+ # RT #​63700 - parse failed on &infix​:\ + our Array multi method min( $values​: Code $by = sub { $^a cmp $^b } ) { + my @​list = $values.list; + return +Inf unless @​list.elems; + my $res = @​list.shift; + for @​list -> $x { + if (&$by($res, $x) > 0) { + $res = $x; + } + } + $res; + }; }

Why are C\ and C\ specced as returning C\ and C\? Shouldn't they just return the single element that is the minimum or maximum from the list?

I agree, so I removed the non-sense return types from the specs.

Cheers, Moritz

p6rt commented 15 years ago

From @bacek

Patrick R. Michaud via RT wrote​:

On Sat, Mar 07, 2009 at 09​:50​:02PM -0800, Vasily Chekalkin wrote​:

+our List multi min(*@​values) { + my $by = @​values[0] ~~ Code ?? shift @​values !! sub { $^a cmp $^b }; + @​values.min($by); +}

This doesn't match the spec -- the $by parameter is required. At any rate, the first argument should not be explicitly checked for being a Code object -- if we do this it should be via a multi signature.

Yes, the PIR code was "cheating" in this respect, but I don't want to blindly copy our cheats from PIR into the setting without subjecting them to another review.

This is workaround for this situation​:

\ rakudo​: multi sub foo(Code &x, *@​values) {...}; multi sub foo(*@​values) {...}; say foo(sub {}, 1,2,3); \ rakudo 2daf6b​: OUTPUT«Ambiguous dispatch to multi 'foo'. Ambiguous candidates had signatures​:␤​:(Code x, Any @​values)␤​:(Any @​values)␤␤current instr.​: '_block14' pc 101 (EVAL_18​:52)␤»

Should I fill bug report for this?

-- Bacek

p6rt commented 15 years ago

From @pmichaud

On Mon Mar 09 00​:41​:24 2009, bacek wrote​:

Patrick R. Michaud via RT wrote​:

At any rate, the first argument should not be explicitly checked for being a Code object -- if we do this it should be via a multi signature.

Yes, the PIR code was "cheating" in this respect, but I don't want to blindly copy our cheats from PIR into the setting without subjecting them to another review.

This is workaround for this situation​:

\ rakudo​: multi sub foo(Code &x, *@​values) {...}; multi sub foo(*@​values) {...}; say foo(sub {}, 1,2,3); \ rakudo 2daf6b​: OUTPUT«Ambiguous dispatch to multi 'foo'. Ambiguous candidates had signatures​:␤​:(Code x, Any @​values)␤​:(Any @​values)␤␤current instr.​: '_block14' pc 101 (EVAL_18​:52)␤»

Should I fill bug report for this?

Yes please. Closing this ticket, thanks!

Pm

p6rt commented 15 years ago

@pmichaud - Status changed from 'open' to 'resolved'