Skip to content

[WIP] std.range, map, ndslice type normalization#4935

Closed
9il wants to merge 12 commits intodlang:masterfrom
9il:nd-range
Closed

[WIP] std.range, map, ndslice type normalization#4935
9il wants to merge 12 commits intodlang:masterfrom
9il:nd-range

Conversation

@9il
Copy link
Member

@9il 9il commented Dec 7, 2016

No description provided.

@9il 9il requested review from andralex and removed request for andralex December 7, 2016 22:48
@9il 9il changed the title [WIP] std.range and map review [WIP] std.range, map, ndslice type normalization Dec 8, 2016
Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very interesting work, thanks. How about the following idea, which allows us extensibility without having to change map to recognize special type patterns:

  • Look for a method __map inside the range
  • If the range defines that method, map should use it
  • Otherwise, just do the usual

For the beginning, we don't document this and we only implement __map in the std ranges in this PR. If things go well, we change __map to mapImpl or even map and publish the API for anyone.

I've been thinking of this a lot lately: we should allow ranges to define additional methods under a specific nomenclature, similar to what allocators do. It's a great way to go about implementing features modularly.

}
else
// Type_normalization: take.map -> map.take
static if (is(Range : Take!R, R))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about TakeExactly?

@9il
Copy link
Member Author

9il commented Dec 8, 2016

Look for a method __map inside the range
If the range defines that method, map should use it
Otherwise, just do the usual

Yes, will do.

The PR changes also retro and stride. The diff is very large, you may want press to the file to see it. Do you want also add __stride and __retro?

@andralex
Copy link
Member

andralex commented Dec 8, 2016

Eh, thanks I didn't see those two. I'd say how about we do this for map and see how it goes? Thx!

@9il 9il closed this Dec 10, 2016
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.

2 participants