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

Set.perl inconsistent with Bag/Mix.perl #1958

Closed
zoffixznet opened this Issue Jun 24, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@zoffixznet
Contributor

zoffixznet commented Jun 24, 2018

The bag and mix get .perl in a form of bag()/mix(), but set gets dumped as either ().Set or set() (kinda surprised Set.new and set() get dumped differently, given they compare === as True)

<Zoffix__> m: .perl.say for Set.new, Bag.new, Mix.new
<camelia> rakudo-moar e9351cbaa: OUTPUT: «().Set␤bag()␤mix()␤»
<Zoffix__> m: .perl.say for set(), bag(), mix()
<camelia> rakudo-moar e9351cbaa: OUTPUT: «set()␤bag()␤mix()␤»
<Zoffix__> m: dd set() === Set.new
<camelia> rakudo-moar e9351cbaa: OUTPUT: «Bool::True␤»
@TisonKun

This comment has been minimized.

Show comment
Hide comment
@TisonKun

TisonKun Jun 24, 2018

Contributor

rakudo/src/core/Baggy.pm6

Lines 327 to 355 in e9351cb

multi method perl(Baggy:D: --> Str:D) {
nqp::if(
$!elems && nqp::elems($!elems),
nqp::concat(
nqp::concat(
'(',
nqp::join(',',
Rakudo::QuantHash.RAW-VALUES-MAP(self, {
nqp::if(
(my $value := nqp::getattr($_,Pair,'$!value')) == 1,
nqp::getattr($_,Pair,'$!key').perl,
"{nqp::getattr($_,Pair,'$!key').perl}=>$value"
)
})
)
),
nqp::concat(').',self.^name)
),
nqp::if(
nqp::istype(self,Bag),
'bag()',
nqp::if(
nqp::istype(self,Mix),
'mix()',
nqp::concat('().',self.^name)
)
)
)
}

rakudo/src/core/Setty.pm6

Lines 160 to 175 in e9351cb

multi method perl(Setty:D $ : --> Str:D) {
nqp::if(
nqp::eqaddr(self,set()),
'set()',
nqp::concat(
'(',
nqp::concat(
nqp::join(",",Rakudo::QuantHash.RAW-VALUES-MAP(self, *.perl)),
nqp::concat(
').',
self.^name
)
)
)
)
}

Cherry pick from Baggy to Setty is enough.

Contributor

TisonKun commented Jun 24, 2018

rakudo/src/core/Baggy.pm6

Lines 327 to 355 in e9351cb

multi method perl(Baggy:D: --> Str:D) {
nqp::if(
$!elems && nqp::elems($!elems),
nqp::concat(
nqp::concat(
'(',
nqp::join(',',
Rakudo::QuantHash.RAW-VALUES-MAP(self, {
nqp::if(
(my $value := nqp::getattr($_,Pair,'$!value')) == 1,
nqp::getattr($_,Pair,'$!key').perl,
"{nqp::getattr($_,Pair,'$!key').perl}=>$value"
)
})
)
),
nqp::concat(').',self.^name)
),
nqp::if(
nqp::istype(self,Bag),
'bag()',
nqp::if(
nqp::istype(self,Mix),
'mix()',
nqp::concat('().',self.^name)
)
)
)
}

rakudo/src/core/Setty.pm6

Lines 160 to 175 in e9351cb

multi method perl(Setty:D $ : --> Str:D) {
nqp::if(
nqp::eqaddr(self,set()),
'set()',
nqp::concat(
'(',
nqp::concat(
nqp::join(",",Rakudo::QuantHash.RAW-VALUES-MAP(self, *.perl)),
nqp::concat(
').',
self.^name
)
)
)
)
}

Cherry pick from Baggy to Setty is enough.

@MasterDuke17

This comment has been minimized.

Show comment
Hide comment
@MasterDuke17

MasterDuke17 Jun 25, 2018

Contributor

See #1722 for some related discussion.

Contributor

MasterDuke17 commented Jun 25, 2018

See #1722 for some related discussion.

@zoffixznet

This comment has been minimized.

Show comment
Hide comment
@zoffixznet

zoffixznet Jun 29, 2018

Contributor

There's a related ticket that could use your input as well: #1959

That's now resolved as not-a-bug, so it's fine for set()/bag()/mix() to be .perl'ed differently since in Rakudo we intern those.

The actual inconsistency now is that Set.new returns a separate object, but Mix.new/Bag.new return an interned copy. So we can "legally" make Set.new return an interned copy as well.

Contributor

zoffixznet commented Jun 29, 2018

There's a related ticket that could use your input as well: #1959

That's now resolved as not-a-bug, so it's fine for set()/bag()/mix() to be .perl'ed differently since in Rakudo we intern those.

The actual inconsistency now is that Set.new returns a separate object, but Mix.new/Bag.new return an interned copy. So we can "legally" make Set.new return an interned copy as well.

@lizmat

This comment has been minimized.

Show comment
Hide comment
@lizmat

lizmat Jul 26, 2018

Contributor

AFAIK, Bag.new and Mix.new don't return an interned copy anymore?

Contributor

lizmat commented Jul 26, 2018

AFAIK, Bag.new and Mix.new don't return an interned copy anymore?

@lizmat

This comment has been minimized.

Show comment
Hide comment
@lizmat

lizmat Jul 26, 2018

Contributor

So I think this ticket can be closed. Please re-open if you disagree.

Contributor

lizmat commented Jul 26, 2018

So I think this ticket can be closed. Please re-open if you disagree.

@lizmat lizmat closed this Jul 26, 2018

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