XLS Style Guide
The Google style guides recommend enforcing local consistency where stylistic choices are not predefined. 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.
C++
- Align the pointer or reference modifier token with the type; e.g.
Foo& foo = ...
instead ofFoo &foo = ...
, andFoo* foo = ...
instead ofFoo *foo= ...
.
- Use
/*parameter_name=*/value
style comments if you choose to annotate arguments in a function invocation.clang-tidy
recognizes this form, and provides a Tricorder notification ifparameter_name
is mismatched against the parameter name of the callee.
- Prefer
int64_t
overint
to avoid any possibility of overflow.
- Always use
Status
orStatusOr
for any error that a user could encounter.
- Other than user-facing errors, use
Status
only in exceptional situations. For example,Status
is 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 useStatus
orStatusOr
.
- Prefer to brace single-statement blocks. Because the
XLS_ASSIGN_OR_RETURN
macro expands into multiple statements, this can cause problems when using unbraced single-statement blocks. Instead of XLS developers needing to think about individual cases of single statement blocks, we brace all single statement blocks.
- Prefer using
XLS_ASSIGN_OR_RETURN
/XLS_RETURN_IF_ERROR
when appropriate, but when binding aStatusOr
wrapped value prefer to name itthing_or
so that it can be referenced without the wrapper asthing
; e.g.absl::StatusOr<Thing> thing_or = f(); if (!thing_or.ok()) { // ... handling of the status via thing_or.status() and returning ... } const Thing& thing = thing_or.value();
- Prefer
CHECK
toDCHECK
, except thatDCHECK
can 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.
- Follow the C++ style guide for capitalization guidelines; however, in the
somewhat ambiguous case of I/O (short for Input/Output, which we use often),
the slash counts as internal spacing and therefore the capitalization we use
is
IO
, as inWrapIO
orStreamingIOReader
.
- Prefer to use the
XLS_FRIEND_TEST
macro vs friending manually-mangled test names.At times it can be useful to test unit test a private/protected member of a class, and the
XLS_FRIEND_TEST
macro makes this possible. Note that the test case must live outside an unnamed namespace in the test file for the "friending" to work properly.
- For simple const accessors, for the sake of consistency in the code base,
and a weak preference towards the benefits of information hiding, prefer to
return view types over the apparent type of the member; e.g.
class MyClass { public: // This return type is preferrable to `const std::vector<uint64_t>&`. absl::Span<const uint64_t> values() const { return values_; } private: std::vector<uint64_t> values_; };
- Follow the
style guide's
decision to avoid RTTI. In practice, this means
down_cast<>
should be used instead ofdynamic_cast<>
. However, the style guide says to avoid hand-implementing RTTI-like workarounds. The DSLX and XLScc frontends are places where avoiding RTTI would require implementing workarounds that end up looking a lot like RTTI, sodynamic_cast<>
is common and accepted for those parts of the codebase. Elsewhere, especially with IRNode
types,down_cast<>
should be used instead.
Functions
- 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> // ...
IR nodes
- 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.
Protocol buffers
- Prefer to use proto3 specifications in all new protocol buffer files.
FAQ
How heavyweight is StatusOr
?
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
a non-ok StatusOr
is expensive (that is, imagine the non-ok Status
within a StatusOr
is the expensive part).
The implication being: if there's an API where "not found" is a reasonable
outcome, prefer std::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
not-found 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 Status
es
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.