Isn't `each` too much of a good thing?

[Jonathan M Davis]

On Thursday, September 17, 2020 10:18:18 AM MDT Andrei Alexandrescu via 
Digitalmars-d wrote:
> This is in the same league with supporting enums that use string as a
> base, as "some kinda sorta string thing".
>
> Too much!
>
> I have no idea how to improve this code because it will break one of the
> suitably overspecialized unittests. But this kind of stuff has no place
> in stdv2021.

Agreed.

I think that it's yet another case of something being added, because someone
thought that it would be useful - probably several things over several
iterations of the code. Getting stray stuff into Phobos like that doesn't
really have a particularly high bar. Pull requests either tend to either sit
around for too long and/or just get merged because they don't do anything
clearly bad. Sometimes, someone will object to a particular pull request due
to something like adding support for static arrays, and the changes won't
get in, but I think that it's more likely that it will get in simply because
the work was done, and it passes the tests. We've never really had enough
people looking at pull requests, and we haven't had clear guidelines for how
some stuff should be handled. So, a number of things get handled
inconsistently. On some level, I think that our attempts to not PRs sitting
around forever have also led to more questionable code getting committed in
a number of cases. I don't know what a good way to fix that is.

Of course, another part of the calculus here is how changing how we've done
stuff over time has caused some fun problems - like templatizing a function
that used to explicitly take string. You then get a function that shouldn't
accept static arrays or enums with a base type of string but which has to in
order to avoid breaking code. isConvertibleToString is one of the big
mistakes that came out of that, and we still haven't purged it from Phobos.

Regardless, IMHO, we should be avoiding support for static arrays and
opApply in range-based code. At most, maybe there should be a way to convert
a type with opApply to a range, but static arrays are already solved just by
slicing them. In general, by requiring that someone explicitly do a
conversion or wrap something in another type in order to make it a range, we
can really simplify code. And in general, supporting implicit conversions in
templated code just adds complexity and landmines (e.g. you can easily get
bugs by testing that a type is implicitly convertible in the constraint but
then not explicitly converting the type within the function). IMHO, for the
most part, the less we support implicit conversions the better. I honestly
wish that static arrays didn't even implicitly convert to dynamic arrays.

If we're really going to do another version of Phobos and rework ranges,
then I think that as part of that, we should make various rules about how
Phobos will handle ranges (and how most code should handle ranges) clear.
Even when there's agreement on some of that stuff right now among most of
the key developers, it's not necessarily well-documented. We should probably
have explicitly documented rules and guidelines about stuff like not
supporting static ranges in range-based code. That would then at least allow
us to avoid some of the problems like shown here with each - as well as
better document how ranges work and are supposed to be used in general.

- Jonathan M Davis