From 7b7b1823cbfbf95589bf1f8fc07253f3f5ad7477 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Fri, 22 Apr 2016 15:31:49 +0200 Subject: [PATCH] Clang: Avoid parse loop if libclang crashed or file vanished Remember whether clang_parseTranslationUnit() or clang_reparseTranslationUnit() failed the last time and do not trigger parse/reparse again. Also, check whether the main file exists before reparsing. Task-number: QTCREATORBUG-16051 Task-number: QTCREATORBUG-16140 Change-Id: Ied39e66a18032854911229898573941fe2ada35b Reviewed-by: Erik Verbruggen --- .../ipcsource/clangbackendclangipc-source.pri | 2 + .../ipcsource/clangtranslationunit.cpp | 77 ++++++++++++++----- .../ipcsource/clangtranslationunit.h | 8 +- .../translationunitparseerrorexception.cpp | 5 +- .../translationunitreparseerrorexception.cpp | 68 ++++++++++++++++ .../translationunitreparseerrorexception.h | 61 +++++++++++++++ .../ipcsource/translationunits.cpp | 4 +- tests/unit/unittest/translationunitstest.cpp | 26 +++++++ tests/unit/unittest/translationunittest.cpp | 27 ++++++- 9 files changed, 251 insertions(+), 27 deletions(-) create mode 100644 src/tools/clangbackend/ipcsource/translationunitreparseerrorexception.cpp create mode 100644 src/tools/clangbackend/ipcsource/translationunitreparseerrorexception.h diff --git a/src/tools/clangbackend/ipcsource/clangbackendclangipc-source.pri b/src/tools/clangbackend/ipcsource/clangbackendclangipc-source.pri index 344a0372076..6140f1ca888 100644 --- a/src/tools/clangbackend/ipcsource/clangbackendclangipc-source.pri +++ b/src/tools/clangbackend/ipcsource/clangbackendclangipc-source.pri @@ -12,6 +12,7 @@ HEADERS += $$PWD/clangipcserver.h \ $$PWD/projects.h \ $$PWD/translationunits.h \ $$PWD/translationunitparseerrorexception.h \ + $$PWD/translationunitreparseerrorexception.h \ $$PWD/projectpart.h \ $$PWD/translationunitfilenotexitexception.h \ $$PWD/translationunitdoesnotexistexception.h \ @@ -46,6 +47,7 @@ SOURCES += $$PWD/clangipcserver.cpp \ $$PWD/projects.cpp \ $$PWD/translationunits.cpp \ $$PWD/translationunitparseerrorexception.cpp \ + $$PWD/translationunitreparseerrorexception.cpp \ $$PWD/projectpart.cpp \ $$PWD/translationunitfilenotexitexception.cpp \ $$PWD/translationunitdoesnotexistexception.cpp \ diff --git a/src/tools/clangbackend/ipcsource/clangtranslationunit.cpp b/src/tools/clangbackend/ipcsource/clangtranslationunit.cpp index 3460e6ff46d..0644c53e3f0 100644 --- a/src/tools/clangbackend/ipcsource/clangtranslationunit.cpp +++ b/src/tools/clangbackend/ipcsource/clangtranslationunit.cpp @@ -39,6 +39,7 @@ #include "translationunitfilenotexitexception.h" #include "translationunitisnullexception.h" #include "translationunitparseerrorexception.h" +#include "translationunitreparseerrorexception.h" #include "translationunits.h" #include "unsavedfiles.h" @@ -76,6 +77,8 @@ public: Utf8StringVector fileArguments; Utf8String filePath; CXTranslationUnit translationUnit = nullptr; + CXErrorCode parseErrorCode = CXError_Success; + int reparseErrorCode = 0; CXIndex index = nullptr; uint documentRevision = 0; bool needsToBeReparsed = false; @@ -155,6 +158,16 @@ void TranslationUnit::reparse() const reparseTranslationUnit(); } +bool TranslationUnit::parseWasSuccessful() const +{ + return d->parseErrorCode == CXError_Success; +} + +bool TranslationUnit::reparseWasSuccessful() const +{ + return d->reparseErrorCode == 0; +} + CXIndex TranslationUnit::index() const { checkIfNull(); @@ -350,7 +363,7 @@ void TranslationUnit::checkIfNull() const void TranslationUnit::checkIfFileExists() const { - if (!QFileInfo::exists(d->filePath.toString())) + if (!fileExists()) throw TranslationUnitFileNotExitsException(d->filePath); } @@ -396,16 +409,16 @@ void TranslationUnit::createTranslationUnitIfNeeded() const if (isVerboseModeEnabled()) args.print(); - CXErrorCode errorCode = clang_parseTranslationUnit2(index(), - NULL, - args.data(), - args.count(), - unsavedFiles().cxUnsavedFiles(), - unsavedFiles().count(), - defaultOptions(), - &d->translationUnit); + d->parseErrorCode = clang_parseTranslationUnit2(index(), + NULL, + args.data(), + args.count(), + unsavedFiles().cxUnsavedFiles(), + unsavedFiles().count(), + defaultOptions(), + &d->translationUnit); - checkTranslationUnitErrorCode(errorCode); + checkParseErrorCode(); updateIncludeFilePaths(); @@ -413,22 +426,33 @@ void TranslationUnit::createTranslationUnitIfNeeded() const } } -void TranslationUnit::checkTranslationUnitErrorCode(CXErrorCode errorCode) const +void TranslationUnit::checkParseErrorCode() const { - switch (errorCode) { - case CXError_Success: break; - default: throw TranslationUnitParseErrorException(d->filePath, - d->projectPart.projectPartId(), - errorCode); + if (!parseWasSuccessful()) { + throw TranslationUnitParseErrorException(d->filePath, + d->projectPart.projectPartId(), + d->parseErrorCode); + } +} + +void TranslationUnit::checkReparseErrorCode() const +{ + if (!reparseWasSuccessful()) { + throw TranslationUnitReparseErrorException(d->filePath, + d->projectPart.projectPartId(), + d->reparseErrorCode); } } void TranslationUnit::reparseTranslationUnit() const { - clang_reparseTranslationUnit(d->translationUnit, - unsavedFiles().count(), - unsavedFiles().cxUnsavedFiles(), - clang_defaultReparseOptions(d->translationUnit)); + d->reparseErrorCode = clang_reparseTranslationUnit( + d->translationUnit, + unsavedFiles().count(), + unsavedFiles().cxUnsavedFiles(), + clang_defaultReparseOptions(d->translationUnit)); + + checkReparseErrorCode(); updateIncludeFilePaths(); @@ -474,6 +498,19 @@ void TranslationUnit::updateIncludeFilePaths() const d->translationUnits.addWatchedFiles(d->dependedFilePaths); } +bool TranslationUnit::fileExists() const +{ + return QFileInfo::exists(d->filePath.toString()); +} + +bool TranslationUnit::isIntact() const +{ + return !isNull() + && fileExists() + && parseWasSuccessful() + && reparseWasSuccessful(); +} + CommandLineArguments TranslationUnit::commandLineArguments() const { return CommandLineArguments(d->filePath.constData(), diff --git a/src/tools/clangbackend/ipcsource/clangtranslationunit.h b/src/tools/clangbackend/ipcsource/clangtranslationunit.h index 9a311b62bfd..553a3f28de5 100644 --- a/src/tools/clangbackend/ipcsource/clangtranslationunit.h +++ b/src/tools/clangbackend/ipcsource/clangtranslationunit.h @@ -91,6 +91,8 @@ public: void reset(); void reparse() const; + bool isIntact() const; + CXIndex index() const; CXTranslationUnit cxTranslationUnit() const; CXTranslationUnit cxTranslationUnitWithoutReparsing() const; @@ -150,10 +152,14 @@ private: bool projectPartIsOutdated() const; bool isMainFileAndExistsOrIsOtherFile(const Utf8String &filePath) const; void createTranslationUnitIfNeeded() const; - void checkTranslationUnitErrorCode(CXErrorCode errorCode) const; + void checkParseErrorCode() const; + void checkReparseErrorCode() const; void reparseTranslationUnit() const; void reparseTranslationUnitIfFilesAreChanged() const; + bool parseWasSuccessful() const; + bool reparseWasSuccessful() const; void updateIncludeFilePaths() const; + bool fileExists() const; static void includeCallback(CXFile included_file, CXSourceLocation * /*inclusion_stack*/, unsigned /*include_len*/, diff --git a/src/tools/clangbackend/ipcsource/translationunitparseerrorexception.cpp b/src/tools/clangbackend/ipcsource/translationunitparseerrorexception.cpp index 3cd99c53e8a..27f1799e09d 100644 --- a/src/tools/clangbackend/ipcsource/translationunitparseerrorexception.cpp +++ b/src/tools/clangbackend/ipcsource/translationunitparseerrorexception.cpp @@ -65,12 +65,13 @@ static const char *errorCodeToText(CXErrorCode errorCode) const char *TranslationUnitParseErrorException::what() const Q_DECL_NOEXCEPT { if (what_.isEmpty()) { - what_ += Utf8StringLiteral("Parse error for file ") + what_ += Utf8StringLiteral("clang_parseTranslationUnit() failed for file ") + filePath() + Utf8StringLiteral(" in project ") + projectPartId() + Utf8StringLiteral(": ") - + Utf8String::fromUtf8(errorCodeToText(errorCode_)); + + Utf8String::fromUtf8(errorCodeToText(errorCode_)) + + Utf8StringLiteral("."); } return what_.constData(); diff --git a/src/tools/clangbackend/ipcsource/translationunitreparseerrorexception.cpp b/src/tools/clangbackend/ipcsource/translationunitreparseerrorexception.cpp new file mode 100644 index 00000000000..6f1e7f0d7de --- /dev/null +++ b/src/tools/clangbackend/ipcsource/translationunitreparseerrorexception.cpp @@ -0,0 +1,68 @@ +/**************************************************************************** +** +** Copyright (C) 2016 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +****************************************************************************/ + +#include "translationunitreparseerrorexception.h" + +#include + +namespace ClangBackEnd { + +TranslationUnitReparseErrorException::TranslationUnitReparseErrorException( + const Utf8String &filePath, + const Utf8String &projectPartId, + int errorCode) + : filePath_(filePath), + projectPartId_(projectPartId), + errorCode_(errorCode) +{ +} + +const Utf8String &TranslationUnitReparseErrorException::filePath() const +{ + return filePath_; +} + +const Utf8String &TranslationUnitReparseErrorException::projectPartId() const +{ + return projectPartId_; +} + +const char *TranslationUnitReparseErrorException::what() const Q_DECL_NOEXCEPT +{ + if (what_.isEmpty()) { + what_ += Utf8StringLiteral("clang_reparseTranslationUnit() failed for file ") + + filePath() + + Utf8StringLiteral(" in project ") + + projectPartId() + + Utf8StringLiteral(": ") + + Utf8String::fromString(QString::number(errorCode_)) + + Utf8StringLiteral("."); + } + + return what_.constData(); +} + +} // namespace ClangBackEnd + diff --git a/src/tools/clangbackend/ipcsource/translationunitreparseerrorexception.h b/src/tools/clangbackend/ipcsource/translationunitreparseerrorexception.h new file mode 100644 index 00000000000..126a24c7e9e --- /dev/null +++ b/src/tools/clangbackend/ipcsource/translationunitreparseerrorexception.h @@ -0,0 +1,61 @@ +/**************************************************************************** +** +** Copyright (C) 2016 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +****************************************************************************/ + +#pragma once + +#include + +#include + +#include + +namespace ClangBackEnd { + +class TranslationUnitReparseErrorException : public std::exception +{ +public: + TranslationUnitReparseErrorException(const Utf8String &filePath, + const Utf8String &projectPartId, + int errorCode); + + const Utf8String &filePath() const; + const Utf8String &projectPartId() const; + + const char *what() const Q_DECL_NOEXCEPT override; + +#if defined(__GNUC__) && !defined(__clang__) +# if !__GNUC_PREREQ(4,8) + ~TranslationUnitReparseErrorException() noexcept {} +# endif +#endif + +private: + Utf8String filePath_; + Utf8String projectPartId_; + int errorCode_; + mutable Utf8String what_; +}; + +} // namespace ClangBackEnd diff --git a/src/tools/clangbackend/ipcsource/translationunits.cpp b/src/tools/clangbackend/ipcsource/translationunits.cpp index 5b118a96529..6f9defc3564 100644 --- a/src/tools/clangbackend/ipcsource/translationunits.cpp +++ b/src/tools/clangbackend/ipcsource/translationunits.cpp @@ -199,7 +199,9 @@ namespace { bool translationUnitHasNewDocumentAnnotations(const TranslationUnit &translationUnit) { - return translationUnit.hasNewDiagnostics() || translationUnit.hasNewHighlightingInformations(); + return translationUnit.isIntact() + && (translationUnit.hasNewDiagnostics() + || translationUnit.hasNewHighlightingInformations()); } } diff --git a/tests/unit/unittest/translationunitstest.cpp b/tests/unit/unittest/translationunitstest.cpp index 25c64080cdb..9a8b44664bc 100644 --- a/tests/unit/unittest/translationunitstest.cpp +++ b/tests/unit/unittest/translationunitstest.cpp @@ -40,6 +40,8 @@ #include #include +#include + #include #include "mocksenddocumentannotationscallback.h" @@ -83,6 +85,7 @@ class TranslationUnits : public ::testing::Test { protected: void SetUp() override; + void createDirtyTranslationUnitAndDeleteFile(); void sendAllDocumentAnnotations(); void sendAllDocumentAnnotationsForCurrentEditor(); void sendAllDocumentAnnotationsForVisibleEditors(); @@ -130,6 +133,15 @@ TEST_F(TranslationUnits, DoNotThrowForAddingNonExistingFileWithUnsavedContent) ASSERT_NO_THROW(translationUnits.create({fileContainer})); } +TEST_F(TranslationUnits, DoNotSendDocumentAnnotationsForVanishedMainFile) +{ + createDirtyTranslationUnitAndDeleteFile(); + + EXPECT_CALL(mockSendDocumentAnnotationsCallback, sendDocumentAnnotations()).Times(0); + + sendAllDocumentAnnotations(); +} + TEST_F(TranslationUnits, Add) { ClangBackEnd::FileContainer fileContainer(filePath, projectPartId, Utf8StringVector(), 74u); @@ -504,6 +516,20 @@ void TranslationUnits::SetUp() translationUnits.setSendDocumentAnnotationsCallback(callback); } +void TranslationUnits::createDirtyTranslationUnitAndDeleteFile() +{ + QTemporaryFile temporaryFile(QLatin1String("XXXXXX.cpp")); + EXPECT_TRUE(temporaryFile.open()); + const QString temporaryFilePath = Utf8String::fromString(temporaryFile.fileName()); + + ClangBackEnd::FileContainer fileContainer(temporaryFilePath, + projectPartId, Utf8String(), true); + translationUnits.create({fileContainer}); + auto translationUnit = translationUnits.translationUnit(fileContainer); + translationUnit.setIsUsedByCurrentEditor(true); + translationUnit.setDirtyIfDependencyIsMet(translationUnit.filePath()); +} + void TranslationUnits::sendAllDocumentAnnotations() { auto sendState = DocumentAnnotationsSendState::MaybeThereAreDocumentAnnotations; diff --git a/tests/unit/unittest/translationunittest.cpp b/tests/unit/unittest/translationunittest.cpp index f4931235d1c..e6d14218cb2 100644 --- a/tests/unit/unittest/translationunittest.cpp +++ b/tests/unit/unittest/translationunittest.cpp @@ -72,7 +72,7 @@ class TranslationUnit : public ::testing::Test { protected: void SetUp() override; - ::TranslationUnit createTemporaryTranslationUnit(); + ::TranslationUnit createTranslationUnitAndDeleteFile(); QByteArray readContentFromTranslationUnitFile() const; protected: @@ -92,6 +92,13 @@ TEST_F(TranslationUnit, DefaultTranslationUnitIsInvalid) ASSERT_TRUE(translationUnit.isNull()); } +TEST_F(TranslationUnit, DefaultTranslationUnitIsNotIntact) +{ + ::TranslationUnit translationUnit; + + ASSERT_FALSE(translationUnit.isIntact()); +} + TEST_F(TranslationUnit, ThrowExceptionForNonExistingFilePath) { ASSERT_THROW(::TranslationUnit(Utf8StringLiteral("file.cpp"), projectPart, Utf8StringVector(), translationUnits), @@ -193,7 +200,7 @@ TEST_F(TranslationUnit, DependedFilePaths) TEST_F(TranslationUnit, DeletedFileShouldNotNeedReparsing) { - auto translationUnit = createTemporaryTranslationUnit(); + auto translationUnit = createTranslationUnitAndDeleteFile(); translationUnit.setDirtyIfDependencyIsMet(translationUnit.filePath()); @@ -244,6 +251,20 @@ TEST_F(TranslationUnit, NeedsNoReparsingAfterReparsing) ASSERT_FALSE(translationUnit.isNeedingReparse()); } +TEST_F(TranslationUnit, IsIntactAfterCreation) +{ + translationUnit.cxTranslationUnit(); + + ASSERT_TRUE(translationUnit.isIntact()); +} + +TEST_F(TranslationUnit, IsNotIntactForDeletedFile) +{ + auto translationUnit = createTranslationUnitAndDeleteFile(); + + ASSERT_FALSE(translationUnit.isIntact()); +} + TEST_F(TranslationUnit, HasNewDiagnosticsAfterCreation) { translationUnit.cxTranslationUnit(); @@ -364,7 +385,7 @@ void TranslationUnit::SetUp() translationUnit = createdTranslationUnits.front(); } -::TranslationUnit TranslationUnit::createTemporaryTranslationUnit() +::TranslationUnit TranslationUnit::createTranslationUnitAndDeleteFile() { QTemporaryFile temporaryFile; EXPECT_TRUE(temporaryFile.open());