This is often a (much) better option for this code as we have many types
that are just an integer, pointer, or enum of data that would be much
more efficient as a by-value parameter. It is also a cleaner design in
most cases.
One place can't migrate in this way: iterators that use the LLVM CRTP
library for filling out iterator methods. The CRTP dispatch expects
operators to be actual members, and so those are left as-is in this
patch.
This patch tries to make everything adhere to the Google declaration
order. This is tricky as there doesn't appear to be a `clang-tidy` check
that helps us here at all.
One of the complex cases are the `enum`-wrapping classes we use. These
need to have a *public* conversion operator to the nested `enum` type to support
`switch` statements and the like. However, this makes it impossible to
fully respect the Google declaration order. We need to define the enum
type before the public API in order to use it in contexts like the
conversion operator. Even defining out-of-line won't help avoid this. It
is weird to have a type in the public API that is private, but again
this is only intended to be used for implicit conversions within
a `switch` statement or a `case` label. In one case, we had the
non-conforming order of declaration. I've added a comment to explain why
there. In the other case, the conversion operator is actually *private*
rather than public, which doesn't actually work in practice. I've made
this public and moved the declaration order to match with a matching
comment.
Most of these were fixed automatically (including things like adding
`[[nodiscard]]` and such). A number of others required manual edits.
I think all of them were pretty nice improvements.
There were a few places where the issues really stem from external
constraints and I've disabled the checks: GoogleTest macros or the
specific LibFuzzer entry points.
The only other places I disabled are the implicit conversions to
a private `enum` in the classes wrapping those `enum`s. These implicit
conversions are necessarily implicit to serve their only purpose:
enabling their use in `switch` statements and `case` labels. When these
were highlighted, it showed that one of these was actually converting to
an *`int`*. I've switched that to use the private `enum` instead as
doing so is important to enable warnings on non-covering `switch`
statements over than `enum`. And indeed, there is a `switch` that was
was implicitly relying on falling through in this way, so I've added the
explicit documentation of the intentional pattern to address that
warning.
Sorry this is so large, all of this somewhat fell out of enabling the
`clang-tidy` checks. If it is too difficult to review as lump, I can
work on breaking it apart as needed. Just let me know.
Only change is to update the path to the fuzzer build extension.
Original main commit message:
> Add an initial parser library. (#30)
>
> This library builds a parse tree, very similar to a concrete syntax
> tree. There are no semantics here, simply introducing the basic
> syntactic structure.
>
> The current focus has been on the APIs and the data structures used to
> represent the parse tree, and not on the actual code doing the
> parsing. The code doing the parsing tries to be reasonably efficient
> and reasonably easy to understand recursive descent parser. But there
> is likely much that can be done to improve this code path. A notable
> area where very little thought has been given yet are emitting good
> diagnostics and doing good recovery in the event of parse errors.
>
> Also, this code does not try to match the current under-discussion
> grammar closely. It is only partial and reflects discussions from some
> time ago. It should be updated incrementally to reflect the current
> expected grammar.
>
> The data structure used for the parse tree is unusual. The first
> constraint is that there is a precise one-to-one correspondence
> between the tokens produced by the lexer and the nodes in the parse
> tree. Every token results in exactly one node. In that way, the parse
> tree can be thought of as merely shaping the token stream into a tree.
>
> Each node is also represented with a fixed set of data that is densely
> packed. Combined with the exact relationship to tokens, this allows us
> to fully allocate the parse tree's storage, and to use a dense array
> rather than a pointer-based tree structure.
>
> The tree structure itself is implicitly defined by tracking the size
> of each subtree rooted at a particular node. See the code comments for
> more details (and I'm happy to add more comments where necessary). The
> goal is to minimize both the allocations (one), the working set size
> of the tree as a whole, and optimize common iteration patterns. The
> tree is stored in postorder. This allows depth-first postorder
> iteration as well as topological iteration by walking in reverse.
>
> Building the parse tree in postorder is a natural consequence of the
> grammar being LR rather than LL, which is a consequence of supporting
> infix operators.
>
> As with the Lexer, the parser supports an API for operating on the
> parse tree, as well as the ability to print the tree in both
> a human-readable and machine-readable format (YAML-based). It includes
> significant unit tests and a fuzz tester. The fuzzer's corpus will be
> in a follow-up commit.
>
> This is the largest chunk of code already written by several of us
> prior to open sourcing. (There are a few more pieces, but they are
> significantly smaller and less interesting.) If there are major things
> that folks would like to see happen here, it may make sense to move
> them into issues for tracking. I have tried to update the code to
> follow the style guidelines, but apologies if I missed anything, just
> let me know. We also have issues #19 and #29 to track things that
> already came up with the lexer.
Co-authored-by: Jon Meow <46229924+jonmeow@users.noreply.github.com>
Chandler Carruth
