[clang-format] spaceRequiredBefore vs spaceRequiredBetween

To determine whether a space is needed between two tokens, the function TokenAnnotator::spaceRequiredBefore is used. If none of the rules match, execution jumps to TokenAnnotator::spaceRequiredBetween for more rules. The latter isn’t called anywhere else. How to determine which function a rule should be in?

For the record, the function is at llvm-project/TokenAnnotator.cpp at main · llvm/llvm-project · GitHub.

In my opinion, the original idea was to use spaceRequiredBefore when it was decidable on the right token only and to use spaceRequiredBetween when both were needed to make the decision.
Unfortunately, both are mixed up now and spaceRequiredBefore looks at the left token as well in the majority of rules.

My 2 cents.

So should we merge the functions?

I don’t know. Probably some clean up would be nice.

To refactor those two is on my list, but I don’t think I will have the time to do that. So if you want please go ahead.
Here what I would have done:

  • Merge them.
  • Check the language specific stuff first (as far as possible) with a switch over the langauge, not the else if stuff.
  • Replace all the ifs with switches on right, as far as possible. And if one need to disambiguate local switches on left.
  • All this in I don’t know how many patches. I would go small steps.

What one could also think of is splitting it into smaller functions again, but with some documentation and sense. For example to install a hierarchy and only if level X did not come to a conclusion do the X+1 checks.

Right now it is a hell to debug these functions to check where did come the decision of adding or removing the space, and then you have to move some ifs around and see if you haven’t broken anything (known of).