[std.range docs] drop and dropOne may not mutate the range

[ntrel]

Fixes #10828.

[dlang-bot]

Thanks for your pull request and interest in making D better, @ntrel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#10829"

[pbackus]

Since drop does not call .save on its argument, it cannot guarantee that the original range is not mutated, because the copy may share state with the original (e.g., if the range is a class instance).

[0xEAB]

Maybe we should also outline the condition that determines when this “may” may apply.

[jmdavis]

If we want something fairly short and clear, then something like

Unlike $(LREF popFrontN), the range argument is passed by value, not by
$(K_REF), so the range is copied, and it's the copy which has elements
popped off prior to it being returned. Whether the range argument is affected
by drop depends on the copy semantics of the range type.

would probably be better.

[jmdavis]

IMHO, talking about mutation isn't the right thing to be talking about. drop copies the range. It's the copy that gets mutated. What happens to the original depends entirely on the copy semantics of the range type. And because what happens to the original depends on the copy semantics of the range type, generic code should assume that when it passes the range to drop, that range should never be used again unless it's assigned a new value. You can fudge that if you're dealing with a specific range type in non-generic code, because then you can rely on the copy semantics, but the issue here is not mutation. The issue is copying.

So, what the caller needs to know is that when they call drop, they should use the return value and not the function argument when they continue their iteration. And if they want to continue to use the function argument after calling drop, they need to call save when they pass the argument to drop. That holds true for any range-based function which takes a range by value instead of by ref.

I would have to sit down and think about this more to come up with a correct way of stating this which isn't long, but IMHO, talking about mutation is the wrong thing to do, because it's the wrong mindset to have. The correct mindset to have is that when you copy a range, you cannot use it again without assigning it a new value first, and that if you want to use the range after you've made a copy, you need to call save on it to make the copy explicitly. What drop does with the copy is pretty much irrelevant.

[pbackus]

The point of this documentation change is to communicate to users that they cannot write

r.drop(3);

...and then expect r itself to change. I think the current wording successfully communicates this.

The full complexity of range copy semantics should definitely be documented somewhere, but I do not think that the DDoc for std.range.drop is the most appropriate place for that information to live. After all, there are many functions in std.range that take range arguments by value, and it would be absurd to add this explanation to all of them. A better approach would be to document the copy semantics of ranges once in some central location, like std.range.primitives, and then cross-reference it from other parts of the documentation when necessary.

[jmdavis]

I'm not arguing for laying out the whole issue here. As I said, I'd need to sit down and think about this more to come up with a concise way to say it.

My point is that talking about mutation is talking about the wrong thing (particularly since whether any mutation occurs to the argument is implementation-defined). The documentation should point out that the range is copied rather than passed by ref, so it's the copy that's iterated, and the return value is what needs to be used.

But if it is going to talk about mutation, I'd argue that it should say something like that whether the original range is mutated depends on the copy semantics of the range type. Talking about "may mutate" is too vague IMHO. Either way, the key point here is the fact that the range is copied and not passed by ref.

[ntrel]

if it is going to talk about mutation

I've changed it to "the source range may not get popped". I think it's outside the scope of these functions to describe when that is, but if there is a good explanation somewhere we can add a link to it.

the key point here is the fact that the range is copied and not passed by ref

Yes, if my change is not approved the docs could just say that instead IMO.

[jmdavis]

if it is going to talk about mutation

I've changed it to "the source range may not get popped". I think it's outside the scope of these functions to describe when that is, but if there is a good explanation somewhere we can add a link to it.

the key point here is the fact that the range is copied and not passed by ref

Yes, if my change is not approved the docs could just say that instead IMO.

IMHO, talking about popping or mutation is giving entirely the wrong message. The key thing is that a copy takes place. And if we want to talk about what does or doesn't happen to the range that's passed in, we should be saying that it's implementation-defined, because the copy semantics are implementation-defined. We don't have to go into all of the details about why that is, but saying things like the elements "might not be popped off" or that the range "might not be mutated" is incredibly vague IMHO without actually explaining anything. What folks need to be cognizant of is that a copy is being made rather than the argument being passed by ref.