[Syntax] Do not add a node for 'eof' into the tree

[ilya-biryukov]

Summary

While useful as a sentinel value when iterating over tokens, having
'eof' in the tree, seems to do more harm than good.

clang/lib/Tooling/Syntax/BuildTree.cpp

public:
   void foldNode(llvm::ArrayRef<syntax::Token> Range, syntax::Tree *New);
 
   /// Set role for a token starting at \p Loc.
   void markChildToken(SourceLocation Loc, tok::TokenKind Kind, NodeRole R);
 
   /// Finish building the tree and consume the root node.
   syntax::TranslationUnit *finalize() && {
     auto Tokens = Arena.tokenBuffer().expandedTokens();
+    assert(!Tokens.empty());
+    assert(Tokens.back().kind() == tok::eof);
+
     // Build the root of the tree, consuming all the children.
-    Pending.foldChildren(Tokens,
+    Pending.foldChildren(Tokens.drop_back(),
                          new (Arena.allocator()) syntax::TranslationUnit);
 
     return cast<syntax::TranslationUnit>(std::move(Pending).finalize());
   }
 
   /// getRange() finds the syntax tokens corresponding to the passed source
   /// locations.
   /// \p First is the start position of the first token and \p Last is the start
private:
   /// A collection of trees covering the input tokens.
   /// When created, each tree corresponds to a single token in the file.
   /// Clients call 'foldChildren' to attach one or more subtrees to a parent
   /// node and update the list of trees accordingly.
   ///
   /// Ensures that added nodes properly nest and cover the whole token stream.
   struct Forest {
     Forest(syntax::Arena &A) {
-      // FIXME: do not add 'eof' to the tree.
-
+      assert(!A.tokenBuffer().expandedTokens().empty());
+      assert(A.tokenBuffer().expandedTokens().back().kind() == tok::eof);
       // Create all leaf nodes.
-      for (auto &T : A.tokenBuffer().expandedTokens())
+      // Note that we do not have 'eof' in the tree.
+      for (auto &T : A.tokenBuffer().expandedTokens().drop_back())
         Trees.insert(Trees.end(),
                      {&T, NodeAndRole{new (A.allocator()) syntax::Leaf(&T)}});
     }
 
     void assignRole(llvm::ArrayRef<syntax::Token> Range,
                     syntax::NodeRole Role) {
       assert(!Range.empty());
       auto It = Trees.lower_bound(Range.begin());

clang/unittests/Tooling/Syntax/TreeTest.cpp

 |-TopLevelDeclaration
 | |-int
 | |-main
 | |-(
 | |-)
 | `-CompoundStatement
 |   |-2: {
 |   `-3: }
-|-TopLevelDeclaration
-| |-void
-| |-foo
-| |-(
-| |-)
-| `-CompoundStatement
-|   |-2: {
-|   `-3: }
-`-<eof>
+`-TopLevelDeclaration
+  |-void
+  |-foo
+  |-(
+  |-)
+  `-CompoundStatement
+    |-2: {
+    `-3: }
 )txt"},
   };
 
   for (const auto &T : Cases) {
     auto *Root = buildTree(T.first);
     std::string Expected = llvm::StringRef(T.second).trim().str();
     std::string Actual = llvm::StringRef(Root->dump(*Arena)).trim();
     EXPECT_EQ(Expected, Actual) << "the resulting dump is:\n" << Actual;
   }
 }
 } // namespace