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

Add Set candidate for SetHash's STORE #1847

Conversation

MasterDuke17
Copy link
Contributor

So set metaops like ∖= will DWIM. A possible final fix for GH #1203. perl6 -e 'my %days := SetHash.new: Date.today … Date.new: "2018-06-02"; say %days.elems; %days ∖= %days.grep: *.key.day-of-week > 5; say %days.elems' now prints out 14 and 10, instead of 14 and 1.

Rakudo builds ok and passes make m-test m-spectest (will add some new test to roast if this is merged).

So metaops like `∖=` will DWIM. A possible final fix for GH rakudo#1203.
@lizmat
Copy link
Contributor

lizmat commented May 20, 2018

FWIW, I'm -1 on this fix atm on the grounds that it doesn't feel right: will need some time to look into the problem deeper, which will be tomorrow.

@MasterDuke17
Copy link
Contributor Author

To add some additional data, the docs say that the set operators on SetHashes return immutable sets (https://docs.perl6.org/type/SetHash#Operators), which does seem to be specced in roast (e.g., https://github.com/perl6/roast/blob/master/S03-operators/set_symmetric_difference.t#L43).

@MasterDuke17
Copy link
Contributor Author

@lizmat ping. Any further thoughts?

@lizmat
Copy link
Contributor

lizmat commented Aug 5, 2018

Looking at this again now :-)

@MasterDuke17
Copy link
Contributor Author

ISTR @jnthn had some comments on IRC, I'll see if I can find them and add them here.

@MasterDuke17
Copy link
Contributor Author

I haven't found the chat yet, but I think it was something about how it should be done as a new candidate for METAOP_ASSIGN (https://github.com/rakudo/rakudo/blob/master/src/core/metaops.pm6#L2-L4), but since that just takes an op and returns a sub, it would be a bit tricky.

@MasterDuke17
Copy link
Contributor Author

@lizmat
Copy link
Contributor

lizmat commented Jun 22, 2019

Sorry, rejecting this PR on the same grounds as #3013

@lizmat lizmat closed this Jun 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants