XLS Style Guide
The Google style guides recommend enforcing local consistency where stylistic choices are not pre-defined. This file notes some of the choices we make locally in the XLS project, with the relevant Google style guides (C++, Python) as their bases.
- Align the pointer or reference modifier token with the type; e.g.
Foo& foo = ...instead of
Foo &foo = ..., and
Foo* foo = ...instead of
Foo *foo= ....
/*parameter_name=*/valuestyle comments if you choose to annotate arguments in a function invocation.
clang-tidyrecognizes this form, and provides a Tricorder notification if
parameter_nameis mismatched against the parameter name of the callee.
intto avoid any possibility of overflow.
- Always use
StatusOrfor any error that a user could encounter.
- Other than user-facing errors, use
Statusonly in exceptional situations. For example,
Statusis good to signal that a required file does not exist but not for signaling that constant folding did not constant fold an expression.
See how heavyweight is StatusOr for more details on thinking about the costs involved.
- Internal errors for conditions that should never be false can use
CHECK, but may also use
DCHECK, except that
DCHECKcan be used to verify conditions that it would be too expensive to verify in production, but that are fast enough to include outside of production.
- Short or easily-explained argument lists (as defined by the developer) can
be explained inline with the rest of the function comment. For more complex
argument lists, the following pattern should be used:
// <Function description> // Args: // arg1: <arg1 description> // arg2: <arg2 description> // ...
- Unlike most data, IR elements should be passed as non-const pointers, even when expected to be const (which would usually indicate passing them as const references). Experience has shown that IR elements often develop non-const usages over time. Consider the case of IR analysis passes - those passes themselves rarely need to mutate their input data, but they build up data structures whose users often need to mutate their contents. In addition, treating elements as pointers makes equality comparisons more straightforward (avoid taking an address of a reference) and helps avoid accidental copies (assigning a reference to local, etc.). Non-const pointer usage propagates outwards such that the few cases where a const reference could actually be appropriate become odd outliers, so our guidance is that IR elements should uniformly be passed as non-const pointers.
How heavyweight is
What follows is the general guidance on how absl::StatusOr is used -- it is used extensively throughout the XLS code base as an error-style indicator object wrapper, so it is important to understand the mental model used for its cost.
Consider cost wise that: a) creating an ok
StatusOr is cheap, b) creating
StatusOr is expensive (that is, imagine the non-ok
StatusOr is the expensive part).
The implication being: if there's an API where "not found" is a reasonable
absl::optional<> as a return value to indicate that / go with
the grain of cost.
Something like a filesystem API would be a classic example -- where you
shouldn't be rooting around looking for files that aren't there -- so a
absl::StatusOr result would be fine to use.
A good potential mental model is to imagine the program may run with logging of
a traceback for every non-ok status that is created. (This is related to a
debugging capability in Google internally called
--util_status_save_stack_trace that captures backtraces when error
are created.) Ideally, with such a logging flag turned on, the screen wouldn't
fill up with "non error tracebacks", only tracebacks from events where something
really went wrong.