Try to move towards the Google style guide declaration order. (#221)

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.

lexer/token_kind.h

@@ -13,6 +13,8 @@
 namespace Carbon {
 
 class TokenKind {
+  // Note that this must be declared earlier in the class so that its type can
+  // be used, for example in the conversion operator.
   enum class KindEnum : int8_t {
 #define CARBON_TOKEN(TokenName) TokenName,
 #include "lexer/token_registry.def"

lexer/tokenized_buffer.h

@@ -274,15 +274,15 @@ class TokenizedBuffer {
   // Specifies minimum widths to use when printing a token's fields via
   // `printToken`.
   struct PrintWidths {
+    // Widens `this` to the maximum of `this` and `new_width` for each
+    // dimension.
+    void Widen(const PrintWidths& new_width);
+
     int index;
     int kind;
     int column;
     int line;
     int indent;
-
-    // Widens `this` to the maximum of `this` and `new_width` for each
-    // dimension.
-    void Widen(const PrintWidths& new_width);
   };
 
   struct TokenInfo {

lexer/tokenized_buffer_test_helpers.h

@@ -24,13 +24,6 @@ inline void PrintTo(const TokenizedBuffer& buffer, std::ostream* output) {
 namespace Testing {
 
 struct ExpectedToken {
-  TokenKind kind;
-  int line = -1;
-  int column = -1;
-  int indent_column = -1;
-  bool recovery = false;
-  llvm::StringRef text = "";
-
   friend auto operator<<(std::ostream& output, const ExpectedToken& expected)
       -> std::ostream& {
     output << "\ntoken: { kind: '" << expected.kind.Name().str();
@@ -52,6 +45,13 @@ struct ExpectedToken {
     output << " }";
     return output;
   }
+
+  TokenKind kind;
+  int line = -1;
+  int column = -1;
+  int indent_column = -1;
+  bool recovery = false;
+  llvm::StringRef text = "";
 };
 
 // TODO: Consider rewriting this into a `TokenEq` matcher which is used inside

parser/parse_node_kind.h

@@ -24,6 +24,13 @@ namespace Carbon {
 // member functions. These instances are designed specifically to be usable in
 // `case` labels of `switch` statements just like an enumerator would.
 class ParseNodeKind {
+  // Note that this must be declared earlier in the class so that its type can
+  // be used, for example in the conversion operator.
+  enum class KindEnum : uint8_t {
+#define CARBON_PARSE_NODE_KIND(Name) Name,
+#include "parser/parse_node_kind.def"
+  };
+
  public:
   // The formatting for this macro is weird due to a `clang-format` bug. See
   // https://bugs.llvm.org/show_bug.cgi?id=48320 for details.
@@ -47,20 +54,15 @@ class ParseNodeKind {
   // Gets a friendly name for the token for logging or debugging.
   [[nodiscard]] auto GetName() const -> llvm::StringRef;
 
- private:
-  enum class KindEnum : uint8_t {
-#define CARBON_PARSE_NODE_KIND(Name) Name,
-#include "parser/parse_node_kind.def"
-  };
-
-  constexpr explicit ParseNodeKind(KindEnum k) : kind(k) {}
-
   // Enable conversion to our private enum, including in a `constexpr` context,
   // to enable usage in `switch` and `case`. The enum remains private and
   // nothing else should be using this.
   // NOLINTNEXTLINE(google-explicit-constructor)
   constexpr operator KindEnum() const { return kind; }
 
+ private:
+  constexpr explicit ParseNodeKind(KindEnum k) : kind(k) {}
+
   KindEnum kind;
 };
 

parser/parse_tree.h

@@ -140,6 +140,10 @@ class ParseTree {
   // The in-memory representation of data used for a particular node in the
   // tree.
   struct NodeImpl {
+    explicit NodeImpl(ParseNodeKind k, TokenizedBuffer::Token t,
+                      int subtree_size_arg)
+        : kind(k), token(t), subtree_size(subtree_size_arg) {}
+
     // The kind of this node. Note that this is only a single byte.
     ParseNodeKind kind;
 
@@ -179,10 +183,6 @@ class ParseTree {
     // This field should always be a positive integer as at least this node is
     // part of its subtree.
     int32_t subtree_size;
-
-    explicit NodeImpl(ParseNodeKind k, TokenizedBuffer::Token t,
-                      int subtree_size_arg)
-        : kind(k), token(t), subtree_size(subtree_size_arg) {}
   };
 
   static_assert(sizeof(NodeImpl) == 12,
@@ -301,9 +301,9 @@ class ParseTree::PostorderIterator
  private:
   friend class ParseTree;
 
-  Node node;
-
   explicit PostorderIterator(Node n) : node(n) {}
+
+  Node node;
 };
 
 // A forward iterator across the silbings at a particular level in the parse
@@ -343,12 +343,12 @@ class ParseTree::SiblingIterator
  private:
   friend class ParseTree;
 
+  explicit SiblingIterator(const ParseTree& tree_arg, Node n)
+      : tree(&tree_arg), node(n) {}
+
   const ParseTree* tree;
 
   Node node;
-
-  explicit SiblingIterator(const ParseTree& tree_arg, Node n)
-      : tree(&tree_arg), node(n) {}
 };
 
 }  // namespace Carbon

parser/parser_impl.h

@@ -25,12 +25,6 @@ class ParseTree::Parser {
  private:
   struct SubtreeStart;
 
-  ParseTree& tree;
-  TokenizedBuffer& tokens;
-
-  TokenizedBuffer::TokenIterator position;
-  TokenizedBuffer::TokenIterator end;
-
   explicit Parser(ParseTree& tree_arg, TokenizedBuffer& tokens_arg)
       : tree(tree_arg),
         tokens(tokens_arg),
@@ -127,6 +121,12 @@ class ParseTree::Parser {
   // skipping errors, can be parsed, it is returned. There may be parse errors
   // even when a node is returned.
   auto ParseDeclaration() -> llvm::Optional<Node>;
+
+  ParseTree& tree;
+  TokenizedBuffer& tokens;
+
+  TokenizedBuffer::TokenIterator position;
+  TokenizedBuffer::TokenIterator end;
 };
 
 }  // namespace Carbon