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

Test.pm6 subs need to avoid containerizing arguments #1771

Open
zoffixznet opened this issue Apr 25, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@zoffixznet
Copy link
Contributor

commented Apr 25, 2018

Because it affects how stuff is matched. is-deeply is one of such subs; I'm guessing cmp-ok is similarly affected and basically all of the routines in Test.pm6 need to be audited for this issue:

11:26 | lizmat | m: use Test; my %h = a => Mu; is-deeply %h, %h  # disappointing  :-(
-- | -- | --
11:26 | camelia | rakudo-moar 79ed89ba4: OUTPUT: «Type check failed in binding to parameter '<anon>'; expected Any but got Mu (Mu)␤  in sub _is_deeply at /home/camelia/rakudo-m-inst-1/share/perl6/sources/C712FE6969F786C9380D643DF17E85D06868219E (Test) line 651␤  in sub is-deeply at /home/camelia/…»

11:29 |   | m: my %h = a => Mu; say %h eqv %h
-- | -- | --
11:29 | camelia | rakudo-moar 79ed89ba4: OUTPUT: «True␤»
11:29 | Zoffix | huh
11:32 |   | m: my %h = a => Mu; say $%h eqv $%h
11:32 | camelia | rakudo-moar 79ed89ba4: OUTPUT: «Type check failed in binding to parameter '<anon>'; expected Any but got Mu (Mu)␤  in block <unit> at <tmp> line 1␤␤»
11:33 | Zoffix | s: &infix:<eqv>, do { my %h = a => Mu; \(%h, %h)}
11:33 | SourceBaby | Zoffix, Sauce is at https://github.com/rakudo/rakudo/blob/79ed89ba4/src/core/Map.pm6#L446
11:33 | Zoffix | s: &infix:<eqv>, do { my %h = a => Mu; \($%h, $%h)}
11:33 | SourceBaby | Zoffix, Sauce is at https://github.com/rakudo/rakudo/blob/79ed89ba4/src/core/Map.pm6#L446
11:33 | Zoffix | Ah, the non-conted version goes through the eqaddr fast-past

The fix in most cases would be just changing $foo param to \foo or adding is raw trait if sigil has to be kept.

@jnthn

This comment has been minimized.

Copy link
Member

commented Apr 25, 2018

I suspect this will not be so simple, because people will have relied on top-level containerization not mattering when writing tests with is-deeply. (How do I know? Because I've got such tests.)

@zoffixznet

This comment has been minimized.

Copy link
Contributor Author

commented Apr 25, 2018

is-deeply is becoming a monster 😃 It already does different things than eqv with Seqs. Now containerization is in play.

I can argue for making it stay the way it is too: the eqaddr fast-path in OP code hides the explosion due to Mus that would occur anytime first arg is not the same object as the second arg.

@jnthn

This comment has been minimized.

Copy link
Member

commented Apr 25, 2018

To be clear, I'm not (yet 😀) arguing for any particular behavior (current or different), just noting that we may well quite a lot of reliance on the current behavior. Either way, we want to nail down and document want the semantics are, though. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.