浏览代码

Prevent accidents with uninitialized nodes (#1139)

This comes from #1092 where I wrote bad code and I felt it should have been caught more clearly.
Jon Meow 4 年之前
父节点
当前提交
50d9fc5189
共有 3 个文件被更改,包括 25 次插入1 次删除
  1. 6 0
      toolchain/parser/parse_tree.cpp
  2. 8 1
      toolchain/parser/parse_tree.h
  3. 11 0
      toolchain/parser/parse_tree_test.cpp

+ 6 - 0
toolchain/parser/parse_tree.cpp

@@ -37,6 +37,7 @@ auto ParseTree::postorder() const -> llvm::iterator_range<PostorderIterator> {
 
 
 auto ParseTree::postorder(Node n) const
 auto ParseTree::postorder(Node n) const
     -> llvm::iterator_range<PostorderIterator> {
     -> llvm::iterator_range<PostorderIterator> {
+  CHECK(n.is_valid());
   // The postorder ends after this node, the root, and begins at the start of
   // The postorder ends after this node, the root, and begins at the start of
   // its subtree.
   // its subtree.
   int end_index = n.index_ + 1;
   int end_index = n.index_ + 1;
@@ -47,6 +48,7 @@ auto ParseTree::postorder(Node n) const
 
 
 auto ParseTree::children(Node n) const
 auto ParseTree::children(Node n) const
     -> llvm::iterator_range<SiblingIterator> {
     -> llvm::iterator_range<SiblingIterator> {
+  CHECK(n.is_valid());
   int end_index = n.index_ - node_impls_[n.index_].subtree_size;
   int end_index = n.index_ - node_impls_[n.index_].subtree_size;
   return {SiblingIterator(*this, Node(n.index_ - 1)),
   return {SiblingIterator(*this, Node(n.index_ - 1)),
           SiblingIterator(*this, Node(end_index))};
           SiblingIterator(*this, Node(end_index))};
@@ -59,18 +61,22 @@ auto ParseTree::roots() const -> llvm::iterator_range<SiblingIterator> {
 }
 }
 
 
 auto ParseTree::node_has_error(Node n) const -> bool {
 auto ParseTree::node_has_error(Node n) const -> bool {
+  CHECK(n.is_valid());
   return node_impls_[n.index_].has_error;
   return node_impls_[n.index_].has_error;
 }
 }
 
 
 auto ParseTree::node_kind(Node n) const -> ParseNodeKind {
 auto ParseTree::node_kind(Node n) const -> ParseNodeKind {
+  CHECK(n.is_valid());
   return node_impls_[n.index_].kind;
   return node_impls_[n.index_].kind;
 }
 }
 
 
 auto ParseTree::node_token(Node n) const -> TokenizedBuffer::Token {
 auto ParseTree::node_token(Node n) const -> TokenizedBuffer::Token {
+  CHECK(n.is_valid());
   return node_impls_[n.index_].token;
   return node_impls_[n.index_].token;
 }
 }
 
 
 auto ParseTree::GetNodeText(Node n) const -> llvm::StringRef {
 auto ParseTree::GetNodeText(Node n) const -> llvm::StringRef {
+  CHECK(n.is_valid());
   return tokens_->GetTokenText(node_impls_[n.index_].token);
   return tokens_->GetTokenText(node_impls_[n.index_].token);
 }
 }
 
 

+ 8 - 1
toolchain/parser/parse_tree.h

@@ -259,18 +259,25 @@ class ParseTree::Node {
   // Prints the node index.
   // Prints the node index.
   auto Print(llvm::raw_ostream& output) const -> void;
   auto Print(llvm::raw_ostream& output) const -> void;
 
 
+  // Returns true if the node is valid; in other words, it was not default
+  // initialized.
+  auto is_valid() -> bool { return index_ != InvalidValue; }
+
  private:
  private:
   friend ParseTree;
   friend ParseTree;
   friend Parser;
   friend Parser;
   friend PostorderIterator;
   friend PostorderIterator;
   friend SiblingIterator;
   friend SiblingIterator;
 
 
+  // Value for uninitialized nodes.
+  static constexpr int InvalidValue = -1;
+
   // Constructs a node with a specific index into the parse tree's postorder
   // Constructs a node with a specific index into the parse tree's postorder
   // sequence of node implementations.
   // sequence of node implementations.
   explicit Node(int index) : index_(index) {}
   explicit Node(int index) : index_(index) {}
 
 
   // The index of this node's implementation in the postorder sequence.
   // The index of this node's implementation in the postorder sequence.
-  int32_t index_;
+  int32_t index_ = InvalidValue;
 };
 };
 
 
 // A random-access iterator to the depth-first postorder sequence of parse nodes
 // A random-access iterator to the depth-first postorder sequence of parse nodes

+ 11 - 0
toolchain/parser/parse_tree_test.cpp

@@ -48,6 +48,17 @@ class ParseTreeTest : public ::testing::Test {
   DiagnosticConsumer& consumer = ConsoleDiagnosticConsumer();
   DiagnosticConsumer& consumer = ConsoleDiagnosticConsumer();
 };
 };
 
 
+TEST_F(ParseTreeTest, DefaultInvalid) {
+  ParseTree::Node node;
+  EXPECT_FALSE(node.is_valid());
+}
+
+TEST_F(ParseTreeTest, IsValid) {
+  TokenizedBuffer tokens = GetTokenizedBuffer("");
+  ParseTree tree = ParseTree::Parse(tokens, consumer);
+  EXPECT_TRUE((*tree.postorder().begin()).is_valid());
+}
+
 TEST_F(ParseTreeTest, Empty) {
 TEST_F(ParseTreeTest, Empty) {
   TokenizedBuffer tokens = GetTokenizedBuffer("");
   TokenizedBuffer tokens = GetTokenizedBuffer("");
   ParseTree tree = ParseTree::Parse(tokens, consumer);
   ParseTree tree = ParseTree::Parse(tokens, consumer);