Because of a widely known language design flaw of C, for which we don't have a nice workaround. See my comment in the related devtalk thread.

I only have a rough understanding of what the function does. All I had to do was making the precedence of handle sides controllable by a caller, the remaining logic should be valid still.
I can add a few words on what the (badly named...) function does though.

Because clang-format :)

It's worse even, the name doesn't fit what it's actually doing either.

The comment is poorly worded indeed. The handles weren't intentionally hided, but by default only triples with a selected control-point/handle are visible.
We have to skip those in some cases.

From all feedback that I got, the old default wasn't at all what animators wanted. The reason it was using the "bad" default, is that the "good" default was broken in a way:
With View → Only Selected Keyframes Handles enabled, box selecting with include_handles would act on handles that were not visible.
William, Hjalti and I agreed on fixing this and changing the default.

See rB70d86d55c393.

I should mention that the logic here is very similar to the logic we use in multiple select operators, wherever we've introduced "drag-all-selected" now. I'm personally not too happy with the naming either, I stuck with what was already there though.
I also think it's useful to have naming and usage similar to the other operators, so that it's easy to catch the similarities.
Nevertheless I could do some local improvements here.

If already_selected is changed in some way, so that the NULL-check is removed, this would break with possibly hard to find consequences. Note how the variable is only there for readability, it's not used anywhere outside of the if. So the first NULL-check is only there to prevent NULL-pointer-dereference, while the one in the if matters for the actual logic.

In such cases I personally prefer the redundancy this introduces.

Good question, I copied and updated the comment for my changes, but this could be made more clear.

It is rather difficult to describe this well.
Basically it is logic that's designed to get a very specific behavior to work with our event-handling system. Bastien was kinda forced to introduce it, but he wasn't really happy about it either, see discussion in T63994. Brecht helped improve things quite a bit, but it's still complicated.

I'm not sure how to convey the logic here, maybe I'll just link to T63994 in cases where this is used.

See my comment above regarding this trickery.

We use this term to describe click+drag events, in the code and in the UI (see keymap editor). So it is the technically correct term here, even though I agree it's overloaded a bit.

Yeah, again things are a bit hard to explain...

There are some lower level functions taking a use_handles parameter, some of which are shared with other code. We call them from multiple places here in the transform code, so it makes sense to have a constant in some header file to toggle the behavior.

The reason I ended up with this is because I think it helps understanding how things work together. You can just search for IS_USE_HANDLE or disable the define to see how it affects the behavior and how the parts using it are connected. It wasn't easy to find this out myself so I prefer helping others to get there faster.

I could explain this in the comment. And maybe it's not worth keeping the alternative behavior.

AFAIK and FWIW, "vertices" is the term we use for this elsewhere, but in this case I can explicitly name handles and keys/control-points.

T63994 has a huge discussion attached to it, and no summary in the task description. I wouldn't rely on it when trying to explain the current implementation.

I'm just commenting on local code readability here. It's conditionally changing a variable that's used only in a conditional in the next statement. This makes the code unnecessarily complex.

This code:

if (select_mode != SELECT_REPLACE) {
  wait_to_deselect_others = false;
}
if (wait_to_deselect_others && (nvi->bezt->f2 & SELECT)) ...

is, as long as I'm not missing anything, equivalent to:

if (select_mode == SELECT_REPLACE && wait_to_deselect_others && (nvi->bezt->f2 & SELECT)) ...

Once you have this, you can see that both the if and the else if require select_mode == SELECT_REPLACE, and thus you can move that to an outer if-statement:

if (select_mode == SELECT_REPLACE) {
  if (wait_to_deselect_others && (nvi->bezt->f2 & SELECT)) {
    ret_val = OPERATOR_RUNNING_MODAL;
  }
  else {
    select_mode = SELECT_ADD;
    deselect_graph_keys(ac, 0, SELECT_SUBTRACT, false);
  }
}

It's only technically correct if it has been properly defined in a technical manner. AFAICS this is not the case.

Click + drag, is that mouse down, mouse up, mouse down, move? If it's just mouse down + move, what is the difference between "tweak" and "drag"?

Yeah, again things are a bit hard to explain...

To me this is unacceptable. If something is hard to explain it MUST be explained. If something is easy to explain, ok, then you might consider not adding a comment. But if it's hard, what reason is there to leave out any explanation?

There are some lower level functions taking a use_handles parameter, some of which are shared with other code. We call them from multiple places here in the transform code, so it makes sense to have a constant in some header file to toggle the behavior.

This doesn't make sense to me. The fact that code is used in different places doesn't imply to me that we want to be able to switch behaviour by editing some preprocessor directive.

And maybe it's not worth keeping the alternative behavior.

I think simpler code is easier to explain. As you said, the UI code is already complex, and when things can be changed around, without any clear explanation why, it only makes understanding things harder.

The difference is explained here: https://docs.blender.org/manual/en/latest/advanced/keymap_editing.html?highlight=tweak#multi-action-keys.

Checking over this again, I notice that the changes to this aren't needed actually. I had a wrong understanding of what option this flag belongs to.

Reverted related changes.

It defines neither "click" nor "tweak".

True, it just explains how click/drag and click/tweak can be used differently.
But this is a documentation issue, since it is literally the term we use in the UI to describe this specific event type:

It's a documentation issue for sure. I would suggest to limit ourselves to the English meaning of words, or to properly define such terms in the glossary.

Probably easier to read if you use - for itemisation rather than *.

Also, what is a 'select-mouse'?

Apart from that, great to see more documentation of this behaviour, it clarifies quite a bit.

What's the difference between 'tweak-move' and 'tweak'? If 'tweak-move' is 'click+drag', is 'tweak' the same as 'click'?

Okay, I've set up a Glossary in the HIG now, where we can give more precise technical definitions.
Also added a definition of Tweak and Drag (as I understand it ;) https://wiki.blender.org/wiki/Human_Interface_Guidelines/Glossary#Actions

We should probably improve terminology but I'll have to leave that up to others for now.
Creating better definitions and applying them more consequently is something we'll have to do progressively. And it will have to become a habit to look definitions up in the Glossary (or add definitions). Could be a great improvement to our process.

It's RMB for right click select, LMB for left click select :)
But I guess it's clear with out the select- prefix, so got rid of it.

Indeed, I thought it was easier to understand that way, but in fact it might be just as well misleading. Adjusted it slightly.