Reimplementing the bulk of std.meta iteratively

[Jackel]

On Tuesday, 29 September 2020 at 03:14:34 UTC, Andrei 
Alexandrescu wrote:
> Or take the foreach statement. Through painful trial and error, 
> somebody figured out all possible shapes of foreach, and 
> defined `each` to support most:
>
> https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L904
>
> What should have been a simple forwarding problem took 190 
> lines that could be characterized as very involved. And mind 
> you, it doesn't capture all cases because per 
> https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L1073:
>
> // opApply with >2 parameters. count the delegate args.
> // only works if it is not templated (otherwise we cannot count 
> the args)
>
> I know that stuff (which will probably end on my forehead) 
> because I went and attempted to improve things a bit in 
> https://github.com/dlang/phobos/pull/7638/files#diff-0d6463fc6f41c5fb774300832e3135f5R805, which attempts to simplify matters by reducing the foreach cases to seven shapes.
>
> To paraphrase Alan Perlis: "If your foreach has seven possible 
> shapes, you probably missed some."
>
> Over time, things did get better. We became adept of things 
> such as lowering, and we require precision in DIPs.

I think this is a mischaracterization of a problem. I'm not 
really a fan of how ranges are implemented, especially how UFCS 
is used to chain functions together. When they start to become 
10+ lines of a chain it is almost impossible to profile such 
code; with tools. Let alone try to understand what is happening, 
it would require to know how each function is implemented and how 
it expands and ends up being executed. Some things are lazy, 
somethings aren't; eg requiring `.array` after a `.map`. This is 
where I feel there shouldn't even be a `each` to begin with, just 
use a for loop.

Now that's my opinion, I deal with low level code that needs to 
be optimized and those UFSC chains are likened to scripting 
languages that usually don't need to worry about optimization. 
For short chains I don't think it's that big of a deal, it's 
usually easy enough to figure out what is going on and not enough 
is happening for too much to go wrong.

> And mind you, it doesn't capture all cases because per 
> https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L1073:
>
> // opApply with >2 parameters. count the delegate args.
> // only works if it is not templated (otherwise we cannot count 
> the args)

If you use a template of `opApply()` you lose type inference (as 
you do with regular templates). It's a limitation of the feature. 
You'd have the same problem if you have an overload of 
`opApply()` with delegates that have the same number of 
arguments; it won't know which one to pick. If you want to 
support both you'd just need 1 other case, but honestly if you 
have a template `opApply`, that's just bad practice (due to 
losing type inference). It doesn't need to be supported. The only 
reason I've ever wanted a templated opApply() is for auto 
attribute inference, but that's not worth losing the auto type 
inference you get with foreach. There was a suggestion for adding 
a new feature, or rather altering the behavior to allow for both, 
but I don't think it was well received.


> I know that stuff (which will probably end on my forehead) 
> because I went and attempted to improve things a bit in 
> https://github.com/dlang/phobos/pull/7638/files#diff-0d6463fc6f41c5fb774300832e3135f5R805, which attempts to simplify matters by reducing the foreach cases to seven shapes.

You are going the wrong way about simplifying the code. Why not 
add an opApply() property to the internal container types (array, 
assoc array, etc...)? Shouldn't be that difficult. Cases 4, 5, 6, 
and 7 would collapse into one then.

Cases 1, 2, and 3 aren't really different ways of using foreach. 
They are just conveniences for ranges. That tuple expansion is 
odd though, doesn't seem to be documented and it's a feature that 
foreach doesn't have (unlike the second parameter for the index). 
Doesn't need to be there, but someone probably uses it somewhere 
now.

So really it's just 2 cases with an additional case to support an 
index counter. That fits the bill, case 1: foreach, case 2: 
phobos ranges.

It's not as bad as you make it out to be, for this case.