Generality creep

[Walter Bright]

On 3/28/2019 4:05 PM, ag0aep6g wrote:
> I don't mind you being against those changes, but the changes so far are hardly 
> "large". Ignoring the tests, both my PRs combined add meager 20 lines. And 
> that's for seven bug fixes.

Right, but those changes cut across a large number of algorithms, and will cause 
every algorithm implementer to do the same. That makes it a very large change. 
I'm glad I noticed this before a lot more work was expended in that direction.


> By the way, you recently made changes to `save` methods in std.utf:
> 
> https://github.com/dlang/phobos/pull/6900
> https://github.com/dlang/phobos/pull/6903
> 
> I'm not sure why you made those,

The problem I ran into was ranges that had const fields would fail, because one 
cannot assign to a const field. But one can construct a const field. Doing some 
digging, it became apparent that save() is equivalent to copy construction, and 
so should be implemented using copy construction.

Assignment and construction are fundamentally different, and save() is clearly a 
construction.


> but they touch the same kind of code as my PRs, 
> and they actually fix the issue I was fixing, too. Are you planning on applying 
> that pattern to all of Phobos, or just std.utf for some reason?

My immediate goal was simply to get phobos to compile with DIP1000. Over time, I 
expect a well-implemented save() function should use copy construction. I did 
write a PR to that effect:

https://github.com/dlang/phobos/pull/6905/files

Your advice on that would be appreciated.


> If you're going for all of Phobos, you might end up fixing issue 18657 as a side 
> effect. But removing RefRange.opAssign would still be nice, because it's just 
> weird.

Fixing RefRange.save() using copy construction, and ditching opAssign, would be 
acceptable.