From 3e9cffcc62ca6013404dec848220c6c72984f8fb Mon Sep 17 00:00:00 2001 From: Jacob Dufault Date: Sun, 22 Oct 2017 22:07:50 -0700 Subject: [PATCH] Cleanup clang::TranslationUnit API so callers have to handle failures. --- src/clang_complete.cc | 44 +++++++----- src/indexer.cc | 12 ++-- src/libclangmm/TranslationUnit.cc | 112 ++++++++++++++---------------- src/libclangmm/TranslationUnit.h | 23 +++--- 4 files changed, 97 insertions(+), 94 deletions(-) diff --git a/src/clang_complete.cc b/src/clang_complete.cc index a6a8a0ff..ccd0f79b 100644 --- a/src/clang_complete.cc +++ b/src/clang_complete.cc @@ -283,10 +283,10 @@ void BuildDetailString(CXCompletionString completion_string, } } -void EnsureDocumentParsed(ClangCompleteManager* manager, - std::shared_ptr session, - std::unique_ptr* tu, - clang::Index* index) { +void TryEnsureDocumentParsed(ClangCompleteManager* manager, + std::shared_ptr session, + std::unique_ptr* tu, + 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(index, session->file.filename, args, - unsaved, Flags()); + *tu = clang::TranslationUnit::Create(index, session->file.filename, args, + unsaved, Flags()); // Build diagnostics. - if (manager->config_->diagnosticsOnParse && !(*tu)->did_fail) { - // If we're emitting diagnostics, do an immedaite reparse, otherwise we will + if (manager->config_->diagnosticsOnParse && *tu) { + // 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(), - clang_defaultReparseOptions((*tu)->cx_tu)); + *tu = clang::TranslationUnit::Reparse(std::move(*tu), unsaved); NonElidedVector ls_diagnostics; unsigned num_diagnostics = clang_getNumDiagnostics((*tu)->cx_tu); @@ -350,8 +349,8 @@ void CompletionParseMain(ClangCompleteManager* completion_manager) { } std::unique_ptr parsing; - EnsureDocumentParsed(completion_manager, session, &parsing, - &session->index); + TryEnsureDocumentParsed(completion_manager, session, &parsing, + &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 lock(session->tu_lock); Timer timer; - EnsureDocumentParsed(completion_manager, session, &session->tu, - &session->index); - timer.ResetAndPrint("[complete] EnsureDocumentParsed"); + TryEnsureDocumentParsed(completion_manager, session, &session->tu, + &session->index); + timer.ResetAndPrint("[complete] TryEnsureDocumentParsed"); + + // It is possible we failed to create the document despite + // |TryEnsureDocumentParsed|. + if (!session->tu) + continue; std::vector 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->cx_tu, unsaved.size(), unsaved.data(), - clang_defaultReparseOptions(session->tu->cx_tu)); + session->tu = + clang::TranslationUnit::Reparse(std::move(session->tu), unsaved); 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 ls_diagnostics; diff --git a/src/indexer.cc b/src/indexer.cc index cc3f5135..172f2a19 100644 --- a/src/indexer.cc +++ b/src/indexer.cc @@ -1619,21 +1619,19 @@ std::vector> Parse( unsaved_files.push_back(unsaved); } - clang::TranslationUnit tu(index, file, args, unsaved_files, + std::unique_ptr tu = clang::TranslationUnit::Create( + index, file, args, unsaved_files, CXTranslationUnit_KeepGoing | CXTranslationUnit_DetailedPreprocessingRecord); - if (tu.did_fail) { - std::cerr << "!! Failed creating translation unit for " << file - << std::endl; + if (!tu) 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); } diff --git a/src/libclangmm/TranslationUnit.cc b/src/libclangmm/TranslationUnit.cc index c957ad01..92b6ee8c 100644 --- a/src/libclangmm/TranslationUnit.cc +++ b/src/libclangmm/TranslationUnit.cc @@ -4,20 +4,22 @@ #include "../utils.h" #include "Utility.h" +#include #include #include #include #include - namespace clang { -TranslationUnit::TranslationUnit(Index* index, - const std::string& filepath, - const std::vector& arguments, - std::vector unsaved_files, - unsigned flags) { +// static +std::unique_ptr TranslationUnit::Create( + Index* index, + const std::string& filepath, + const std::vector& arguments, + std::vector unsaved_files, + unsigned flags) { std::vector 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) << - // std::endl; - - // cx_tu = clang_createTranslationUnitFromSourceFile( - // index->cx_index, filepath.c_str(), args.size(), args.data(), - // (unsigned)unsaved_files.size(), unsaved_files.data()); - + CXTranslationUnit cx_tu; 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; - break; + return MakeUnique(cx_tu); case CXError_Failure: - std::cerr << "libclang generic failure for " << filepath << " with args " - << StringJoin(args) << std::endl; - did_fail = true; - break; + LOG_S(ERROR) << "libclang generic failure for " << filepath + << " with args " << StringJoin(args); + return nullptr; case CXError_Crashed: - std::cerr << "libclang crashed for " << filepath << " with args " - << StringJoin(args) << std::endl; - did_fail = true; - break; + LOG_S(ERROR) << "libclang crashed for " << filepath << " with args " + << StringJoin(args); + return nullptr; case CXError_InvalidArguments: - std::cerr << "libclang had invalid arguments for " - << " with args " << StringJoin(args) << filepath << std::endl; - did_fail = true; - break; + LOG_S(ERROR) << "libclang had invalid arguments for " + << " with args " << StringJoin(args) << filepath; + return nullptr; case CXError_ASTReadError: - std::cerr << "libclang had ast read error for " << filepath - << " with args " << StringJoin(args) << std::endl; - did_fail = true; - break; + LOG_S(ERROR) << "libclang had ast read error for " << filepath + << " with args " << StringJoin(args); + return nullptr; } + + return nullptr; } +// static +std::unique_ptr TranslationUnit::Reparse( + std::unique_ptr tu, + std::vector& 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& 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)); } diff --git a/src/libclangmm/TranslationUnit.h b/src/libclangmm/TranslationUnit.h index cc53701d..ba1c172d 100644 --- a/src/libclangmm/TranslationUnit.h +++ b/src/libclangmm/TranslationUnit.h @@ -3,9 +3,9 @@ #include "Cursor.h" #include "Index.h" - #include +#include #include #include @@ -13,17 +13,20 @@ namespace clang { class TranslationUnit { public: - TranslationUnit(Index* index, - const std::string& filepath, - const std::vector& arguments, - std::vector unsaved_files, - unsigned flags); + static std::unique_ptr Create( + Index* index, + const std::string& filepath, + const std::vector& arguments, + std::vector unsaved_files, + unsigned flags); + + static std::unique_ptr Reparse( + std::unique_ptr tu, + std::vector& unsaved); + + explicit TranslationUnit(CXTranslationUnit tu); ~TranslationUnit(); - bool did_fail = false; - - void ReparseTranslationUnit(std::vector& unsaved); - Cursor document_cursor() const; CXTranslationUnit cx_tu;