[NFC] Refactor code annotations to make using them easier

[kripken]

Define aliases for their types, allowing users to do things like

makeIf(.., .., CodeAnnotation::BranchLikely(true))

Also move this outside of Function. Annotations can, in principle,
appear on things outside of functions (e.g. code in globals, though
no use exists yet), and also this avoids extra typing of Function::

edit: diff without whitespace is smaller

[tlively]

I don't think these type aliases are beneficial. Users of BranchLikely still need to know that it is actually a std::optional<bool>, so hiding that fact from them just introduces more friction. If we do want to introduce a new type name here, using a three-state enum is probably the way to go. Same for Inline below.

[kripken]

I agree users need to know likely is a bool and inline is an int. But I am trying to help in this way:

// ugly "true" - what is it?
makeIf(condition, arm, true);

// comment... slightly better, but easy to forget
makeIf(condition, arm, true /* likely */);

// type - much nicer
makeIf(condition, arm, BranchLikely(true));

(Maybe an enforced type would be even better?)

If we do want to introduce a new type name here, using a three-state enum is probably the way to go. Same for Inline below

For Inline, a 129-state enum..? 😉

[tlively]

How about using enum BranchLikeliness { Unlikely, Likely }; and a wrapper type with an explicit constructor:

struct InlineHint {
  uint8_t value;
  explicit Inline(uint8_t value) : value(value) {}
);

Then for the parameters in IRBuilder use std::optional<BranchLikeliness> and std::optional<InlineHint>. That puts the optionality where it belongs and makes call sites more readable.

[kripken]

The optionality is also needed in CodeAnnotation itself, not just as call params, since each hint is optional when stored (as appearing in the code before this PR). So we could have BranchLikeliness and InlineHint as you suggested - those look good - but we should also have a nice name for the optional of each of those, given the multiple uses in multiple places... and I don't have a good name idea for them?

[tlively]

I would just continue using std::optional explicitly wherever they need to be stored optionally. I generally don't think introducing a new name to avoid writing std::optional is a good trade off.

[kripken]

Fair enough. In that case, I'm not sure this PR is needed. I'll close it for now - maybe when we start using annotations in more places in the code, we'll have further thoughts on how to improve.