Fix issue #15642 - make std.utf.encode return ubyte

[byebye]

No description provided.

[dlang-bot]

Fix	Bugzilla	Description
✓	15642	std.utf.encode should return ubyte

[burner]

what is the reason for the change?

[byebye]

To make it clear that result is really small and to avoid explicit casts from size_t (it is an issue from the buganizer, so I think it's legit, isn't it?).

[jmdavis]

I don't know. Lengths are normally done with size_t, but if you need to add the result to an int for some reason, having it be size_t can be annoying, and the length is guaranteed to fit in a ubyte, making size_t completely unnecessary from a size perspective. It's just consistent with how we deal with lengths in general - particularly any lengths that could be used with arrays, which is certainly the case here. And I don't really agree that the change is fully backwards compatible - if auto is used, then the type changes, and that could propagate in fun ways and potentially result in a bug in someone's code - but it's questionable that it would actually be a problem in practice. So, if we're sure that we'd be better off using ubyte, we should probably do it. It is still a risk though, and I don't think that it's clear that we should change it.

(it is an issue from the buganizer, so I think it's legit, isn't it?)

It's an enhancement request. It hasn't been specifically approved by Walter or Andrei. So, the fact that it's in bugzilla just means that someone thought that it was a good idea and put it in bugzilla. That doesn't necessarily mean that we should actually do it. It might be a great idea, and it might not be. That has yet to be decided.

Honestly, I think that the argument for making the change is pretty weak and that it would be better to just leave it as-is - particularly since we're trying to use size_t for lengths in general - but it's certainly possible that it would be better to change it.

[burner]

I'm also hesitant to this change.

[byebye]

Understood, many thanks for thorough explanation! I'm deleting this PR then.

[burner]

Thank you for working on phobos

[schveiguy]

The impetus for the enhancement request: https://github.com/schveiguy/iopipe/blob/master/source/iopipe/textpipe.d#L306

i.e. I don't want to add 3 or 7 more bytes to a struct for no reason, especially one that is passed around by value.

Is it fully backwards compatible? You can think of arbitrary code that someone would use auto and then proceeds to add a bunch to that value, but the API sure doesn't lend itself to that usage. You can't just pass in an arbitrary char[], so if you have a variable that represents "the number of bytes in your char[4] that are valid", why does it ever need to be a size_t? Unless you are just thinking of reusing some variable that happened to use encode to infer the type of it originally?

I don't know, I think with VRP in D, using the smallest type that can hold a result makes a difference when writing nice clean code (i.e. I hate having to use casts for such things that should be obvious). If there were some other way to mark a size_t result as only having values in the 0-3 range, it would be a better solution.

If you think the enhancement request is not worthy, please close the request as well. And thanks @byebye for trying!