[Tooling] Switch JSONCompilationDatabase to use JSON parser.

[sammccall]

Summary

Rather than hold all the JSON source in memory, as well as the YAML structures
needed to index into it, we parse eagerly into CompileCommand structures, which
simplifies the implementation a lot.

Possibly at the cost of performance/memory usage, need to measure that.
(I'd expect that eager parsing with string deduplication would be fine at least)

Behavior changes:

• rejects noise at end of JSON files (see fixed test), rather than silently accepting
• ignores unknown attributes, rather than rejecting
• when "arguments" is a single-element array, no longer tries to parse it (I guess this was a bug)

[sammccall]

(just updating description)

[sammccall]

Nevermind, this is like 5x slower to load the chromium CDB. probably because of all the little non-arena allocations.
I'll profile and fix it when I get bored again.

include/clang/Tooling/JSONCompilationDatabase.h

public:
   /// These are the 'file' entries of the JSON objects.
   std::vector<std::string> getAllFiles() const override;
 
   /// Returns all compile commands for all the files in the compilation
   /// database.
   std::vector<CompileCommand> getAllCompileCommands() const override;
 
 private:
-  /// Constructs a JSON compilation database on a memory buffer.
-  JSONCompilationDatabase(std::unique_ptr<llvm::MemoryBuffer> Database,
-                          JSONCommandLineSyntax Syntax)
-      : Database(std::move(Database)), Syntax(Syntax),
-        YAMLStream(this->Database->getBuffer(), SM) {}
-
-  /// Parses the database file and creates the index.
-  ///
-  /// Returns whether parsing succeeded. Sets ErrorMessage if parsing
-  /// failed.
-  bool parse(std::string &ErrorMessage);
-
-  // Tuple (directory, filename, commandline, output) where 'commandline'
-  // points to the corresponding scalar nodes in the YAML stream.
-  // If the command line contains a single argument, it is a shell-escaped
-  // command line.
-  // Otherwise, each entry in the command line vector is a literal
-  // argument to the compiler.
-  // The output field may be a nullptr.
-  using CompileCommandRef =
-      std::tuple<llvm::yaml::ScalarNode *, llvm::yaml::ScalarNode *,
-                 std::vector<llvm::yaml::ScalarNode *>,
-                 llvm::yaml::ScalarNode *>;
-
-  /// Converts the given array of CompileCommandRefs to CompileCommands.
-  void getCommands(ArrayRef<CompileCommandRef> CommandsRef,
-                   std::vector<CompileCommand> &Commands) const;
+  JSONCompilationDatabase(std::vector<CompileCommand> Commands);
 
-  // Maps file paths to the compile command lines for that file.
-  llvm::StringMap<std::vector<CompileCommandRef>> IndexByFile;
+  FileMatchTrie MatchTrie;
 
   /// All the compile commands in the order that they were provided in the
   /// JSON stream.
-  std::vector<CompileCommandRef> AllCommands;
-
-  FileMatchTrie MatchTrie;
-
-  std::unique_ptr<llvm::MemoryBuffer> Database;
-  JSONCommandLineSyntax Syntax;
-  llvm::SourceMgr SM;
-  llvm::yaml::Stream YAMLStream;
+  std::vector<CompileCommand> AllCommands;
+  // Maps file paths to the indexes of compile command lines (into AllCommands).
+  // Keys are native absolute paths (CompileCommand::Filename may not be).
+  llvm::StringMap<llvm::SmallVector<unsigned, 2>> FileCommands;
 };
 
 } // namespace tooling
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLING_JSONCOMPILATIONDATABASE_H

lib/Tooling/JSONCompilationDatabase.cpp

 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <memory>
 #include <string>
 #include <system_error>
-#include <tuple>
 #include <utility>
 #include <vector>
 
 using namespace clang;
 using namespace tooling;
 
 namespace {
 
JSONCompilationDatabase::loadFromFile(StringRef FilePath,
                                       std::string &ErrorMessage,
                                       JSONCommandLineSyntax Syntax) {
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> DatabaseBuffer =
       llvm::MemoryBuffer::getFile(FilePath);
   if (std::error_code Result = DatabaseBuffer.getError()) {
     ErrorMessage = "Error while opening JSON database: " + Result.message();
     return nullptr;
   }
-  std::unique_ptr<JSONCompilationDatabase> Database(
-      new JSONCompilationDatabase(std::move(*DatabaseBuffer), Syntax));
-  if (!Database->parse(ErrorMessage))
-    return nullptr;
-  return Database;
+  return loadFromBuffer((*DatabaseBuffer)->getBuffer(), ErrorMessage, Syntax);
+}
+
+static bool fromJSON(const llvm::json::Value &J, CompileCommand &C,
+                     JSONCommandLineSyntax Syntax) {
+  llvm::json::ObjectMapper O(J);
+  if (!O || !O.map("file", C.Filename) || !O.map("directory", C.Directory))
+    return false;
+  // Either "arguments" (array) or "command" (string) must be given.
+  if (!O.map("arguments", C.CommandLine)) {
+    if (auto Command = J.getAsObject()->getString("command"))
+      C.CommandLine = unescapeCommandLine(Syntax, *Command);
+    else
+      return false;
+  }
+  O.map("output", C.Output); // Output is optional.
+  return true;
+}
+
+JSONCompilationDatabase::JSONCompilationDatabase(
+    std::vector<CompileCommand> Commands)
+    : AllCommands(std::move(Commands)) {
+  unsigned Index = 0;
+  for (const CompileCommand &C : AllCommands) {
+    // Lookup and trie are keyed by native absolute paths.
+    SmallString<128> NativeFilePath;
+    if (llvm::sys::path::is_relative(C.Filename)) {
+      SmallString<128> AbsolutePath(C.Directory);
+      llvm::sys::path::append(AbsolutePath, C.Filename);
+      llvm::sys::path::native(AbsolutePath, NativeFilePath);
+    } else {
+      llvm::sys::path::native(C.Filename, NativeFilePath);
+    }
+    FileCommands[NativeFilePath].push_back(Index);
+    MatchTrie.insert(NativeFilePath);
+    ++Index;
+  }
 }
 
 std::unique_ptr<JSONCompilationDatabase>
 JSONCompilationDatabase::loadFromBuffer(StringRef DatabaseString,
                                         std::string &ErrorMessage,
                                         JSONCommandLineSyntax Syntax) {
-  std::unique_ptr<llvm::MemoryBuffer> DatabaseBuffer(
-      llvm::MemoryBuffer::getMemBuffer(DatabaseString));
-  std::unique_ptr<JSONCompilationDatabase> Database(
-      new JSONCompilationDatabase(std::move(DatabaseBuffer), Syntax));
-  if (!Database->parse(ErrorMessage))
+  auto JSON = llvm::json::parse(DatabaseString);
+  if (!JSON) {
+    ErrorMessage = llvm::toString(JSON.takeError());
     return nullptr;
-  return Database;
+  }
+  const llvm::json::Array *JSONCommands = JSON->getAsArray();
+  if (!JSONCommands) {
+    ErrorMessage = "Expected JSON compilation database to be an array";
+    return nullptr;
+  }
+  std::vector<CompileCommand> Commands;
+  for (const auto &JSONCommand : *JSONCommands) {
+    Commands.emplace_back();
+    if (!fromJSON(JSONCommand, Commands.back(), Syntax)) {
+      ErrorMessage = llvm::formatv(
+          "Failed to parse JSON compilation database entry {0:2}", JSONCommand);
+      return nullptr;
+    }
+  }
+  return std::unique_ptr<JSONCompilationDatabase>(
+      new JSONCompilationDatabase(std::move(Commands)));
 }
 
 std::vector<CompileCommand>
 JSONCompilationDatabase::getCompileCommands(StringRef FilePath) const {
   SmallString<128> NativeFilePath;
   llvm::sys::path::native(FilePath, NativeFilePath);
 
   std::string Error;
   llvm::raw_string_ostream ES(Error);
   StringRef Match = MatchTrie.findEquivalent(NativeFilePath, ES);
   if (Match.empty())
     return {};
-  const auto CommandsRefI = IndexByFile.find(Match);
-  if (CommandsRefI == IndexByFile.end())
-    return {};
   std::vector<CompileCommand> Commands;
-  getCommands(CommandsRefI->getValue(), Commands);
+  for (unsigned Index : FileCommands.lookup(Match))
+    Commands.push_back(AllCommands[Index]);
   return Commands;
 }
 
 std::vector<std::string>
 JSONCompilationDatabase::getAllFiles() const {
   std::vector<std::string> Result;
-  for (const auto &CommandRef : IndexByFile)
+  for (const auto &CommandRef : FileCommands)
     Result.push_back(CommandRef.first().str());
   return Result;
 }
 
 std::vector<CompileCommand>
 JSONCompilationDatabase::getAllCompileCommands() const {
-  std::vector<CompileCommand> Commands;
-  getCommands(AllCommands, Commands);
-  return Commands;
-}
-
-static std::vector<std::string>
-nodeToCommandLine(JSONCommandLineSyntax Syntax,
-                  const std::vector<llvm::yaml::ScalarNode *> &Nodes) {
-  SmallString<1024> Storage;
-  if (Nodes.size() == 1)
-    return unescapeCommandLine(Syntax, Nodes[0]->getValue(Storage));
-  std::vector<std::string> Arguments;
-  for (const auto *Node : Nodes)
-    Arguments.push_back(Node->getValue(Storage));
-  return Arguments;
-}
-
-void JSONCompilationDatabase::getCommands(
-    ArrayRef<CompileCommandRef> CommandsRef,
-    std::vector<CompileCommand> &Commands) const {
-  for (const auto &CommandRef : CommandsRef) {
-    SmallString<8> DirectoryStorage;
-    SmallString<32> FilenameStorage;
-    SmallString<32> OutputStorage;
-    auto Output = std::get<3>(CommandRef);
-    Commands.emplace_back(
-        std::get<0>(CommandRef)->getValue(DirectoryStorage),
-        std::get<1>(CommandRef)->getValue(FilenameStorage),
-        nodeToCommandLine(Syntax, std::get<2>(CommandRef)),
-        Output ? Output->getValue(OutputStorage) : "");
-  }
+  return AllCommands;
 }
 
-bool JSONCompilationDatabase::parse(std::string &ErrorMessage) {
-  llvm::yaml::document_iterator I = YAMLStream.begin();
-  if (I == YAMLStream.end()) {
-    ErrorMessage = "Error while parsing YAML.";
-    return false;
-  }
-  llvm::yaml::Node *Root = I->getRoot();
-  if (!Root) {
-    ErrorMessage = "Error while parsing YAML.";
-    return false;
-  }
-  auto *Array = dyn_cast<llvm::yaml::SequenceNode>(Root);
-  if (!Array) {
-    ErrorMessage = "Expected array.";
-    return false;
-  }
-  for (auto &NextObject : *Array) {
-    auto *Object = dyn_cast<llvm::yaml::MappingNode>(&NextObject);
-    if (!Object) {
-      ErrorMessage = "Expected object.";
-      return false;
-    }
-    llvm::yaml::ScalarNode *Directory = nullptr;
-    llvm::Optional<std::vector<llvm::yaml::ScalarNode *>> Command;
-    llvm::yaml::ScalarNode *File = nullptr;
-    llvm::yaml::ScalarNode *Output = nullptr;
-    for (auto& NextKeyValue : *Object) {
-      auto *KeyString = dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
-      if (!KeyString) {
-        ErrorMessage = "Expected strings as key.";
-        return false;
-      }
-      SmallString<10> KeyStorage;
-      StringRef KeyValue = KeyString->getValue(KeyStorage);
-      llvm::yaml::Node *Value = NextKeyValue.getValue();
-      if (!Value) {
-        ErrorMessage = "Expected value.";
-        return false;
-      }
-      auto *ValueString = dyn_cast<llvm::yaml::ScalarNode>(Value);
-      auto *SequenceString = dyn_cast<llvm::yaml::SequenceNode>(Value);
-      if (KeyValue == "arguments" && !SequenceString) {
-        ErrorMessage = "Expected sequence as value.";
-        return false;
-      } else if (KeyValue != "arguments" && !ValueString) {
-        ErrorMessage = "Expected string as value.";
-        return false;
-      }
-      if (KeyValue == "directory") {
-        Directory = ValueString;
-      } else if (KeyValue == "arguments") {
-        Command = std::vector<llvm::yaml::ScalarNode *>();
-        for (auto &Argument : *SequenceString) {
-          auto *Scalar = dyn_cast<llvm::yaml::ScalarNode>(&Argument);
-          if (!Scalar) {
-            ErrorMessage = "Only strings are allowed in 'arguments'.";
-            return false;
-          }
-          Command->push_back(Scalar);
-        }
-      } else if (KeyValue == "command") {
-        if (!Command)
-          Command = std::vector<llvm::yaml::ScalarNode *>(1, ValueString);
-      } else if (KeyValue == "file") {
-        File = ValueString;
-      } else if (KeyValue == "output") {
-        Output = ValueString;
-      } else {
-        ErrorMessage = ("Unknown key: \"" +
-                        KeyString->getRawValue() + "\"").str();
-        return false;
-      }
-    }
-    if (!File) {
-      ErrorMessage = "Missing key: \"file\".";
-      return false;
-    }
-    if (!Command) {
-      ErrorMessage = "Missing key: \"command\" or \"arguments\".";
-      return false;
-    }
-    if (!Directory) {
-      ErrorMessage = "Missing key: \"directory\".";
-      return false;
-    }
-    SmallString<8> FileStorage;
-    StringRef FileName = File->getValue(FileStorage);
-    SmallString<128> NativeFilePath;
-    if (llvm::sys::path::is_relative(FileName)) {
-      SmallString<8> DirectoryStorage;
-      SmallString<128> AbsolutePath(
-          Directory->getValue(DirectoryStorage));
-      llvm::sys::path::append(AbsolutePath, FileName);
-      llvm::sys::path::native(AbsolutePath, NativeFilePath);
-    } else {
-      llvm::sys::path::native(FileName, NativeFilePath);
-    }
-    auto Cmd = CompileCommandRef(Directory, File, *Command, Output);
-    IndexByFile[NativeFilePath].push_back(Cmd);
-    AllCommands.push_back(Cmd);
-    MatchTrie.insert(NativeFilePath);
-  }
-  return true;
-}

test/Index/skip-parsed-bodies/compile_commands.json

[
   "file": "t2.cpp"
 },
 {
   "directory": ".",
   "command": "/usr/bin/clang++ -fsyntax-only -fno-ms-compatibility -fno-delayed-template-parsing t3.cpp -DBLAH",
   "file": "t2.cpp"
 }
 ]
-
-// RUN: c-index-test -index-compile-db %s | FileCheck %s
-
-// CHECK:      [startedTranslationUnit]
-// CHECK-NEXT: [enteredMainFile]: t1.cpp
-// CHECK:      [indexDeclaration]: kind: c++-instance-method | name: method_decl | {{.*}} | isRedecl: 0 | isDef: 0 | isContainer: 0
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def1 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: 1
-// CHECK-NEXT: [indexEntityReference]: kind: variable | name: some_val | {{.*}} | loc: ./t.h:9:27
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def2 | {{.*}} | isRedecl: 0 | isDef: 0 | isContainer: 0
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def2 | {{.*}} | isRedecl: 1 | isDef: 1 | isContainer: 1
-// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: NS |
-// CHECK-NEXT: [indexEntityReference]: kind: c++-class | name: C |
-// CHECK-NEXT: [indexEntityReference]: kind: variable | name: some_val | {{.*}} | loc: ./t.h:15:5
-// CHECK-NEXT: [indexDeclaration]: kind: function | name: foo1 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: 1
-// CHECK-NEXT: [indexEntityReference]: kind: variable | name: some_val | {{.*}} | loc: ./t.h:19:5
-// CHECK-NEXT: [diagnostic]: {{.*}} undeclared identifier 'undef_val1'
-// CHECK-NEXT: [diagnostic]: {{.*}} undeclared identifier 'undef_val2'
-// CHECK-NEXT: [diagnostic]: {{.*}} undeclared identifier 'undef_val3'
-
-// CHECK-NEXT: [startedTranslationUnit]
-// CHECK-NEXT: [enteredMainFile]: t2.cpp
-// CHECK:      [indexDeclaration]: kind: c++-instance-method | name: method_decl | {{.*}} | isRedecl: 0 | isDef: 0 | isContainer: 0
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def1 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def2 | {{.*}} | isRedecl: 0 | isDef: 0 | isContainer: 0
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def2 | {{.*}} | isContainer: skipped
-// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: NS |
-// CHECK-NEXT: [indexEntityReference]: kind: c++-class | name: C |
-// CHECK-NEXT: [indexDeclaration]: kind: function | name: foo1 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
-// CHECK-NEXT: [ppIncludedFile]: ./pragma_once.h
-// CHECK-NEXT: [indexDeclaration]: kind: function | name: foo2 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: 1
-// CHECK-NEXT: [indexEntityReference]: kind: variable | name: some_val | {{.*}} | loc: ./t.h:25:5
-// CHECK:      [indexDeclaration]: kind: c++-instance-method | name: tsmeth | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: 1
-// CHECK-NEXT: [indexEntityReference]: kind: variable | name: some_val | {{.*}} | loc: ./pragma_once.h:8:7
-// CHECK:      [indexDeclaration]: kind: function | name: imp_foo | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: 1
-// CHECK-NEXT: [indexEntityReference]: kind: variable | name: some_val | {{.*}} | loc: ./imported.h:4:5
-// CHECK-NEXT: [diagnostic]: {{.*}} undeclared identifier 'undef_val4'
-// CHECK-NEXT: [diagnostic]: {{.*}} undeclared identifier 'undef_tsval'
-// CHECK-NEXT: [diagnostic]: {{.*}} undeclared identifier 'undef_impval'
-
-// CHECK-NEXT: [startedTranslationUnit]
-// CHECK-NEXT: [enteredMainFile]: t3.cpp
-// CHECK:      [indexDeclaration]: kind: c++-instance-method | name: method_decl | {{.*}} | isRedecl: 0 | isDef: 0 | isContainer: 0
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def1 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def2 | {{.*}} | isRedecl: 0 | isDef: 0 | isContainer: 0
-// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def2 | {{.*}} | isRedecl: 1 | isDef: 1 | isContainer: skipped
-// CHECK-NEXT: [indexEntityReference]: kind: namespace | name: NS |
-// CHECK-NEXT: [indexEntityReference]: kind: c++-class | name: C |
-// CHECK-NEXT: [indexDeclaration]: kind: function | name: foo1 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
-// CHECK-NEXT: [ppIncludedFile]: ./pragma_once.h
-// CHECK-NEXT: [indexDeclaration]: kind: function | name: foo2 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
-// CHECK-NEXT: [indexDeclaration]: kind: variable | {{.*}} | loc: ./pragma_once.h:3:12
-// CHECK:      [indexDeclaration]: kind: c++-instance-method | name: tsmeth | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
-// CHECK-NOT:  [indexEntityReference]: kind: variable | name: some_val |
-// CHECK:      [indexDeclaration]: kind: function | name: imp_foo | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
-// CHECK-NOT:  [indexEntityReference]: kind: variable | name: some_val |
-// CHECK-NOT:  [diagnostic]: {{.*}} undeclared identifier

test/Index/skip-parsed-bodies/compile_commands.test

-[
-{
-  "directory": ".",
-  "command": "/usr/bin/clang++ -fsyntax-only -fno-ms-compatibility -fno-delayed-template-parsing t1.cpp",
-  "file": "t1.cpp"
-},
-{
-  "directory": ".",
-  "command": "/usr/bin/clang++ -fsyntax-only -fno-ms-compatibility -fno-delayed-template-parsing t2.cpp -DBLAH",
-  "file": "t2.cpp"
-},
-{
-  "directory": ".",
-  "command": "/usr/bin/clang++ -fsyntax-only -fno-ms-compatibility -fno-delayed-template-parsing t3.cpp -DBLAH",
-  "file": "t2.cpp"
-}
-]
-
-// RUN: c-index-test -index-compile-db %s | FileCheck %s
+// RUN: c-index-test -index-compile-db %S/compile_commands.json | FileCheck %s
 
 // CHECK:      [startedTranslationUnit]
 // CHECK-NEXT: [enteredMainFile]: t1.cpp
 // CHECK:      [indexDeclaration]: kind: c++-instance-method | name: method_decl | {{.*}} | isRedecl: 0 | isDef: 0 | isContainer: 0
 // CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def1 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: 1
 // CHECK-NEXT: [indexEntityReference]: kind: variable | name: some_val | {{.*}} | loc: ./t.h:9:27
 // CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def2 | {{.*}} | isRedecl: 0 | isDef: 0 | isContainer: 0
 // CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: method_def2 | {{.*}} | isRedecl: 1 | isDef: 1 | isContainer: 1

 // CHECK-NEXT: [ppIncludedFile]: ./pragma_once.h
 // CHECK-NEXT: [indexDeclaration]: kind: function | name: foo2 | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
 // CHECK-NEXT: [indexDeclaration]: kind: variable | {{.*}} | loc: ./pragma_once.h:3:12
 // CHECK:      [indexDeclaration]: kind: c++-instance-method | name: tsmeth | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
 // CHECK-NOT:  [indexEntityReference]: kind: variable | name: some_val |
 // CHECK:      [indexDeclaration]: kind: function | name: imp_foo | {{.*}} | isRedecl: 0 | isDef: 1 | isContainer: skipped
 // CHECK-NOT:  [indexEntityReference]: kind: variable | name: some_val |
 // CHECK-NOT:  [diagnostic]: {{.*}} undeclared identifier
+

test/Index/skip-parsed-bodies/lit.local.cfg

-config.suffixes = ['.json']
+config.suffixes = ['.test']