Cleanup clang::TranslationUnit API so callers have to handle failures.

src/clang_complete.cc

@@ -283,10 +283,10 @@ void BuildDetailString(CXCompletionString completion_string,
   }
 }
 
-void EnsureDocumentParsed(ClangCompleteManager* manager,
+void TryEnsureDocumentParsed(ClangCompleteManager* manager,
-                          std::shared_ptr<CompletionSession> session,
+                             std::shared_ptr<CompletionSession> session,
-                          std::unique_ptr<clang::TranslationUnit>* tu,
+                             std::unique_ptr<clang::TranslationUnit>* tu,
-                          clang::Index* index) {
+                             clang::Index* index) {
   // Nothing to do. We already have a translation unit.
   if (*tu)
     return;
@@ -306,15 +306,14 @@ void EnsureDocumentParsed(ClangCompleteManager* manager,
 
   LOG_S(INFO) << "Creating completion session with arguments "
               << StringJoin(args);
-  *tu = MakeUnique<clang::TranslationUnit>(index, session->file.filename, args,
+  *tu = clang::TranslationUnit::Create(index, session->file.filename, args,
-                                           unsaved, Flags());
+                                       unsaved, Flags());
 
   // Build diagnostics.
-  if (manager->config_->diagnosticsOnParse && !(*tu)->did_fail) {
+  if (manager->config_->diagnosticsOnParse && *tu) {
-    // If we're emitting diagnostics, do an immedaite reparse, otherwise we will
+    // If we're emitting diagnostics, do an immediate reparse, otherwise we will
     // emit stale/bad diagnostics.
-    clang_reparseTranslationUnit((*tu)->cx_tu, unsaved.size(), unsaved.data(),
+    *tu = clang::TranslationUnit::Reparse(std::move(*tu), unsaved);
-                                 clang_defaultReparseOptions((*tu)->cx_tu));
 
     NonElidedVector<lsDiagnostic> ls_diagnostics;
     unsigned num_diagnostics = clang_getNumDiagnostics((*tu)->cx_tu);
@@ -350,8 +349,8 @@ void CompletionParseMain(ClangCompleteManager* completion_manager) {
     }
 
     std::unique_ptr<clang::TranslationUnit> parsing;
-    EnsureDocumentParsed(completion_manager, session, &parsing,
+    TryEnsureDocumentParsed(completion_manager, session, &parsing,
-                         &session->index);
+                            &session->index);
 
     // Activate new translation unit.
     // tu_last_parsed_at is only read by this thread, so it doesn't need to be
@@ -374,9 +373,14 @@ void CompletionQueryMain(ClangCompleteManager* completion_manager) {
 
     std::lock_guard<std::mutex> lock(session->tu_lock);
     Timer timer;
-    EnsureDocumentParsed(completion_manager, session, &session->tu,
+    TryEnsureDocumentParsed(completion_manager, session, &session->tu,
-                         &session->index);
+                            &session->index);
-    timer.ResetAndPrint("[complete] EnsureDocumentParsed");
+    timer.ResetAndPrint("[complete] TryEnsureDocumentParsed");
+
+    // It is possible we failed to create the document despite
+    // |TryEnsureDocumentParsed|.
+    if (!session->tu)
+      continue;
 
     std::vector<CXUnsavedFile> unsaved =
         completion_manager->working_files_->AsUnsavedFiles();
@@ -458,10 +462,14 @@ void CompletionQueryMain(ClangCompleteManager* completion_manager) {
       // faster than reparsing the document.
 
       timer.Reset();
-      clang_reparseTranslationUnit(
+      session->tu =
-          session->tu->cx_tu, unsaved.size(), unsaved.data(),
+          clang::TranslationUnit::Reparse(std::move(session->tu), unsaved);
-          clang_defaultReparseOptions(session->tu->cx_tu));
       timer.ResetAndPrint("[complete] clang_reparseTranslationUnit");
+      if (!session->tu) {
+        LOG_S(ERROR) << "Reparsing translation unit for diagnostics failed for "
+                     << path;
+        continue;
+      }
 
       size_t num_diagnostics = clang_getNumDiagnostics(session->tu->cx_tu);
       NonElidedVector<lsDiagnostic> ls_diagnostics;

src/indexer.cc

@@ -1619,21 +1619,19 @@ std::vector<std::unique_ptr<IndexFile>> Parse(
     unsaved_files.push_back(unsaved);
   }
 
-  clang::TranslationUnit tu(index, file, args, unsaved_files,
+  std::unique_ptr<clang::TranslationUnit> tu = clang::TranslationUnit::Create(
+      index, file, args, unsaved_files,
                             CXTranslationUnit_KeepGoing |
                                 CXTranslationUnit_DetailedPreprocessingRecord);
-  if (tu.did_fail) {
+  if (!tu)
-    std::cerr << "!! Failed creating translation unit for " << file
-              << std::endl;
     return {};
-  }
 
   perf->index_parse = timer.ElapsedMicrosecondsAndReset();
 
   if (dump_ast)
-    Dump(tu.document_cursor());
+    Dump(tu->document_cursor());
 
-  return ParseWithTu(file_consumer_shared, perf, &tu, index, file, args,
+  return ParseWithTu(file_consumer_shared, perf, tu.get(), index, file, args,
                      unsaved_files);
 }
 

src/libclangmm/TranslationUnit.cc

@@ -4,20 +4,22 @@
 #include "../utils.h"
 #include "Utility.h"
 
+#include <loguru.hpp>
 
 #include <cassert>
 #include <fstream>
 #include <iostream>
 #include <sstream>
 
-
 namespace clang {
 
-TranslationUnit::TranslationUnit(Index* index,
+// static
-                                 const std::string& filepath,
+std::unique_ptr<TranslationUnit> TranslationUnit::Create(
-                                 const std::vector<std::string>& arguments,
+    Index* index,
-                                 std::vector<CXUnsavedFile> unsaved_files,
+    const std::string& filepath,
-                                 unsigned flags) {
+    const std::vector<std::string>& arguments,
+    std::vector<CXUnsavedFile> unsaved_files,
+    unsigned flags) {
   std::vector<const char*> args;
   for (const std::string& a : arguments)
     args.push_back(a.c_str());
@@ -26,76 +28,68 @@ TranslationUnit::TranslationUnit(Index* index,
   for (const auto& arg : platform_args)
     args.push_back(arg.c_str());
 
-  // std::cerr << "Parsing " << filepath << " with args " << StringJoin(args) <<
+  CXTranslationUnit cx_tu;
-  // std::endl;
-
-  // cx_tu = clang_createTranslationUnitFromSourceFile(
-  //  index->cx_index, filepath.c_str(), args.size(), args.data(),
-  //  (unsigned)unsaved_files.size(), unsaved_files.data());
-
   CXErrorCode error_code = clang_parseTranslationUnit2FullArgv(
       index->cx_index, filepath.c_str(), args.data(), (int)args.size(),
       unsaved_files.data(), (unsigned)unsaved_files.size(), flags, &cx_tu);
 
   switch (error_code) {
     case CXError_Success:
-      did_fail = false;
+      return MakeUnique<TranslationUnit>(cx_tu);
-      break;
     case CXError_Failure:
-      std::cerr << "libclang generic failure for " << filepath << " with args "
+      LOG_S(ERROR) << "libclang generic failure for " << filepath
-                << StringJoin(args) << std::endl;
+                   << " with args " << StringJoin(args);
-      did_fail = true;
+      return nullptr;
-      break;
     case CXError_Crashed:
-      std::cerr << "libclang crashed for " << filepath << " with args "
+      LOG_S(ERROR) << "libclang crashed for " << filepath << " with args "
-                << StringJoin(args) << std::endl;
+                   << StringJoin(args);
-      did_fail = true;
+      return nullptr;
-      break;
     case CXError_InvalidArguments:
-      std::cerr << "libclang had invalid arguments for "
+      LOG_S(ERROR) << "libclang had invalid arguments for "
-                << " with args " << StringJoin(args) << filepath << std::endl;
+                   << " with args " << StringJoin(args) << filepath;
-      did_fail = true;
+      return nullptr;
-      break;
     case CXError_ASTReadError:
-      std::cerr << "libclang had ast read error for " << filepath
+      LOG_S(ERROR) << "libclang had ast read error for " << filepath
-                << " with args " << StringJoin(args) << std::endl;
+                   << " with args " << StringJoin(args);
-      did_fail = true;
+      return nullptr;
-      break;
   }
+
+  return nullptr;
 }
 
+// static
+std::unique_ptr<TranslationUnit> TranslationUnit::Reparse(
+    std::unique_ptr<TranslationUnit> tu,
+    std::vector<CXUnsavedFile>& unsaved) {
+  int error_code = clang_reparseTranslationUnit(
+      tu->cx_tu, (unsigned)unsaved.size(), unsaved.data(),
+      clang_defaultReparseOptions(tu->cx_tu));
+  switch (error_code) {
+    case CXError_Success:
+      return tu;
+    case CXError_Failure:
+      LOG_S(ERROR) << "libclang reparse generic failure";
+      return nullptr;
+    case CXError_Crashed:
+      LOG_S(ERROR) << "libclang reparse crashed";
+      return nullptr;
+    case CXError_InvalidArguments:
+      LOG_S(ERROR) << "libclang reparse had invalid arguments";
+      return nullptr;
+    case CXError_ASTReadError:
+      LOG_S(ERROR) << "libclang reparse had ast read error";
+      return nullptr;
+  }
+
+  return nullptr;
+}
+
+TranslationUnit::TranslationUnit(CXTranslationUnit tu) : cx_tu(tu) {}
+
 TranslationUnit::~TranslationUnit() {
   clang_disposeTranslationUnit(cx_tu);
 }
 
-void TranslationUnit::ReparseTranslationUnit(
-    std::vector<CXUnsavedFile>& unsaved) {
-  int error_code = clang_reparseTranslationUnit(
-      cx_tu, (unsigned)unsaved.size(), unsaved.data(),
-      clang_defaultReparseOptions(cx_tu));
-  switch (error_code) {
-    case CXError_Success:
-      did_fail = false;
-      break;
-    case CXError_Failure:
-      std::cerr << "libclang reparse generic failure" << std::endl;
-      did_fail = true;
-      break;
-    case CXError_Crashed:
-      std::cerr << "libclang reparse crashed " << std::endl;
-      did_fail = true;
-      break;
-    case CXError_InvalidArguments:
-      std::cerr << "libclang reparse had invalid arguments" << std::endl;
-      did_fail = true;
-      break;
-    case CXError_ASTReadError:
-      std::cerr << "libclang reparse had ast read error" << std::endl;
-      did_fail = true;
-      break;
-  }
-}
-
 Cursor TranslationUnit::document_cursor() const {
   return Cursor(clang_getTranslationUnitCursor(cx_tu));
 }

src/libclangmm/TranslationUnit.h

@@ -3,9 +3,9 @@
 #include "Cursor.h"
 #include "Index.h"
 
-
 #include <clang-c/Index.h>
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -13,17 +13,20 @@ namespace clang {
 
 class TranslationUnit {
  public:
-  TranslationUnit(Index* index,
+  static std::unique_ptr<TranslationUnit> Create(
-                  const std::string& filepath,
+      Index* index,
-                  const std::vector<std::string>& arguments,
+      const std::string& filepath,
-                  std::vector<CXUnsavedFile> unsaved_files,
+      const std::vector<std::string>& arguments,
-                  unsigned flags);
+      std::vector<CXUnsavedFile> unsaved_files,
+      unsigned flags);
+
+  static std::unique_ptr<TranslationUnit> Reparse(
+      std::unique_ptr<TranslationUnit> tu,
+      std::vector<CXUnsavedFile>& unsaved);
+
+  explicit TranslationUnit(CXTranslationUnit tu);
   ~TranslationUnit();
 
-  bool did_fail = false;
-
-  void ReparseTranslationUnit(std::vector<CXUnsavedFile>& unsaved);
-
   Cursor document_cursor() const;
 
   CXTranslationUnit cx_tu;