From a7bbcb8e398aaeafca72a3646ed4f20574f856af Mon Sep 17 00:00:00 2001 From: Robert Loehning Date: Fri, 18 Jan 2019 18:30:38 +0100 Subject: [PATCH 01/19] Squish: Stabilize tst_simple_analyze The total time may be more than 1s when the CPU is busy. Ignoring this column. Change-Id: I9c80fecb618bcad1bc3e7922c5a573964cd019a4 Reviewed-by: Christian Stenger --- tests/system/suite_debugger/tst_simple_analyze/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/suite_debugger/tst_simple_analyze/test.py b/tests/system/suite_debugger/tst_simple_analyze/test.py index 202c5053b25..111336e33a9 100644 --- a/tests/system/suite_debugger/tst_simple_analyze/test.py +++ b/tests/system/suite_debugger/tst_simple_analyze/test.py @@ -103,7 +103,7 @@ def performTest(workingDir, projectName, availableConfigs): compareEventsTab(model, "events_qt%s.tsv" % qtVersion) test.compare(dumpItems(model, column=colPercent)[0], '100 %') # cannot run following test on colShortest (unstable) - for i in [colTotal, colMean, colMedian, colLongest]: + for i in [colMean, colMedian, colLongest]: for item in dumpItems(model, column=i)[2:5]: test.verify(item.endswith('ms'), "Verify that '%s' ends with 'ms'" % item) for i in [colTotal, colMean, colMedian, colLongest, colShortest]: From dd0156d1facc5422464dc1b430696a0b63db605f Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Wed, 30 Jan 2019 14:46:16 +0100 Subject: [PATCH 02/19] macOS 10.14: Allow user applications to request access to services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In macOS Mojave applications have to provide information about the reason they want to access certain services (Camera, Microphone, etc). If they do not provide this information in their Info.plist, they possibly are even killed by the system. For direct child processes, like user applications are when started from Qt Creator, this information is taken from the parent application, i.e. Qt Creator, even if the child process provides the information in it's own Info.plist. Fixes: QTCREATORBUG-21887 Change-Id: I1bc149fea928a26d59d126f6b1be71649a4369ed Reviewed-by: Morten Johan Sørvig Reviewed-by: Leena Miettinen Reviewed-by: Vikas Pachdha --- src/app/app-Info.plist | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/app/app-Info.plist b/src/app/app-Info.plist index 91104f163d5..d87130d5e49 100644 --- a/src/app/app-Info.plist +++ b/src/app/app-Info.plist @@ -254,5 +254,23 @@ @SHORT_VERSION@ LSMinimumSystemVersion @MACOSX_DEPLOYMENT_TARGET@ - - + NSAppleEventsUsageDescription + This application wants to run AppleScript. + NSBluetoothPeripheralUsageDescription + A user application wants to access bluetooth. + NSCalendarsUsageDescription + A user application wants to access calendars. + NSCameraUsageDescription + A user application wants to access the camera. + NSContactsUsageDescription + A user application wants to access contacts. + NSLocationWhenInUseUsageDescription + A user application wants to access location information. + NSMicrophoneUsageDescription + A user application wants to access the microphone. + NSPhotoLibraryAddUsageDescription + A user application wants write access to the photo library. + NSPhotoLibraryUsageDescription + A user application wants to access the photo library. + NSRemindersUsageDescription + A user application wants to access the reminders. From 68bce815ac4c53251df4c1038596ea87469562da Mon Sep 17 00:00:00 2001 From: Ivan Donchevskii Date: Thu, 31 Jan 2019 10:31:39 +0100 Subject: [PATCH 03/19] Clang: Ignore missing includes while indexing Change-Id: I0319fbbbde132320c06cb8b04f7d31aed9c0e218 Reviewed-by: Marco Bubke --- .../source/collectmacrossourcefilecallbacks.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp index d96fb05026c..e8e77f2e231 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp +++ b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp @@ -45,6 +45,7 @@ bool CollectMacrosSourceFileCallbacks::handleBeginSource(clang::CompilerInstance compilerInstance.getPreprocessorPtr(), m_sourcesManager); + compilerInstance.getPreprocessorPtr()->SetSuppressIncludeNotFoundError(true); compilerInstance.getPreprocessorPtr()->addPPCallbacks(std::move(callbacks)); return true; From 536b733f2978cf81d72cc01572bf6338248aac2d Mon Sep 17 00:00:00 2001 From: Ivan Donchevskii Date: Mon, 28 Jan 2019 08:13:33 +0100 Subject: [PATCH 04/19] ClangFormat: Format more code while typing With the extra option "Format while typing" checked try to format text before the current position without breaking the current input. To accomplish that we make proper choices which replacements to apply. The advantage of this change is to decrease the need to manually format code which is just written. Some minor bugs are fixed during the testing of this change. Change-Id: Ibed569042e6c46a881e7a83b1882cf2bb23b3626 Reviewed-by: Leena Miettinen Reviewed-by: Marco Bubke --- .../clangformat/clangformatbaseindenter.cpp | 240 +++++++++++++----- .../clangformat/clangformatbaseindenter.h | 10 +- .../clangformat/clangformatconfigwidget.cpp | 21 +- .../clangformat/clangformatconfigwidget.h | 3 + .../clangformat/clangformatconfigwidget.ui | 7 + .../clangformat/clangformatconstants.h | 1 + .../clangformat/clangformatindenter.cpp | 11 + src/plugins/clangformat/clangformatindenter.h | 2 + .../clangformat/clangformatsettings.cpp | 13 + src/plugins/clangformat/clangformatsettings.h | 4 + tests/unit/unittest/clangformat-test.cpp | 93 +++++++ 11 files changed, 340 insertions(+), 65 deletions(-) diff --git a/src/plugins/clangformat/clangformatbaseindenter.cpp b/src/plugins/clangformat/clangformatbaseindenter.cpp index d69498d0473..f5bc876020a 100644 --- a/src/plugins/clangformat/clangformatbaseindenter.cpp +++ b/src/plugins/clangformat/clangformatbaseindenter.cpp @@ -27,6 +27,7 @@ #include +#include #include #include #include @@ -57,20 +58,29 @@ static llvm::StringRef clearExtraNewline(llvm::StringRef text) static clang::tooling::Replacements filteredReplacements( const clang::tooling::Replacements &replacements, int offset, - int extraOffsetToAdd, - bool onlyIndention) + int utf8LineLengthBeforeCursor, + int extraOffsetFromStartOfFile, + int extraEmptySpaceOffset, + ReplacementsToKeep replacementsToKeep) { clang::tooling::Replacements filtered; for (const clang::tooling::Replacement &replacement : replacements) { int replacementOffset = static_cast(replacement.getOffset()); - if (onlyIndention && replacementOffset != offset - 1) + const bool replacementDoesNotMatchRestriction + = (replacementsToKeep == ReplacementsToKeep::OnlyIndent + && replacementOffset != offset - 1) + || (replacementsToKeep == ReplacementsToKeep::OnlyBeforeIndent + && replacementOffset >= offset + utf8LineLengthBeforeCursor - 1); + if (replacementDoesNotMatchRestriction) continue; - if (replacementOffset + 1 >= offset) - replacementOffset += extraOffsetToAdd; + if (replacementOffset >= offset - 1) + replacementOffset += extraEmptySpaceOffset; + replacementOffset += extraOffsetFromStartOfFile; - llvm::StringRef text = onlyIndention ? clearExtraNewline(replacement.getReplacementText()) - : replacement.getReplacementText(); + llvm::StringRef text = replacementsToKeep == ReplacementsToKeep::OnlyIndent + ? clearExtraNewline(replacement.getReplacementText()) + : replacement.getReplacementText(); llvm::Error error = filtered.add( clang::tooling::Replacement(replacement.getFilePath(), @@ -78,8 +88,14 @@ static clang::tooling::Replacements filteredReplacements( replacement.getLength(), text)); // Throws if error is not checked. - if (error) + if (error) { + error = llvm::handleErrors(std::move(error), + [](const llvm::ErrorInfoBase &) -> llvm::Error { + return llvm::Error::success(); + }); + QTC_CHECK(!error && "Error must be a \"success\" at this point"); break; + } } return filtered; } @@ -140,9 +156,16 @@ static int previousEmptyLinesLength(const QTextBlock ¤tBlock) static void modifyToIndentEmptyLines( QByteArray &buffer, int offset, int &length, const QTextBlock &block, bool secondTry) { - const QString blockText = block.text().trimmed(); - const bool closingParenBlock = blockText.startsWith(')'); - if (blockText.isEmpty() || closingParenBlock) { + const QString blockText = block.text(); + int firstNonWhitespace = Utils::indexOf(blockText, + [](const QChar &ch) { return !ch.isSpace(); }); + if (firstNonWhitespace > 0) + offset += firstNonWhitespace; + + const bool closingParenBlock = firstNonWhitespace >= 0 + && blockText.at(firstNonWhitespace) == ')'; + + if (firstNonWhitespace < 0 || closingParenBlock) { //This extra text works for the most cases. QByteArray dummyText("a;"); @@ -153,15 +176,8 @@ static void modifyToIndentEmptyLines( prevBlock = prevBlock.previous(); prevBlockIsEmpty = prevBlock.position() > 0 && prevBlock.text().trimmed().isEmpty(); } - if (prevBlock.text().endsWith(',')) - dummyText = "int a"; - - if (closingParenBlock) { - if (prevBlock.text().endsWith(',')) - dummyText = "int a"; - else - dummyText = "&& a"; - } + if (closingParenBlock || prevBlock.text().endsWith(',')) + dummyText = "&& a"; length += dummyText.length(); buffer.insert(offset, dummyText); @@ -169,6 +185,9 @@ static void modifyToIndentEmptyLines( if (secondTry) { int nextLinePos = buffer.indexOf('\n', offset); + if (nextLinePos < 0) + nextLinePos = buffer.size() - 1; + if (nextLinePos > 0) { // If first try was not successful try to put ')' in the end of the line to close possibly // unclosed parentheses. @@ -187,7 +206,9 @@ static Utils::LineColumn utf16LineColumn(const QTextBlock &block, int utf8Offset) { // If lastIndexOf('\n') returns -1 then we are fine to add 1 and get 0 offset. - const int lineStartUtf8Offset = utf8Buffer.lastIndexOf('\n', utf8Offset - 1) + 1; + const int lineStartUtf8Offset = utf8Offset == 0 + ? 0 + : utf8Buffer.lastIndexOf('\n', utf8Offset - 1) + 1; int line = block.blockNumber() + 1; // Init with the line corresponding the block. if (utf8Offset < blockOffsetUtf8) { @@ -299,8 +320,10 @@ void ClangFormatBaseIndenter::indent(const QTextCursor &cursor, if (currentBlock.isValid()) { const int blocksAmount = m_doc->blockCount(); indentBlock(currentBlock, typedChar, cursorPositionInEditor); - QTC_CHECK(blocksAmount == m_doc->blockCount() - && "ClangFormat plugin indentation changed the amount of blocks."); + + // Only blocks before current might be added/removed, so it's safe to modify the index. + if (blocksAmount != m_doc->blockCount()) + currentBlockNumber += (m_doc->blockCount() - blocksAmount); } } } else { @@ -328,24 +351,30 @@ TextEditor::Replacements ClangFormatBaseIndenter::format(const QTextCursor &curs { int utf8Offset; int utf8Length; + QTextBlock block; + const QByteArray buffer = m_doc->toPlainText().toUtf8(); - QTextBlock block = cursor.block(); if (cursor.hasSelection()) { block = m_doc->findBlock(cursor.selectionStart()); const QTextBlock end = m_doc->findBlock(cursor.selectionEnd()); utf8Offset = Utils::Text::utf8NthLineOffset(m_doc, buffer, block.blockNumber() + 1); QTC_ASSERT(utf8Offset >= 0, return TextEditor::Replacements();); utf8Length = selectedLines(m_doc, block, end).toUtf8().size(); - } else { - const QTextBlock block = cursor.block(); + block = cursor.block(); utf8Offset = Utils::Text::utf8NthLineOffset(m_doc, buffer, block.blockNumber() + 1); QTC_ASSERT(utf8Offset >= 0, return TextEditor::Replacements();); + utf8Length = block.text().toUtf8().size(); } - const TextEditor::Replacements toReplace - = replacements(buffer, utf8Offset, utf8Length, block, QChar::Null, false); + const TextEditor::Replacements toReplace = replacements(buffer, + utf8Offset, + utf8Length, + block, + cursorPositionInEditor, + ReplacementsToKeep::All, + QChar::Null); applyReplacements(block, toReplace); return toReplace; @@ -359,17 +388,62 @@ TextEditor::Replacements ClangFormatBaseIndenter::format( return format(cursor, cursorPositionInEditor); } +int ClangFormatBaseIndenter::indentBeforeCursor(const QTextBlock &block, + const QChar &typedChar, + int cursorPositionInEditor) +{ + const QByteArray buffer = m_doc->toPlainText().toUtf8(); + const int utf8Offset = Utils::Text::utf8NthLineOffset(m_doc, buffer, block.blockNumber() + 1); + QTC_ASSERT(utf8Offset >= 0, return cursorPositionInEditor;); + const TextEditor::Replacements toReplace = replacements(buffer, + utf8Offset, + 0, + block, + cursorPositionInEditor, + ReplacementsToKeep::OnlyBeforeIndent, + typedChar); + applyReplacements(block, toReplace); + for (const TextEditor::Replacement &replacement : toReplace) + cursorPositionInEditor += replacement.text.length() - replacement.length; + return cursorPositionInEditor; +} + void ClangFormatBaseIndenter::indentBlock(const QTextBlock &block, const QChar &typedChar, - int /*cursorPositionInEditor*/) + int cursorPositionInEditor) { - trimFirstNonEmptyBlock(block); - trimCurrentBlock(block); + QTextBlock currentBlock = block; + const int blockPosition = currentBlock.position(); + trimFirstNonEmptyBlock(currentBlock); + + if (formatWhileTyping() + && (cursorPositionInEditor == -1 || cursorPositionInEditor >= blockPosition)) { + // Format before current position only in case the cursor is inside the indented block. + // So if cursor position is less then the block position then the current line is before + // the indented block - don't trigger extra formatting in this case. + // cursorPositionInEditor == -1 means the consition matches automatically. + if (cursorPositionInEditor >= 0) + cursorPositionInEditor += currentBlock.position() - blockPosition; + else + cursorPositionInEditor = currentBlock.position(); + + cursorPositionInEditor = indentBeforeCursor(currentBlock, typedChar, cursorPositionInEditor); + currentBlock = m_doc->findBlock(cursorPositionInEditor); + } + + trimCurrentBlock(currentBlock); const QByteArray buffer = m_doc->toPlainText().toUtf8(); const int utf8Offset = Utils::Text::utf8NthLineOffset(m_doc, buffer, block.blockNumber() + 1); QTC_ASSERT(utf8Offset >= 0, return;); - applyReplacements(block, replacements(buffer, utf8Offset, 0, block, typedChar)); + applyReplacements(currentBlock, + replacements(buffer, + utf8Offset, + 0, + currentBlock, + cursorPositionInEditor, + ReplacementsToKeep::OnlyIndent, + typedChar)); } void ClangFormatBaseIndenter::indentBlock(const QTextBlock &block, @@ -380,7 +454,7 @@ void ClangFormatBaseIndenter::indentBlock(const QTextBlock &block, indentBlock(block, typedChar, cursorPositionInEditor); } -int ClangFormatBaseIndenter::indentFor(const QTextBlock &block, int /*cursorPositionInEditor*/) +int ClangFormatBaseIndenter::indentFor(const QTextBlock &block, int cursorPositionInEditor) { trimFirstNonEmptyBlock(block); trimCurrentBlock(block); @@ -388,7 +462,12 @@ int ClangFormatBaseIndenter::indentFor(const QTextBlock &block, int /*cursorPosi const int utf8Offset = Utils::Text::utf8NthLineOffset(m_doc, buffer, block.blockNumber() + 1); QTC_ASSERT(utf8Offset >= 0, return 0;); - const TextEditor::Replacements toReplace = replacements(buffer, utf8Offset, 0, block); + const TextEditor::Replacements toReplace = replacements(buffer, + utf8Offset, + 0, + block, + cursorPositionInEditor, + ReplacementsToKeep::OnlyIndent); if (toReplace.empty()) return -1; @@ -447,12 +526,30 @@ clang::format::FormatStyle ClangFormatBaseIndenter::styleForFile() const return clang::format::getLLVMStyle(); } +static int formattingRangeStart(const QTextBlock ¤tBlock, + const QByteArray &buffer, + int documentRevision) +{ + QTextBlock prevBlock = currentBlock.previous(); + while ((prevBlock.position() > 0 || prevBlock.length() > 0) + && prevBlock.revision() != documentRevision + && (currentBlock.blockNumber() - prevBlock.blockNumber() < kMaxLinesFromCurrentBlock)) { + // Find the first block with not matching revision. + prevBlock = prevBlock.previous(); + } + if (prevBlock.revision() == documentRevision) + prevBlock = prevBlock.next(); + + return Utils::Text::utf8NthLineOffset(prevBlock.document(), buffer, prevBlock.blockNumber() + 1); +} + TextEditor::Replacements ClangFormatBaseIndenter::replacements(QByteArray buffer, int utf8Offset, int utf8Length, const QTextBlock &block, + int cursorPositionInEditor, + ReplacementsToKeep replacementsToKeep, const QChar &typedChar, - bool onlyIndention, bool secondTry) const { clang::format::FormatStyle style = styleForFile(); @@ -461,13 +558,22 @@ TextEditor::Replacements ClangFormatBaseIndenter::replacements(QByteArray buffer int originalLengthUtf8 = utf8Length; QByteArray originalBuffer = buffer; - int extraOffset = 0; - if (onlyIndention) { + int utf8LineLengthBeforeCursor = 0; + if (cursorPositionInEditor > 0 && typedChar != QChar::Null) { + // Format starting with the electric character if it's present. + utf8LineLengthBeforeCursor + = block.text().left(cursorPositionInEditor - block.position()).toUtf8().size(); + } + + int extraOffsetFromStartOfFile = 0; + int extraEmptySpaceOffset = 0; + int rangeStart = 0; + if (replacementsToKeep != ReplacementsToKeep::All) { if (block.blockNumber() > kMaxLinesFromCurrentBlock) { - extraOffset = Utils::Text::utf8NthLineOffset(block.document(), - buffer, - block.blockNumber() - - kMaxLinesFromCurrentBlock); + extraOffsetFromStartOfFile + = Utils::Text::utf8NthLineOffset(block.document(), + buffer, + block.blockNumber() - kMaxLinesFromCurrentBlock); } int endOffset = Utils::Text::utf8NthLineOffset(block.document(), buffer, @@ -476,22 +582,30 @@ TextEditor::Replacements ClangFormatBaseIndenter::replacements(QByteArray buffer if (endOffset == -1) endOffset = buffer.size(); - buffer = buffer.mid(extraOffset, endOffset - extraOffset); - utf8Offset -= extraOffset; - const int emptySpaceLength = previousEmptyLinesLength(block); - utf8Offset -= emptySpaceLength; - buffer.remove(utf8Offset, emptySpaceLength); + if (replacementsToKeep == ReplacementsToKeep::OnlyBeforeIndent) + rangeStart = formattingRangeStart(block, buffer, lastSaveRevision()); - extraOffset += emptySpaceLength; + buffer = buffer.mid(extraOffsetFromStartOfFile, endOffset - extraOffsetFromStartOfFile); + utf8Offset -= extraOffsetFromStartOfFile; + rangeStart -= extraOffsetFromStartOfFile; - adjustFormatStyleForLineBreak(style); - if (typedChar == QChar::Null) - modifyToIndentEmptyLines(buffer, utf8Offset, utf8Length, block, secondTry); + extraEmptySpaceOffset = previousEmptyLinesLength(block); + utf8Offset -= extraEmptySpaceOffset; + buffer.remove(utf8Offset, extraEmptySpaceOffset); + + if (replacementsToKeep == ReplacementsToKeep::OnlyIndent) + adjustFormatStyleForLineBreak(style); + modifyToIndentEmptyLines(buffer, utf8Offset, utf8Length, block, secondTry); } - std::vector ranges{ - {static_cast(utf8Offset), static_cast(utf8Length)}}; + if (replacementsToKeep != ReplacementsToKeep::OnlyBeforeIndent || utf8Offset < rangeStart) + rangeStart = utf8Offset; + + unsigned int rangeLength = static_cast(utf8Offset + utf8Length - rangeStart); + + std::vector ranges{{static_cast(rangeStart), rangeLength}}; + clang::format::FormattingAttemptStatus status; clang::tooling::Replacements clangReplacements = reformat(style, @@ -500,21 +614,25 @@ TextEditor::Replacements ClangFormatBaseIndenter::replacements(QByteArray buffer m_fileName.toString().toStdString(), &status); - if (!status.FormatComplete) - return TextEditor::Replacements(); - - const clang::tooling::Replacements filtered = filteredReplacements(clangReplacements, - utf8Offset, - extraOffset, - onlyIndention); - const bool canTryAgain = onlyIndention && typedChar == QChar::Null && !secondTry; + clang::tooling::Replacements filtered; + if (status.FormatComplete) { + filtered = filteredReplacements(clangReplacements, + utf8Offset, + utf8LineLengthBeforeCursor, + extraOffsetFromStartOfFile, + extraEmptySpaceOffset, + replacementsToKeep); + } + const bool canTryAgain = replacementsToKeep == ReplacementsToKeep::OnlyIndent + && typedChar == QChar::Null && !secondTry; if (canTryAgain && filtered.empty()) { return replacements(originalBuffer, originalOffsetUtf8, originalLengthUtf8, block, + cursorPositionInEditor, + replacementsToKeep, typedChar, - onlyIndention, true); } diff --git a/src/plugins/clangformat/clangformatbaseindenter.h b/src/plugins/clangformat/clangformatbaseindenter.h index 4071422cfde..1a73d9bfbcf 100644 --- a/src/plugins/clangformat/clangformatbaseindenter.h +++ b/src/plugins/clangformat/clangformatbaseindenter.h @@ -31,6 +31,8 @@ namespace ClangFormat { +enum class ReplacementsToKeep { OnlyIndent, OnlyBeforeIndent, All }; + class ClangFormatBaseIndenter : public TextEditor::Indenter { public: @@ -69,18 +71,24 @@ public: protected: virtual clang::format::FormatStyle styleForFile() const; virtual bool formatCodeInsteadOfIndent() const { return false; } + virtual bool formatWhileTyping() const { return false; } + virtual int lastSaveRevision() const { return 0; } private: TextEditor::Replacements format(const QTextCursor &cursor, int cursorPositionInEditor); void indent(const QTextCursor &cursor, const QChar &typedChar, int cursorPositionInEditor); void indentBlock(const QTextBlock &block, const QChar &typedChar, int cursorPositionInEditor); int indentFor(const QTextBlock &block, int cursorPositionInEditor); + int indentBeforeCursor(const QTextBlock &block, + const QChar &typedChar, + int cursorPositionInEditor); TextEditor::Replacements replacements(QByteArray buffer, int utf8Offset, int utf8Length, const QTextBlock &block, + int cursorPositionInEditor, + ReplacementsToKeep replacementsToKeep, const QChar &typedChar = QChar::Null, - bool onlyIndention = true, bool secondTry = false) const; }; diff --git a/src/plugins/clangformat/clangformatconfigwidget.cpp b/src/plugins/clangformat/clangformatconfigwidget.cpp index 3d105f55801..bcede8c0101 100644 --- a/src/plugins/clangformat/clangformatconfigwidget.cpp +++ b/src/plugins/clangformat/clangformatconfigwidget.cpp @@ -127,12 +127,27 @@ ClangFormatConfigWidget::ClangFormatConfigWidget(ProjectExplorer::Project *proje initialize(); } +void ClangFormatConfigWidget::hideGlobalCheckboxes() +{ + m_ui->formatAlways->hide(); + m_ui->formatWhileTyping->hide(); +} + +void ClangFormatConfigWidget::showGlobalCheckboxes() +{ + m_ui->formatAlways->setChecked(ClangFormatSettings::instance().formatCodeInsteadOfIndent()); + m_ui->formatAlways->show(); + + m_ui->formatWhileTyping->setChecked(ClangFormatSettings::instance().formatWhileTyping()); + m_ui->formatWhileTyping->show(); +} + void ClangFormatConfigWidget::initialize() { m_ui->projectHasClangFormat->show(); m_ui->clangFormatOptionsTable->show(); m_ui->applyButton->show(); - m_ui->formatAlways->hide(); + hideGlobalCheckboxes(); QLayoutItem *lastItem = m_ui->verticalLayout->itemAt(m_ui->verticalLayout->count() - 1); if (lastItem->spacerItem()) @@ -171,8 +186,7 @@ void ClangFormatConfigWidget::initialize() "and can be configured in Projects > Code Style > C++.")); } createStyleFileIfNeeded(true); - m_ui->formatAlways->setChecked(ClangFormatSettings::instance().formatCodeInsteadOfIndent()); - m_ui->formatAlways->show(); + showGlobalCheckboxes(); m_ui->applyButton->hide(); } @@ -196,6 +210,7 @@ void ClangFormatConfigWidget::apply() if (!m_project) { ClangFormatSettings &settings = ClangFormatSettings::instance(); settings.setFormatCodeInsteadOfIndent(m_ui->formatAlways->isChecked()); + settings.setFormatWhileTyping(m_ui->formatWhileTyping->isChecked()); settings.write(); } diff --git a/src/plugins/clangformat/clangformatconfigwidget.h b/src/plugins/clangformat/clangformatconfigwidget.h index 5db16e3c967..7d3818b7a52 100644 --- a/src/plugins/clangformat/clangformatconfigwidget.h +++ b/src/plugins/clangformat/clangformatconfigwidget.h @@ -51,6 +51,9 @@ private: void initialize(); void fillTable(); + void hideGlobalCheckboxes(); + void showGlobalCheckboxes(); + ProjectExplorer::Project *m_project; std::unique_ptr m_ui; }; diff --git a/src/plugins/clangformat/clangformatconfigwidget.ui b/src/plugins/clangformat/clangformatconfigwidget.ui index 8fcc87235d8..37f228e97cc 100644 --- a/src/plugins/clangformat/clangformatconfigwidget.ui +++ b/src/plugins/clangformat/clangformatconfigwidget.ui @@ -33,6 +33,13 @@ + + + + Format while typing + + + diff --git a/src/plugins/clangformat/clangformatconstants.h b/src/plugins/clangformat/clangformatconstants.h index aa2389bbfeb..40b43e56407 100644 --- a/src/plugins/clangformat/clangformatconstants.h +++ b/src/plugins/clangformat/clangformatconstants.h @@ -32,5 +32,6 @@ static const char SETTINGS_FILE_ALT_NAME[] = "_clang-format"; static const char SAMPLE_FILE_NAME[] = "test.cpp"; static const char SETTINGS_ID[] = "ClangFormat"; static const char FORMAT_CODE_INSTEAD_OF_INDENT_ID[] = "ClangFormat.FormatCodeInsteadOfIndent"; +static const char FORMAT_WHILE_TYPING_ID[] = "ClangFormat.FormatWhileTyping"; } // namespace Constants } // namespace ClangFormat diff --git a/src/plugins/clangformat/clangformatindenter.cpp b/src/plugins/clangformat/clangformatindenter.cpp index 1d0400d323f..f1ece90164a 100644 --- a/src/plugins/clangformat/clangformatindenter.cpp +++ b/src/plugins/clangformat/clangformatindenter.cpp @@ -28,6 +28,7 @@ #include "clangformatutils.h" #include +#include using namespace clang; using namespace format; @@ -49,6 +50,11 @@ bool ClangFormatIndenter::formatCodeInsteadOfIndent() const return ClangFormatSettings::instance().formatCodeInsteadOfIndent(); } +bool ClangFormatIndenter::formatWhileTyping() const +{ + return ClangFormatSettings::instance().formatWhileTyping(); +} + Utils::optional ClangFormatIndenter::tabSettings() const { FormatStyle style = currentProjectStyle(); @@ -76,4 +82,9 @@ Utils::optional ClangFormatIndenter::tabSettings() const return tabSettings; } +int ClangFormatIndenter::lastSaveRevision() const +{ + return qobject_cast(m_doc->documentLayout())->lastSaveRevision; +} + } // namespace ClangFormat diff --git a/src/plugins/clangformat/clangformatindenter.h b/src/plugins/clangformat/clangformatindenter.h index 507ea68f415..dca5f583a64 100644 --- a/src/plugins/clangformat/clangformatindenter.h +++ b/src/plugins/clangformat/clangformatindenter.h @@ -39,7 +39,9 @@ public: private: bool formatCodeInsteadOfIndent() const override; + bool formatWhileTyping() const override; clang::format::FormatStyle styleForFile() const override; + int lastSaveRevision() const override; }; } // namespace ClangFormat diff --git a/src/plugins/clangformat/clangformatsettings.cpp b/src/plugins/clangformat/clangformatsettings.cpp index 0547bde9604..d8369bf5efd 100644 --- a/src/plugins/clangformat/clangformatsettings.cpp +++ b/src/plugins/clangformat/clangformatsettings.cpp @@ -42,6 +42,8 @@ ClangFormatSettings::ClangFormatSettings() settings->beginGroup(QLatin1String(Constants::SETTINGS_ID)); m_formatCodeInsteadOfIndent = settings->value(QLatin1String(Constants::FORMAT_CODE_INSTEAD_OF_INDENT_ID), false).toBool(); + m_formatWhileTyping = settings->value(QLatin1String(Constants::FORMAT_WHILE_TYPING_ID), false) + .toBool(); settings->endGroup(); } @@ -51,6 +53,7 @@ void ClangFormatSettings::write() const settings->beginGroup(QLatin1String(Constants::SETTINGS_ID)); settings->setValue(QLatin1String(Constants::FORMAT_CODE_INSTEAD_OF_INDENT_ID), m_formatCodeInsteadOfIndent); + settings->setValue(QLatin1String(Constants::FORMAT_WHILE_TYPING_ID), m_formatWhileTyping); settings->endGroup(); } @@ -64,4 +67,14 @@ bool ClangFormatSettings::formatCodeInsteadOfIndent() const return m_formatCodeInsteadOfIndent; } +void ClangFormatSettings::setFormatWhileTyping(bool enable) +{ + m_formatWhileTyping = enable; +} + +bool ClangFormatSettings::formatWhileTyping() const +{ + return m_formatWhileTyping; +} + } // namespace ClangFormat diff --git a/src/plugins/clangformat/clangformatsettings.h b/src/plugins/clangformat/clangformatsettings.h index 06205932286..4a62eb83035 100644 --- a/src/plugins/clangformat/clangformatsettings.h +++ b/src/plugins/clangformat/clangformatsettings.h @@ -37,8 +37,12 @@ public: void setFormatCodeInsteadOfIndent(bool enable); bool formatCodeInsteadOfIndent() const; + + void setFormatWhileTyping(bool enable); + bool formatWhileTyping() const; private: bool m_formatCodeInsteadOfIndent = false; + bool m_formatWhileTyping = false; }; } // namespace ClangFormat diff --git a/tests/unit/unittest/clangformat-test.cpp b/tests/unit/unittest/clangformat-test.cpp index 771e3159fe5..8b64fb19149 100644 --- a/tests/unit/unittest/clangformat-test.cpp +++ b/tests/unit/unittest/clangformat-test.cpp @@ -51,12 +51,27 @@ public: } }; +class ClangFormatExtendedIndenter : public ClangFormatIndenter +{ +public: + ClangFormatExtendedIndenter(QTextDocument *doc) + : ClangFormatIndenter(doc) + {} + + bool formatWhileTyping() const override + { + return true; + } +}; + class ClangFormat : public ::testing::Test { protected: void SetUp() final { indenter.setFileName(Utils::FileName::fromString(TESTDATA_DIR "/clangformat/test.cpp")); + extendedIndenter.setFileName( + Utils::FileName::fromString(TESTDATA_DIR "/clangformat/test.cpp")); } void insertLines(const std::vector &lines) @@ -85,6 +100,7 @@ protected: QTextDocument doc; ClangFormatIndenter indenter{&doc}; + ClangFormatExtendedIndenter extendedIndenter{&doc}; QTextCursor cursor{&doc}; }; @@ -315,6 +331,83 @@ TEST_F(ClangFormat, NoExtraIndentAfterBraceInitialization) "return 0;")); } +TEST_F(ClangFormat, IndentFunctionBodyAndFormatBeforeIt) +{ + insertLines({"int foo(int a, int b,", + " int c, int d", + " ) {", + "", + "}"}); + + extendedIndenter.indentBlock(doc.findBlockByNumber(3), QChar::Null, TextEditor::TabSettings()); + + ASSERT_THAT(documentLines(), ElementsAre("int foo(int a, int b, int c, int d)", + "{", + " ", + "}")); +} + +TEST_F(ClangFormat, IndentAfterFunctionBodyAndNotFormatBefore) +{ + insertLines({"int foo(int a, int b, int c, int d)", + "{", + " ", + "}"}); + + extendedIndenter.indentBlock(doc.findBlockByNumber(3), + QChar::Null, + TextEditor::TabSettings(), + doc.characterCount() - 3); + + ASSERT_THAT(documentLines(), ElementsAre("int foo(int a, int b, int c, int d)", + "{", + " ", + "}")); +} + +TEST_F(ClangFormat, ReformatToEmptyFunction) +{ + insertLines({"int foo(int a, int b, int c, int d)", + "{", + " ", + "}", + ""}); + + extendedIndenter.indentBlock(doc.findBlockByNumber(4), QChar::Null, TextEditor::TabSettings()); + + ASSERT_THAT(documentLines(), ElementsAre("int foo(int a, int b, int c, int d) {}", + "")); +} + +TEST_F(ClangFormat, ReformatToNonEmptyFunction) +{ + insertLines({"int foo(int a, int b) {", + "", + "}"}); + + extendedIndenter.indentBlock(doc.findBlockByNumber(1), QChar::Null, TextEditor::TabSettings()); + + ASSERT_THAT(documentLines(), ElementsAre("int foo(int a, int b)", + "{", + " ", + "}")); +} + +TEST_F(ClangFormat, IndentIfBodyAndFormatBeforeIt) +{ + insertLines({"if(a && b", + " &&c && d", + " ) {", + "", + "}"}); + + extendedIndenter.indentBlock(doc.findBlockByNumber(3), QChar::Null, TextEditor::TabSettings()); + + ASSERT_THAT(documentLines(), ElementsAre("if (a && b && c && d) {", + " ", + "}")); +} + TEST_F(ClangFormat, FormatBasicFile) { insertLines({"int main()", From 5792291520fb128764264140497233b5545c375d Mon Sep 17 00:00:00 2001 From: Ivan Donchevskii Date: Mon, 28 Jan 2019 12:25:36 +0100 Subject: [PATCH 05/19] ClangFormat: Format edited chunks of file on save The similar implementation to the one in Beautifier plugin with the difference that the clangformat indenter logic is used and only modified chunks are formatted. That means that all code which was not touched will stay in the initial condition. Change-Id: I47b11eb99852454ed0031ef6cfc6dbed1ecd390d Reviewed-by: Leena Miettinen Reviewed-by: Marco Bubke --- .../clangformat/clangformatconfigwidget.cpp | 6 ++- .../clangformat/clangformatconfigwidget.ui | 7 +++ .../clangformat/clangformatconstants.h | 1 + .../clangformat/clangformatindenter.cpp | 5 +++ src/plugins/clangformat/clangformatindenter.h | 1 + .../clangformat/clangformatsettings.cpp | 13 ++++++ src/plugins/clangformat/clangformatsettings.h | 4 ++ src/plugins/cppeditor/cppeditordocument.cpp | 43 +++++++++++++++++++ src/plugins/cppeditor/cppeditordocument.h | 4 ++ src/plugins/texteditor/indenter.h | 2 + 10 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/plugins/clangformat/clangformatconfigwidget.cpp b/src/plugins/clangformat/clangformatconfigwidget.cpp index bcede8c0101..f0d4808cf5c 100644 --- a/src/plugins/clangformat/clangformatconfigwidget.cpp +++ b/src/plugins/clangformat/clangformatconfigwidget.cpp @@ -131,6 +131,7 @@ void ClangFormatConfigWidget::hideGlobalCheckboxes() { m_ui->formatAlways->hide(); m_ui->formatWhileTyping->hide(); + m_ui->formatOnSave->hide(); } void ClangFormatConfigWidget::showGlobalCheckboxes() @@ -140,6 +141,9 @@ void ClangFormatConfigWidget::showGlobalCheckboxes() m_ui->formatWhileTyping->setChecked(ClangFormatSettings::instance().formatWhileTyping()); m_ui->formatWhileTyping->show(); + + m_ui->formatOnSave->setChecked(ClangFormatSettings::instance().formatOnSave()); + m_ui->formatOnSave->show(); } void ClangFormatConfigWidget::initialize() @@ -200,7 +204,6 @@ void ClangFormatConfigWidget::fillTable() std::string configText = clang::format::configurationAsText(style); std::istringstream stream(configText); readTable(m_ui->clangFormatOptionsTable, stream); - } ClangFormatConfigWidget::~ClangFormatConfigWidget() = default; @@ -211,6 +214,7 @@ void ClangFormatConfigWidget::apply() ClangFormatSettings &settings = ClangFormatSettings::instance(); settings.setFormatCodeInsteadOfIndent(m_ui->formatAlways->isChecked()); settings.setFormatWhileTyping(m_ui->formatWhileTyping->isChecked()); + settings.setFormatOnSave(m_ui->formatOnSave->isChecked()); settings.write(); } diff --git a/src/plugins/clangformat/clangformatconfigwidget.ui b/src/plugins/clangformat/clangformatconfigwidget.ui index 37f228e97cc..2094bca1090 100644 --- a/src/plugins/clangformat/clangformatconfigwidget.ui +++ b/src/plugins/clangformat/clangformatconfigwidget.ui @@ -40,6 +40,13 @@ + + + + Format edited code on file save + + + diff --git a/src/plugins/clangformat/clangformatconstants.h b/src/plugins/clangformat/clangformatconstants.h index 40b43e56407..8b7cbcf9bd2 100644 --- a/src/plugins/clangformat/clangformatconstants.h +++ b/src/plugins/clangformat/clangformatconstants.h @@ -33,5 +33,6 @@ static const char SAMPLE_FILE_NAME[] = "test.cpp"; static const char SETTINGS_ID[] = "ClangFormat"; static const char FORMAT_CODE_INSTEAD_OF_INDENT_ID[] = "ClangFormat.FormatCodeInsteadOfIndent"; static const char FORMAT_WHILE_TYPING_ID[] = "ClangFormat.FormatWhileTyping"; +static const char FORMAT_CODE_ON_SAVE_ID[] = "ClangFormat.FormatCodeOnSave"; } // namespace Constants } // namespace ClangFormat diff --git a/src/plugins/clangformat/clangformatindenter.cpp b/src/plugins/clangformat/clangformatindenter.cpp index f1ece90164a..ef9dbf5d1ca 100644 --- a/src/plugins/clangformat/clangformatindenter.cpp +++ b/src/plugins/clangformat/clangformatindenter.cpp @@ -87,4 +87,9 @@ int ClangFormatIndenter::lastSaveRevision() const return qobject_cast(m_doc->documentLayout())->lastSaveRevision; } +bool ClangFormatIndenter::formatOnSave() const +{ + return ClangFormatSettings::instance().formatOnSave(); +} + } // namespace ClangFormat diff --git a/src/plugins/clangformat/clangformatindenter.h b/src/plugins/clangformat/clangformatindenter.h index dca5f583a64..6d605eb5c91 100644 --- a/src/plugins/clangformat/clangformatindenter.h +++ b/src/plugins/clangformat/clangformatindenter.h @@ -36,6 +36,7 @@ class ClangFormatIndenter final : public ClangFormatBaseIndenter public: ClangFormatIndenter(QTextDocument *doc); Utils::optional tabSettings() const override; + bool formatOnSave() const override; private: bool formatCodeInsteadOfIndent() const override; diff --git a/src/plugins/clangformat/clangformatsettings.cpp b/src/plugins/clangformat/clangformatsettings.cpp index d8369bf5efd..8be2b3cefee 100644 --- a/src/plugins/clangformat/clangformatsettings.cpp +++ b/src/plugins/clangformat/clangformatsettings.cpp @@ -44,6 +44,8 @@ ClangFormatSettings::ClangFormatSettings() = settings->value(QLatin1String(Constants::FORMAT_CODE_INSTEAD_OF_INDENT_ID), false).toBool(); m_formatWhileTyping = settings->value(QLatin1String(Constants::FORMAT_WHILE_TYPING_ID), false) .toBool(); + m_formatOnSave = settings->value(QLatin1String(Constants::FORMAT_CODE_ON_SAVE_ID), false) + .toBool(); settings->endGroup(); } @@ -54,6 +56,7 @@ void ClangFormatSettings::write() const settings->setValue(QLatin1String(Constants::FORMAT_CODE_INSTEAD_OF_INDENT_ID), m_formatCodeInsteadOfIndent); settings->setValue(QLatin1String(Constants::FORMAT_WHILE_TYPING_ID), m_formatWhileTyping); + settings->setValue(QLatin1String(Constants::FORMAT_CODE_ON_SAVE_ID), m_formatOnSave); settings->endGroup(); } @@ -77,4 +80,14 @@ bool ClangFormatSettings::formatWhileTyping() const return m_formatWhileTyping; } +void ClangFormatSettings::setFormatOnSave(bool enable) +{ + m_formatOnSave = enable; +} + +bool ClangFormatSettings::formatOnSave() const +{ + return m_formatOnSave; +} + } // namespace ClangFormat diff --git a/src/plugins/clangformat/clangformatsettings.h b/src/plugins/clangformat/clangformatsettings.h index 4a62eb83035..0ea9ed9747e 100644 --- a/src/plugins/clangformat/clangformatsettings.h +++ b/src/plugins/clangformat/clangformatsettings.h @@ -40,9 +40,13 @@ public: void setFormatWhileTyping(bool enable); bool formatWhileTyping() const; + + void setFormatOnSave(bool enable); + bool formatOnSave() const; private: bool m_formatCodeInsteadOfIndent = false; bool m_formatWhileTyping = false; + bool m_formatOnSave = false; }; } // namespace ClangFormat diff --git a/src/plugins/cppeditor/cppeditordocument.cpp b/src/plugins/cppeditor/cppeditordocument.cpp index c8951c12ebc..d18735c7501 100644 --- a/src/plugins/cppeditor/cppeditordocument.cpp +++ b/src/plugins/cppeditor/cppeditordocument.cpp @@ -44,6 +44,7 @@ #include #include +#include #include #include @@ -447,5 +448,47 @@ TextEditor::TabSettings CppEditorDocument::tabSettings() const return indenter()->tabSettings().value_or(TextEditor::TextDocument::tabSettings()); } +static int formatRange(QTextDocument *doc, + TextEditor::Indenter *indenter, + std::pair editedRange, + const TextEditor::TabSettings &tabSettings) +{ + QTextCursor cursor(doc); + cursor.setPosition(editedRange.first); + cursor.setPosition(editedRange.second, QTextCursor::KeepAnchor); + const int oldBlockCount = doc->blockCount(); + indenter->format(cursor, tabSettings); + return doc->blockCount() - oldBlockCount; +} + +bool CppEditorDocument::save(QString *errorString, const QString &fileName, bool autoSave) +{ + if (indenter()->formatOnSave()) { + auto *layout = qobject_cast(document()->documentLayout()); + const int documentRevision = layout->lastSaveRevision; + + std::pair editedRange; + for (int i = 0; i < document()->blockCount(); ++i) { + const QTextBlock block = document()->findBlockByNumber(i); + if (block.revision() == documentRevision) { + if (editedRange.first != -1) + i += formatRange(document(), indenter(), editedRange, tabSettings()); + + editedRange = std::make_pair(-1, -1); + continue; + } + + // block.revision() != documentRevision + if (editedRange.first == -1) + editedRange.first = block.position(); + editedRange.second = block.position() + block.length(); + } + if (editedRange.first != -1) + formatRange(document(), indenter(), editedRange, tabSettings()); + } + + return TextEditor::TextDocument::save(errorString, fileName, autoSave); +} + } // namespace Internal } // namespace CppEditor diff --git a/src/plugins/cppeditor/cppeditordocument.h b/src/plugins/cppeditor/cppeditordocument.h index 44fe04becb7..26cff2043aa 100644 --- a/src/plugins/cppeditor/cppeditordocument.h +++ b/src/plugins/cppeditor/cppeditordocument.h @@ -69,6 +69,10 @@ public: QFuture cursorInfo(const CppTools::CursorInfoParams ¶ms); TextEditor::TabSettings tabSettings() const override; + bool save(QString *errorString, + const QString &fileName = QString(), + bool autoSave = false) override; + signals: void codeWarningsUpdated(unsigned contentsRevision, const QList selections, diff --git a/src/plugins/texteditor/indenter.h b/src/plugins/texteditor/indenter.h index bb23c9d7ac2..2907ea783ce 100644 --- a/src/plugins/texteditor/indenter.h +++ b/src/plugins/texteditor/indenter.h @@ -99,6 +99,8 @@ public: return Replacements(); } + virtual bool formatOnSave() const { return false; } + // Expects a list of blocks in order of occurrence in the document. virtual IndentationForBlock indentationForBlocks(const QVector &blocks, const TabSettings &tabSettings, From cd868e8a7bd098dcaee0262308079036312fe53c Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Thu, 24 Jan 2019 15:01:30 +0100 Subject: [PATCH 06/19] Clang: Fix crash on exit There were threads running but the instances they called were already deleted. Now we delete the scheduler first which is the thread holder and which is waiting that all threads are finished. Task-number: QTCREATORBUG-21882 Change-Id: I2e9f4d8381d79fab9a93346cef6598ab8e8f7850 Reviewed-by: Ivan Donchevskii --- .../clangpchmanagerbackendmain.cpp | 20 +++++++++---------- .../source/taskscheduler.h | 5 +++++ .../clangrefactoringbackendmain.cpp | 2 +- .../source/symbolindexing.h | 4 ++-- tests/unit/unittest/symbolindexer-test.cpp | 12 +++++------ 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp index 3656ebd9dc5..5c38a3d8413 100644 --- a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp +++ b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp @@ -183,16 +183,6 @@ struct Data // because we have a cycle dependency PrecompiledHeaderStorage<> preCompiledHeaderStorage{database}; ClangBackEnd::ProgressCounter progressCounter{ [&](int progress, int total) { clangPchManagerServer.setProgress(progress, total); }}; - TaskScheduler systemTaskScheduler{pchCreatorManager, - pchTaskQueue, - progressCounter, - std::thread::hardware_concurrency(), - ClangBackEnd::CallDoInMainThreadAfterFinished::No}; - TaskScheduler projectTaskScheduler{pchCreatorManager, - pchTaskQueue, - progressCounter, - std::thread::hardware_concurrency(), - ClangBackEnd::CallDoInMainThreadAfterFinished::Yes}; ClangBackEnd::PchTaskQueue pchTaskQueue{systemTaskScheduler, projectTaskScheduler, progressCounter, @@ -212,6 +202,16 @@ struct Data // because we have a cycle dependency database}; ClangBackEnd::PchTaskGenerator pchTaskGenerator{buildDependencyProvider, pchTaskMerger}; PchManagerServer clangPchManagerServer{includeWatcher, pchTaskGenerator, projectParts, generatedFiles}; + TaskScheduler systemTaskScheduler{pchCreatorManager, + pchTaskQueue, + progressCounter, + std::thread::hardware_concurrency(), + ClangBackEnd::CallDoInMainThreadAfterFinished::No}; + TaskScheduler projectTaskScheduler{pchCreatorManager, + pchTaskQueue, + progressCounter, + std::thread::hardware_concurrency(), + ClangBackEnd::CallDoInMainThreadAfterFinished::Yes}; }; int main(int argc, char *argv[]) diff --git a/src/tools/clangpchmanagerbackend/source/taskscheduler.h b/src/tools/clangpchmanagerbackend/source/taskscheduler.h index 621baef0909..37fdc744a84 100644 --- a/src/tools/clangpchmanagerbackend/source/taskscheduler.h +++ b/src/tools/clangpchmanagerbackend/source/taskscheduler.h @@ -80,6 +80,11 @@ public: , m_callDoInMainThreadAfterFinished(callDoInMainThreadAfterFinished) {} + ~TaskScheduler() + { + syncTasks(); + } + void addTasks(std::vector &&tasks) { for (auto &task : tasks) { diff --git a/src/tools/clangrefactoringbackend/clangrefactoringbackendmain.cpp b/src/tools/clangrefactoringbackend/clangrefactoringbackendmain.cpp index 07e594b7217..0d483bbeaf1 100644 --- a/src/tools/clangrefactoringbackend/clangrefactoringbackendmain.cpp +++ b/src/tools/clangrefactoringbackend/clangrefactoringbackendmain.cpp @@ -93,8 +93,8 @@ struct Data // because we have a cycle dependency RefactoringDatabaseInitializer databaseInitializer{database}; FilePathCaching filePathCache{database}; GeneratedFiles generatedFiles; - SymbolIndexing symbolIndexing{database, filePathCache, generatedFiles, [&] (int progress, int total) { clangCodeModelServer.setProgress(progress, total); }}; RefactoringServer clangCodeModelServer{symbolIndexing, filePathCache, generatedFiles}; + SymbolIndexing symbolIndexing{database, filePathCache, generatedFiles, [&] (int progress, int total) { clangCodeModelServer.setProgress(progress, total); }}; }; int main(int argc, char *argv[]) diff --git a/src/tools/clangrefactoringbackend/source/symbolindexing.h b/src/tools/clangrefactoringbackend/source/symbolindexing.h index 6f52f863d85..62be193682c 100644 --- a/src/tools/clangrefactoringbackend/source/symbolindexing.h +++ b/src/tools/clangrefactoringbackend/source/symbolindexing.h @@ -124,8 +124,6 @@ private: FileStatusCache m_fileStatusCache{m_filePathCache}; SymbolsCollectorManager m_collectorManger; ProgressCounter m_progressCounter; - SymbolIndexerTaskScheduler m_indexerScheduler; - SymbolIndexerTaskQueue m_indexerQueue{m_indexerScheduler, m_progressCounter}; SymbolIndexer m_indexer{m_indexerQueue, m_symbolStorage, m_buildDependencyStorage, @@ -133,6 +131,8 @@ private: m_filePathCache, m_fileStatusCache, m_symbolStorage.m_database}; + SymbolIndexerTaskQueue m_indexerQueue{m_indexerScheduler, m_progressCounter}; + SymbolIndexerTaskScheduler m_indexerScheduler; }; } // namespace ClangBackEnd diff --git a/tests/unit/unittest/symbolindexer-test.cpp b/tests/unit/unittest/symbolindexer-test.cpp index 23f469cd340..4f112d45298 100644 --- a/tests/unit/unittest/symbolindexer-test.cpp +++ b/tests/unit/unittest/symbolindexer-test.cpp @@ -241,12 +241,6 @@ protected: Manager collectorManger{generatedFiles}; NiceMock> mockSetProgressCallback; ClangBackEnd::ProgressCounter progressCounter{mockSetProgressCallback.AsStdFunction()}; - Scheduler indexerScheduler{collectorManger, - indexerQueue, - progressCounter, - 1, - ClangBackEnd::CallDoInMainThreadAfterFinished::Yes}; - SymbolIndexerTaskQueue indexerQueue{indexerScheduler, progressCounter}; ClangBackEnd::SymbolIndexer indexer{indexerQueue, mockSymbolStorage, mockBuildDependenciesStorage, @@ -254,6 +248,12 @@ protected: filePathCache, fileStatusCache, mockSqliteTransactionBackend}; + SymbolIndexerTaskQueue indexerQueue{indexerScheduler, progressCounter}; + Scheduler indexerScheduler{collectorManger, + indexerQueue, + progressCounter, + 1, + ClangBackEnd::CallDoInMainThreadAfterFinished::Yes}; MockSymbolsCollector &mockCollector{static_cast(collectorManger.unusedProcessor())}; }; From 14a44948d87f9ff0de1e6963fb6ed10fd994f101 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Mon, 4 Feb 2019 15:49:26 +0100 Subject: [PATCH 07/19] ClangPchManager: Fix deferred project parts Change-Id: Ie760c0dd269c643a147d7edf3f1b812cd27fe4c4 Reviewed-by: Ivan Donchevskii --- .../clangpchmanagerbackend/source/pchmanagerserver.cpp | 2 ++ tests/unit/unittest/pchmanagerserver-test.cpp | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp index 30b6e566fa5..06adccff5b4 100644 --- a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp +++ b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp @@ -66,6 +66,8 @@ void PchManagerServer::updateProjectParts(UpdateProjectPartsMessage &&message) if (m_generatedFiles.isValid()) { m_pchTaskGenerator.addProjectParts(std::move(newProjectParts), std::move(message.toolChainArguments)); + } else { + m_projectParts.updateDeferred(newProjectParts); } } diff --git a/tests/unit/unittest/pchmanagerserver-test.cpp b/tests/unit/unittest/pchmanagerserver-test.cpp index 9100b51dc30..0cb6e3d1baf 100644 --- a/tests/unit/unittest/pchmanagerserver-test.cpp +++ b/tests/unit/unittest/pchmanagerserver-test.cpp @@ -47,6 +47,7 @@ using Utils::SmallString; using ClangBackEnd::V2::FileContainer; using ClangBackEnd::V2::FileContainers; using ClangBackEnd::ProjectPartContainer; +using ClangBackEnd::ProjectPartContainers; class PchManagerServer : public ::testing::Test { @@ -198,8 +199,11 @@ TEST_F(PchManagerServer, DontGeneratePchIfGeneratedFilesAreNotValid) { InSequence s; + EXPECT_CALL(mockProjectParts, update(ElementsAre(projectPart1))) + .WillOnce(Return(ProjectPartContainers{projectPart1})); EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(false)); EXPECT_CALL(mockPchTaskGenerator, addProjectParts(_, _)).Times(0); + EXPECT_CALL(mockProjectParts, updateDeferred(ElementsAre(projectPart1))); server.updateProjectParts( ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}}); @@ -209,8 +213,11 @@ TEST_F(PchManagerServer, GeneratePchIfGeneratedFilesAreValid) { InSequence s; + EXPECT_CALL(mockProjectParts, update(ElementsAre(projectPart1))) + .WillOnce(Return(ProjectPartContainers{projectPart1})); EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(true)); EXPECT_CALL(mockPchTaskGenerator, addProjectParts(_, _)); + EXPECT_CALL(mockProjectParts, updateDeferred(_)).Times(0); server.updateProjectParts( ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}}); From 0082a82fc62fef78b64558faf70a7691452bb1f3 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Wed, 30 Jan 2019 18:48:59 +0100 Subject: [PATCH 08/19] ClangPchManager: Merge system pch tasks The merging of the include search paths is quite heuristic but we could provide an option to disable pch merging so users can decide themselves. Maybe we could give user feedback why we cannot merge but this is quite advanced. Task-number: QTCREATORBUG-21381 Change-Id: Iac6af0c587b631d2151f63d6d97215ed6919819f Reviewed-by: Ivan Donchevskii --- src/libs/clangsupport/commandlinebuilder.h | 7 + src/libs/clangsupport/compilermacro.h | 28 +- .../clangpchmanagerbackend/source/pchtask.h | 9 +- .../source/pchtaskgenerator.cpp | 8 +- .../source/pchtasksmerger.cpp | 129 ++++++- .../source/pchtasksmerger.h | 14 +- .../source/usedmacrofilter.h | 92 +++-- .../source/usedmacro.h | 4 + .../unit/unittest/commandlinebuilder-test.cpp | 2 +- .../unit/unittest/gtest-creator-printing.cpp | 7 +- tests/unit/unittest/pchtaskgenerator-test.cpp | 2 - tests/unit/unittest/pchtasksmerger-test.cpp | 326 ++++++++++++++++-- tests/unit/unittest/usedmacrofilter-test.cpp | 37 +- 13 files changed, 544 insertions(+), 121 deletions(-) diff --git a/src/libs/clangsupport/commandlinebuilder.h b/src/libs/clangsupport/commandlinebuilder.h index 6a9975a3ac9..df116197ab1 100644 --- a/src/libs/clangsupport/commandlinebuilder.h +++ b/src/libs/clangsupport/commandlinebuilder.h @@ -182,6 +182,13 @@ public: { CompilerMacros macros = compilerMacros; + macros.erase(std::remove_if(macros.begin(), + macros.end(), + [](const auto ¯o) { + return macro.type == CompilerMacroType::NotDefined; + }), + macros.end()); + std::sort(macros.begin(), macros.end(), [](const CompilerMacro &first, const CompilerMacro &second) { diff --git a/src/libs/clangsupport/compilermacro.h b/src/libs/clangsupport/compilermacro.h index ddba1a40762..798bab2ebd7 100644 --- a/src/libs/clangsupport/compilermacro.h +++ b/src/libs/clangsupport/compilermacro.h @@ -31,6 +31,8 @@ namespace ClangBackEnd { +enum class CompilerMacroType : unsigned char { Define, NotDefined }; + class CompilerMacro { public: @@ -40,39 +42,57 @@ public: : key(std::move(key)) , value(std::move(value)) , index(index) + , type(CompilerMacroType::Define) + {} + + CompilerMacro(Utils::SmallString &&key) + : key(std::move(key)) + {} + + CompilerMacro(const Utils::SmallString &key) + : key(key) {} friend QDataStream &operator<<(QDataStream &out, const CompilerMacro &compilerMacro) { out << compilerMacro.key; out << compilerMacro.value; + out << compilerMacro.index; + out << static_cast(compilerMacro.type); return out; } friend QDataStream &operator>>(QDataStream &in, CompilerMacro &compilerMacro) { + unsigned char type; + in >> compilerMacro.key; in >> compilerMacro.value; + in >> compilerMacro.index; + in >> type; + + compilerMacro.type = static_cast(type); return in; } friend bool operator==(const CompilerMacro &first, const CompilerMacro &second) { - return first.key == second.key - && first.value == second.value; + return first.key == second.key && first.value == second.value && first.type == second.type; } friend bool operator<(const CompilerMacro &first, const CompilerMacro &second) { - return std::tie(first.key, first.value) < std::tie(second.key, second.value); + return std::tie(first.key, first.type, first.value) + < std::tie(second.key, second.type, second.value); } public: Utils::SmallString key; Utils::SmallString value; - int index = 0; + int index = -1; + CompilerMacroType type = CompilerMacroType::NotDefined; }; using CompilerMacros = std::vector; diff --git a/src/tools/clangpchmanagerbackend/source/pchtask.h b/src/tools/clangpchmanagerbackend/source/pchtask.h index 46c3bf2503f..c94d7ac8164 100644 --- a/src/tools/clangpchmanagerbackend/source/pchtask.h +++ b/src/tools/clangpchmanagerbackend/source/pchtask.h @@ -43,7 +43,7 @@ public: FilePathIds &&includes, FilePathIds &&allIncludes, CompilerMacros &&compilerMacros, - UsedMacros &&usedMacros, + Utils::SmallStringVector &&usedMacros, Utils::SmallStringVector toolChainArguments, IncludeSearchPaths systemIncludeSearchPaths, IncludeSearchPaths projectIncludeSearchPaths, @@ -54,7 +54,6 @@ public: , includes(includes) , allIncludes(allIncludes) , compilerMacros(compilerMacros) - , usedMacros(usedMacros) , systemIncludeSearchPaths(std::move(systemIncludeSearchPaths)) , projectIncludeSearchPaths(std::move(projectIncludeSearchPaths)) , toolChainArguments(std::move(toolChainArguments)) @@ -67,7 +66,7 @@ public: FilePathIds &&includes, FilePathIds &&allIncludes, CompilerMacros &&compilerMacros, - UsedMacros &&usedMacros, + Utils::SmallStringVector &&usedMacros, Utils::SmallStringVector toolChainArguments, IncludeSearchPaths systemIncludeSearchPaths, IncludeSearchPaths projectIncludeSearchPaths, @@ -78,7 +77,6 @@ public: , includes(includes) , allIncludes(allIncludes) , compilerMacros(compilerMacros) - , usedMacros(usedMacros) , systemIncludeSearchPaths(std::move(systemIncludeSearchPaths)) , projectIncludeSearchPaths(std::move(projectIncludeSearchPaths)) , toolChainArguments(std::move(toolChainArguments)) @@ -92,7 +90,6 @@ public: return first.systemPchPath == second.systemPchPath && first.projectPartIds == second.projectPartIds && first.includes == second.includes && first.compilerMacros == second.compilerMacros - && first.usedMacros == second.usedMacros && first.systemIncludeSearchPaths == second.systemIncludeSearchPaths && first.projectIncludeSearchPaths == second.projectIncludeSearchPaths && first.toolChainArguments == second.toolChainArguments @@ -109,13 +106,13 @@ public: FilePathIds includes; FilePathIds allIncludes; CompilerMacros compilerMacros; - UsedMacros usedMacros; IncludeSearchPaths systemIncludeSearchPaths; IncludeSearchPaths projectIncludeSearchPaths; Utils::SmallStringVector toolChainArguments; Utils::Language language = Utils::Language::Cxx; Utils::LanguageVersion languageVersion = Utils::LanguageVersion::CXX98; Utils::LanguageExtension languageExtension = Utils::LanguageExtension::None; + bool isMerged = false; }; class PchTaskSet diff --git a/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.cpp b/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.cpp index 569e4a1d8a3..82584ad1dd8 100644 --- a/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.cpp +++ b/src/tools/clangpchmanagerbackend/source/pchtaskgenerator.cpp @@ -42,9 +42,9 @@ void PchTaskGenerator::addProjectParts(ProjectPartContainers &&projectParts, for (auto &projectPart : projectParts) { BuildDependency buildDependency = m_buildDependenciesProvider.create(projectPart); - UsedMacroFilter filter{buildDependency.includes, buildDependency.usedMacros}; - - filter.filter(projectPart.compilerMacros); + UsedMacroFilter filter{buildDependency.includes, + buildDependency.usedMacros, + projectPart.compilerMacros}; pchTaskSets.emplace_back(PchTask{projectPart.projectPartId.clone(), std::move(filter.topSystemIncludes), @@ -53,7 +53,7 @@ void PchTaskGenerator::addProjectParts(ProjectPartContainers &&projectParts, std::move(filter.systemUsedMacros), projectPart.toolChainArguments, projectPart.systemIncludeSearchPaths, - projectPart.projectIncludeSearchPaths, + {}, projectPart.language, projectPart.languageVersion, projectPart.languageExtension}, diff --git a/src/tools/clangpchmanagerbackend/source/pchtasksmerger.cpp b/src/tools/clangpchmanagerbackend/source/pchtasksmerger.cpp index 9ace04e09de..636d9a32364 100644 --- a/src/tools/clangpchmanagerbackend/source/pchtasksmerger.cpp +++ b/src/tools/clangpchmanagerbackend/source/pchtasksmerger.cpp @@ -30,20 +30,10 @@ namespace ClangBackEnd { void PchTasksMerger::mergeTasks(PchTaskSets &&taskSets, - Utils::SmallStringVector &&/*toolChainArguments*/) + Utils::SmallStringVector && /*toolChainArguments*/) { - PchTasks systemTasks; - systemTasks.reserve(taskSets.size()); - PchTasks projectTasks; - projectTasks.reserve(taskSets.size()); - - for (PchTaskSet &taskSet : taskSets) { - projectTasks.push_back(std::move(taskSet.project)); - systemTasks.push_back(std::move(taskSet.system)); - } - - m_pchTaskQueue.addSystemPchTasks(std::move(systemTasks)); - m_pchTaskQueue.addProjectPchTasks(std::move(projectTasks)); + mergeSystemTasks(taskSets); + addProjectTasksToQueue(taskSets); m_pchTaskQueue.processEntries(); } @@ -52,4 +42,117 @@ void PchTasksMerger::removePchTasks(const Utils::SmallStringVector &projectPartI m_pchTaskQueue.removePchTasks(projectPartIds); } +template>> +Result merge(Container &&first, Container &&second) +{ + Result result; + result.reserve(first.size() + second.size()); + + std::set_union(std::make_move_iterator(first.begin()), + std::make_move_iterator(first.end()), + std::make_move_iterator(second.begin()), + std::make_move_iterator(second.end()), + std::back_inserter(result)); + + return result; +} + +CompilerMacros PchTasksMerger::mergeMacros(const CompilerMacros &firstCompilerMacros, + const CompilerMacros &secondCompilerMacros) +{ + return merge(firstCompilerMacros, secondCompilerMacros); +} + +bool PchTasksMerger::hasDuplicates(const CompilerMacros &compilerMacros) +{ + auto found = std::adjacent_find(compilerMacros.begin(), + compilerMacros.end(), + [](const CompilerMacro &first, const CompilerMacro &second) { + return first.key == second.key; + }); + + return found == compilerMacros.end(); +} + +IncludeSearchPaths mergeIncludeSearchPaths(IncludeSearchPaths &&first, IncludeSearchPaths &&second) +{ + if (first.size() > second.size()) + return merge(first, second); + + return merge(first, second); +} + +bool PchTasksMerger::mergePchTasks(PchTask &firstTask, PchTask &secondTask) +{ + if (firstTask.language != secondTask.language || firstTask.isMerged || secondTask.isMerged) + return false; + + CompilerMacros macros = mergeMacros(firstTask.compilerMacros, secondTask.compilerMacros); + + secondTask.isMerged = hasDuplicates(macros); + + if (secondTask.isMerged && firstTask.language == secondTask.language) { + firstTask.projectPartIds = merge(std::move(firstTask.projectPartIds), + std::move(secondTask.projectPartIds)); + firstTask.includes = merge(std::move(firstTask.includes), std::move(secondTask.includes)); + firstTask.allIncludes = merge(std::move(firstTask.allIncludes), + std::move(secondTask.allIncludes)); + firstTask.compilerMacros = std::move(macros); + firstTask.systemIncludeSearchPaths = mergeIncludeSearchPaths( + std::move(firstTask.systemIncludeSearchPaths), + std::move(secondTask.systemIncludeSearchPaths)); + + firstTask.languageVersion = std::max(firstTask.languageVersion, secondTask.languageVersion); + firstTask.languageExtension = firstTask.languageExtension | secondTask.languageExtension; + } + + return secondTask.isMerged; +} + +void PchTasksMerger::mergePchTasks(PchTasks &tasks) +{ + auto begin = tasks.begin(); + + while (begin != tasks.end()) { + begin = std::find_if(begin, tasks.end(), [](const auto &task) { return !task.isMerged; }); + if (begin != tasks.end()) { + PchTask &baseTask = *begin; + ++begin; + + std::for_each(begin, tasks.end(), [&](PchTask ¤tTask) { + mergePchTasks(baseTask, currentTask); + }); + } + } +} + +void PchTasksMerger::addProjectTasksToQueue(PchTaskSets &taskSets) +{ + PchTasks projectTasks; + projectTasks.reserve(taskSets.size()); + + for (PchTaskSet &taskSet : taskSets) + projectTasks.push_back(std::move(taskSet.project)); + + m_pchTaskQueue.addProjectPchTasks(std::move(projectTasks)); +} + +void PchTasksMerger::mergeSystemTasks(PchTaskSets &taskSets) +{ + PchTasks systemTasks; + systemTasks.reserve(taskSets.size()); + + for (PchTaskSet &taskSet : taskSets) + systemTasks.push_back(std::move(taskSet.system)); + + mergePchTasks(systemTasks); + + systemTasks.erase(std::remove_if(systemTasks.begin(), + systemTasks.end(), + [](const PchTask &task) { return task.isMerged; }), + systemTasks.end()); + + m_pchTaskQueue.addSystemPchTasks(std::move(systemTasks)); +} + } // namespace ClangBackEnd diff --git a/src/tools/clangpchmanagerbackend/source/pchtasksmerger.h b/src/tools/clangpchmanagerbackend/source/pchtasksmerger.h index 9c777b8a7cb..26df2022b4d 100644 --- a/src/tools/clangpchmanagerbackend/source/pchtasksmerger.h +++ b/src/tools/clangpchmanagerbackend/source/pchtasksmerger.h @@ -32,7 +32,7 @@ namespace ClangBackEnd { class PchTaskQueueInterface; -class PchTasksMerger : public PchTasksMergerInterface +class PchTasksMerger final : public PchTasksMergerInterface { public: PchTasksMerger(PchTaskQueueInterface &pchTaskQueue) @@ -42,6 +42,18 @@ public: void mergeTasks(PchTaskSets &&taskSets, Utils::SmallStringVector &&toolChainArguments) override; void removePchTasks(const Utils::SmallStringVector &projectPartIds) override; + static CompilerMacros mergeMacros(const CompilerMacros &firstCompilerMacros, + const CompilerMacros &secondCompilerMacros); + static bool hasDuplicates(const CompilerMacros &compilerMacros); + + static bool mergePchTasks(PchTask &first, PchTask &second); + + static void mergePchTasks(PchTasks &tasks); + +private: + void addProjectTasksToQueue(PchTaskSets &taskSets); + void mergeSystemTasks(PchTaskSets &taskSets); + private: PchTaskQueueInterface &m_pchTaskQueue; }; diff --git a/src/tools/clangpchmanagerbackend/source/usedmacrofilter.h b/src/tools/clangpchmanagerbackend/source/usedmacrofilter.h index e7969c4ea1b..0b39ae1f4f2 100644 --- a/src/tools/clangpchmanagerbackend/source/usedmacrofilter.h +++ b/src/tools/clangpchmanagerbackend/source/usedmacrofilter.h @@ -32,6 +32,27 @@ namespace ClangBackEnd { +template +inline OutputIterator set_greedy_intersection(InputIterator1 first1, + InputIterator1 last1, + InputIterator2 first2, + InputIterator2 last2, + OutputIterator result, + Compare comp) +{ + while (first1 != last1 && first2 != last2) + if (comp(*first1, *first2)) + ++first1; + else if (comp(*first2, *first1)) + ++first2; + else { + *result = *first1; + ++first1; + ++result; + } + return result; +} + class UsedMacroFilter { public: @@ -41,11 +62,14 @@ public: FilePathIds system; }; - UsedMacroFilter(const SourceEntries &includes, const UsedMacros &usedMacros) + UsedMacroFilter(const SourceEntries &includes, + const UsedMacros &usedMacros, + const CompilerMacros &compilerMacros) { filterIncludes(includes); systemUsedMacros = filterUsedMarcos(usedMacros, systemIncludes); projectUsedMacros = filterUsedMarcos(usedMacros, projectIncludes); + filter(compilerMacros); } void filterIncludes(const SourceEntries &includes) @@ -98,7 +122,8 @@ private: allIncludes.emplace_back(include.sourceId); } - static UsedMacros filterUsedMarcos(const UsedMacros &usedMacros, const FilePathIds &filePathId) + static Utils::SmallStringVector filterUsedMarcos(const UsedMacros &usedMacros, + const FilePathIds &filePathId) { struct Compare { @@ -113,52 +138,57 @@ private: } }; - UsedMacros filtertedMacros; + Utils::SmallStringVector filtertedMacros; filtertedMacros.reserve(usedMacros.size()); - std::set_intersection(usedMacros.begin(), - usedMacros.end(), - filePathId.begin(), - filePathId.end(), - std::back_inserter(filtertedMacros), - Compare{}); + set_greedy_intersection(usedMacros.begin(), + usedMacros.end(), + filePathId.begin(), + filePathId.end(), + std::back_inserter(filtertedMacros), + Compare{}); - std::sort(filtertedMacros.begin(), - filtertedMacros.end(), - [](const UsedMacro &first, const UsedMacro &second) { - return first.macroName < second.macroName; - }); + std::sort(filtertedMacros.begin(), filtertedMacros.end()); return filtertedMacros; } static CompilerMacros filtercompilerMacros(const CompilerMacros &indexedCompilerMacro, - const UsedMacros &usedMacros) + const Utils::SmallStringVector &usedMacros) { + CompilerMacros filtertedCompilerMacros; + filtertedCompilerMacros.reserve(indexedCompilerMacro.size() + usedMacros.size()); + struct Compare { - bool operator()(const UsedMacro &usedMacro, - const CompilerMacro &compileMacro) + bool operator()(const CompilerMacro &compilerMacro, Utils::SmallStringView usedMacro) { - return usedMacro.macroName < compileMacro.key; + return compilerMacro.key < usedMacro; } - bool operator()(const CompilerMacro &compileMacro, - const UsedMacro &usedMacro) + bool operator()(Utils::SmallStringView usedMacro, const CompilerMacro &compilerMacro) { - return compileMacro.key < usedMacro.macroName; + return usedMacro < compilerMacro.key; } }; - CompilerMacros filtertedCompilerMacros; - filtertedCompilerMacros.reserve(indexedCompilerMacro.size()); + set_greedy_intersection(indexedCompilerMacro.begin(), + indexedCompilerMacro.end(), + usedMacros.begin(), + usedMacros.end(), + std::back_inserter(filtertedCompilerMacros), + Compare{}); - std::set_intersection(indexedCompilerMacro.begin(), - indexedCompilerMacro.end(), - usedMacros.begin(), - usedMacros.end(), - std::back_inserter(filtertedCompilerMacros), - Compare{}); + auto split = filtertedCompilerMacros.end(); + + std::set_difference(usedMacros.begin(), + usedMacros.end(), + filtertedCompilerMacros.begin(), + filtertedCompilerMacros.end(), + std::back_inserter(filtertedCompilerMacros), + Compare{}); + + std::inplace_merge(filtertedCompilerMacros.begin(), split, filtertedCompilerMacros.end()); return filtertedCompilerMacros; } @@ -169,8 +199,8 @@ public: FilePathIds systemIncludes; FilePathIds topProjectIncludes; FilePathIds topSystemIncludes; - UsedMacros projectUsedMacros; - UsedMacros systemUsedMacros; + Utils::SmallStringVector projectUsedMacros; + Utils::SmallStringVector systemUsedMacros; CompilerMacros projectCompilerMacros; CompilerMacros systemCompilerMacros; }; diff --git a/src/tools/clangrefactoringbackend/source/usedmacro.h b/src/tools/clangrefactoringbackend/source/usedmacro.h index a7312b63761..8fc992e39b1 100644 --- a/src/tools/clangrefactoringbackend/source/usedmacro.h +++ b/src/tools/clangrefactoringbackend/source/usedmacro.h @@ -62,6 +62,10 @@ public: { return !(first == second); } + + operator Utils::SmallStringView() const { return macroName; } + operator const Utils::SmallString &() const { return macroName; } + public: Utils::SmallString macroName; FilePathId filePathId; diff --git a/tests/unit/unittest/commandlinebuilder-test.cpp b/tests/unit/unittest/commandlinebuilder-test.cpp index f51a4f6a714..c3b60df3694 100644 --- a/tests/unit/unittest/commandlinebuilder-test.cpp +++ b/tests/unit/unittest/commandlinebuilder-test.cpp @@ -511,7 +511,7 @@ TYPED_TEST(CommandLineBuilder, IncludePchPath) TYPED_TEST(CommandLineBuilder, CompilerMacros) { - this->emptyProjectInfo.compilerMacros = {{"YI", "1", 2}, {"ER", "2", 1}}; + this->emptyProjectInfo.compilerMacros = {{"YI", "1", 2}, {"ER", "2", 1}, {"SAN"}}; Builder builder{this->emptyProjectInfo}; diff --git a/tests/unit/unittest/gtest-creator-printing.cpp b/tests/unit/unittest/gtest-creator-printing.cpp index 682d28740b0..3901cb0d305 100644 --- a/tests/unit/unittest/gtest-creator-printing.cpp +++ b/tests/unit/unittest/gtest-creator-printing.cpp @@ -1157,10 +1157,9 @@ std::ostream &operator<<(std::ostream &out, const PchCreatorIncludes &includes) std::ostream &operator<<(std::ostream &out, const PchTask &task) { return out << "(" << task.projectPartIds << ", " << task.includes << ", " << task.compilerMacros - << ", " << task.usedMacros << ", " << toText(task.language) << ", " - << task.systemIncludeSearchPaths << ", " << task.projectIncludeSearchPaths << ", " - << task.toolChainArguments << ", " << toText(task.languageVersion) << ", " - << toText(task.languageExtension) << ")"; + << toText(task.language) << ", " << task.systemIncludeSearchPaths << ", " + << task.projectIncludeSearchPaths << ", " << task.toolChainArguments << ", " + << toText(task.languageVersion) << ", " << toText(task.languageExtension) << ")"; } std::ostream &operator<<(std::ostream &out, const PchTaskSet &taskSet) diff --git a/tests/unit/unittest/pchtaskgenerator-test.cpp b/tests/unit/unittest/pchtaskgenerator-test.cpp index 0b2ec3968e3..a3ebb3c17be 100644 --- a/tests/unit/unittest/pchtaskgenerator-test.cpp +++ b/tests/unit/unittest/pchtaskgenerator-test.cpp @@ -99,7 +99,6 @@ TEST_F(PchTaskGenerator, AddProjectParts) Field(&PchTask::allIncludes, IsEmpty()), Field(&PchTask::compilerMacros, ElementsAre(CompilerMacro{"SE", "4", 4}, CompilerMacro{"WU", "5", 5})), - Field(&PchTask::usedMacros, ElementsAre(UsedMacro{"SE", 4}, UsedMacro{"WU", 5})), Field(&PchTask::systemIncludeSearchPaths, ElementsAre( IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System}, @@ -120,7 +119,6 @@ TEST_F(PchTaskGenerator, AddProjectParts) Field(&PchTask::allIncludes, ElementsAre(1, 2, 3, 4, 5)), Field(&PchTask::compilerMacros, ElementsAre(CompilerMacro{"YI", "1", 1}, CompilerMacro{"SAN", "3", 3})), - Field(&PchTask::usedMacros, ElementsAre(UsedMacro{"YI", 1}, UsedMacro{"SAN", 3})), Field(&PchTask::systemIncludeSearchPaths, ElementsAre( IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System}, diff --git a/tests/unit/unittest/pchtasksmerger-test.cpp b/tests/unit/unittest/pchtasksmerger-test.cpp index f54e7aca582..e878438f339 100644 --- a/tests/unit/unittest/pchtasksmerger-test.cpp +++ b/tests/unit/unittest/pchtasksmerger-test.cpp @@ -31,11 +31,14 @@ namespace { +using ClangBackEnd::CompilerMacro; +using ClangBackEnd::CompilerMacros; using ClangBackEnd::IncludeSearchPath; using ClangBackEnd::IncludeSearchPathType; using ClangBackEnd::PchTask; using ClangBackEnd::PchTaskSet; - +using Merger = ClangBackEnd::PchTasksMerger; +using Id = ClangBackEnd::FilePathId; class PchTasksMerger : public testing::Test { protected: @@ -51,48 +54,58 @@ protected: ClangBackEnd::PchTasksMerger merger{mockPchTaskQueue}; PchTask systemTask1{"ProjectPart1", {1, 2}, - {1, 2}, + {1, 2, 3}, {{"YI", "1", 1}, {"SAN", "3", 3}}, - {{"LIANG", 0}, {"YI", 1}}, + {"YI", "LIANG"}, {"--yi"}, - {IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System}, - IncludeSearchPath{"/builtin/path", 3, IncludeSearchPathType::BuiltIn}, - IncludeSearchPath{"/framework/path", 1, IncludeSearchPathType::System}}, - {IncludeSearchPath{"/to/path1", 1, IncludeSearchPathType::User}, - IncludeSearchPath{"/to/path2", 2, IncludeSearchPathType::User}}}; + {{"/system/path", 2, IncludeSearchPathType::System}, + {"/builtin/path2", 3, IncludeSearchPathType::BuiltIn}, + {"/framework/path", 1, IncludeSearchPathType::System}}, + {{"/to/path1", 1, IncludeSearchPathType::User}, + {"/to/path2", 2, IncludeSearchPathType::User}}}; PchTask projectTask1{"ProjectPart1", {11, 12}, {11, 12}, {{"SE", "4", 4}, {"WU", "5", 5}}, - {{"ER", 2}, {"SAN", 3}}, + {"ER", "SAN"}, {"--yi"}, - {IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System}, - IncludeSearchPath{"/builtin/path", 3, IncludeSearchPathType::BuiltIn}, - IncludeSearchPath{"/framework/path", 1, IncludeSearchPathType::System}}, - {IncludeSearchPath{"/to/path1", 1, IncludeSearchPathType::User}, - IncludeSearchPath{"/to/path2", 2, IncludeSearchPathType::User}}}; + {{"/system/path", 1, IncludeSearchPathType::System}, + {"/builtin/path", 2, IncludeSearchPathType::BuiltIn}}, + {{"/to/path1", 1, IncludeSearchPathType::User}, + {"/to/path2", 2, IncludeSearchPathType::User}}}; PchTask systemTask2{"ProjectPart2", - {1, 2}, - {1, 2}, - {{"YI", "1", 1}, {"SAN", "3", 3}}, - {{"LIANG", 0}, {"YI", 1}}, + {11, 12}, + {11, 12, 13}, + {{"SE", "4", 4}, {"WU", "5", 5}}, + {"ER", "SAN"}, {"--yi"}, - {IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System}, - IncludeSearchPath{"/builtin/path", 3, IncludeSearchPathType::BuiltIn}, - IncludeSearchPath{"/framework/path", 1, IncludeSearchPathType::System}}, - {IncludeSearchPath{"/to/path1", 1, IncludeSearchPathType::User}, - IncludeSearchPath{"/to/path2", 2, IncludeSearchPathType::User}}}; + {{"/system/path", 2, IncludeSearchPathType::System}, + {"/builtin/path", 3, IncludeSearchPathType::BuiltIn}, + {"/framework/path", 1, IncludeSearchPathType::System}}, + {{"/to/path1", 1, IncludeSearchPathType::User}, + {"/to/path2", 2, IncludeSearchPathType::User}}}; PchTask projectTask2{"ProjectPart2", {11, 12}, {11, 12}, {{"SE", "4", 4}, {"WU", "5", 5}}, - {{"ER", 2}, {"SAN", 3}}, + {"ER", "SAN"}, {"--yi"}, - {IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System}, - IncludeSearchPath{"/builtin/path", 3, IncludeSearchPathType::BuiltIn}, - IncludeSearchPath{"/framework/path", 1, IncludeSearchPathType::System}}, - {IncludeSearchPath{"/to/path1", 1, IncludeSearchPathType::User}, - IncludeSearchPath{"/to/path2", 2, IncludeSearchPathType::User}}}; + {{"/system/path", 2, IncludeSearchPathType::System}, + {"/builtin/path", 3, IncludeSearchPathType::BuiltIn}, + {"/framework/path", 1, IncludeSearchPathType::System}}, + {{"/to/path1", 1, IncludeSearchPathType::User}, + {"/to/path2", 2, IncludeSearchPathType::User}}}; + PchTask systemTask3{"ProjectPart3", + {1, 2}, + {1, 2}, + {{"YI", "2", 1}, {"SAN", "3", 3}}, + {"YI", "LIANG"}, + {"--yi"}, + {{"/system/path", 2, IncludeSearchPathType::System}, + {"/builtin/path", 3, IncludeSearchPathType::BuiltIn}, + {"/framework/path", 1, IncludeSearchPathType::System}}, + {{"/to/path1", 1, IncludeSearchPathType::User}, + {"/to/path2", 2, IncludeSearchPathType::User}}}; Utils::SmallStringVector toolChainArguments = {"toolChainArguments"}; }; @@ -103,21 +116,33 @@ TEST_F(PchTasksMerger, AddProjectTasks) EXPECT_CALL(mockPchTaskQueue, addProjectPchTasks(ElementsAre(projectTask1, projectTask2))); EXPECT_CALL(mockPchTaskQueue, processEntries()); - merger.mergeTasks( - {{clone(systemTask1), clone(projectTask1)}, {clone(systemTask1), clone(projectTask2)}}, - std::move(toolChainArguments)); + merger.mergeTasks({{clone(systemTask1), clone(projectTask1)}, + {clone(systemTask1), clone(projectTask2)}}, + std::move(toolChainArguments)); } TEST_F(PchTasksMerger, AddSystemTasks) { InSequence s; - EXPECT_CALL(mockPchTaskQueue, addSystemPchTasks(ElementsAre(systemTask1, systemTask2))); + EXPECT_CALL(mockPchTaskQueue, addSystemPchTasks(ElementsAre(_, systemTask3))); EXPECT_CALL(mockPchTaskQueue, processEntries()); - merger.mergeTasks( - {{clone(systemTask1), clone(projectTask1)}, {clone(systemTask2), clone(projectTask2)}}, - std::move(toolChainArguments)); + merger.mergeTasks({{clone(systemTask1), clone(projectTask1)}, + {clone(systemTask2), clone(projectTask2)}, + {clone(systemTask3), clone(projectTask2)}}, + std::move(toolChainArguments)); +} + +TEST_F(PchTasksMerger, AddSystemOnlyOneTask) +{ + InSequence s; + + EXPECT_CALL(mockPchTaskQueue, addSystemPchTasks(ElementsAre(systemTask1))); + EXPECT_CALL(mockPchTaskQueue, processEntries()); + + merger.mergeTasks({{clone(systemTask1), clone(projectTask1)}}, + std::move(toolChainArguments)); } TEST_F(PchTasksMerger, RemoveTasks) @@ -126,4 +151,233 @@ TEST_F(PchTasksMerger, RemoveTasks) merger.removePchTasks({"project1", "project2"}); } + +TEST_F(PchTasksMerger, MergeMacros) +{ + CompilerMacros compilerMacros1{{"ER", "2", 2}, {"SE", "4", 1}, {"YI"}, {"SAN", "3", 3}}; + CompilerMacros compilerMacros2{{"BA"}, {"ER", "2", 2}, {"YI", "1", 1}, {"SAN", "3", 3}}; + + auto macros = Merger::mergeMacros(compilerMacros1, compilerMacros2); + + ASSERT_THAT(macros, + ElementsAre(CompilerMacro{"BA"}, + CompilerMacro{"ER", "2", 2}, + CompilerMacro{"SE", "4", 1}, + CompilerMacro{"YI", "1", 1}, + CompilerMacro{"YI"}, + CompilerMacro{"SAN", "3", 3})); } + +TEST_F(PchTasksMerger, MacrosCanBeMerged) +{ + CompilerMacros compilerMacros1{{"ER", "2", 2}, {"QI"}, {"SE", "4", 1}, {"SAN", "3", 3}}; + CompilerMacros compilerMacros2{{"BA"}, {"ER", "2", 2}, {"YI", "1", 1}, {"SAN", "3", 3}}; + + auto canBeMerged = Merger::hasDuplicates(Merger::mergeMacros(compilerMacros1, compilerMacros2)); + + ASSERT_TRUE(canBeMerged); +} + +TEST_F(PchTasksMerger, MacrosCannotBeMergedBecauseDifferentValue) +{ + CompilerMacros compilerMacros1{{"ER", "2", 2}, {"SE", "4", 1}, {"SAN", "3", 3}}; + CompilerMacros compilerMacros2{{"ER", "1", 2}, {"YI", "1", 1}, {"SAN", "3", 3}}; + + auto canBeMerged = Merger::hasDuplicates(Merger::mergeMacros(compilerMacros1, compilerMacros2)); + + ASSERT_FALSE(canBeMerged); +} + +TEST_F(PchTasksMerger, MacrosCannotBeMergedBecauseUndefinedMacro) +{ + CompilerMacros compilerMacros1{{"ER", "2", 2}, {"SE", "4", 1}, {"YI"}, {"SAN", "3", 3}}; + CompilerMacros compilerMacros2{{"ER", "2", 2}, {"YI", "1", 1}, {"SAN", "3", 3}}; + + auto canBeMerged = Merger::hasDuplicates(Merger::mergeMacros(compilerMacros1, compilerMacros2)); + + ASSERT_FALSE(canBeMerged); +} + +TEST_F(PchTasksMerger, CanMergePchTasks) +{ + auto canBeMerged = Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_TRUE(canBeMerged); +} + +TEST_F(PchTasksMerger, CannotMergePchTasks) +{ + auto canBeMerged = Merger::mergePchTasks(systemTask1, systemTask3); + + ASSERT_FALSE(canBeMerged); +} + +TEST_F(PchTasksMerger, IsMergedIsSet) +{ + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_TRUE(systemTask2.isMerged); +} + +TEST_F(PchTasksMerger, IsMergedIsNotSet) +{ + Merger::mergePchTasks(systemTask1, systemTask3); + + ASSERT_FALSE(systemTask3.isMerged); +} + +TEST_F(PchTasksMerger, DontMergeLanguage) +{ + systemTask2.language = Utils::Language::C; + + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_FALSE(systemTask2.isMerged); +} + +TEST_F(PchTasksMerger, MergeCompilerMacros) +{ + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_THAT(systemTask1.compilerMacros, + ElementsAre(CompilerMacro{"SE", "4", 4}, + CompilerMacro{"WU", "5", 5}, + CompilerMacro{"YI", "1", 1}, + CompilerMacro{"SAN", "3", 3})); +} + +TEST_F(PchTasksMerger, DontMergeCompilerMacros) +{ + Merger::mergePchTasks(systemTask1, systemTask3); + + ASSERT_THAT(systemTask1.compilerMacros, + ElementsAre(CompilerMacro{"YI", "1", 1}, CompilerMacro{"SAN", "3", 3})); +} + +TEST_F(PchTasksMerger, MergeIncludes) +{ + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_THAT(systemTask1.includes, ElementsAre(1, 2, 11, 12)); +} + +TEST_F(PchTasksMerger, DontMergeIncludes) +{ + Merger::mergePchTasks(systemTask1, systemTask3); + + ASSERT_THAT(systemTask1.includes, ElementsAre(1, 2)); +} + +TEST_F(PchTasksMerger, MergeAllIncludes) +{ + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_THAT(systemTask1.allIncludes, ElementsAre(1, 2, 3, 11, 12, 13)); +} + +TEST_F(PchTasksMerger, DontAllMergeIncludes) +{ + Merger::mergePchTasks(systemTask1, systemTask3); + + ASSERT_THAT(systemTask1.allIncludes, ElementsAre(1, 2, 3)); +} + +TEST_F(PchTasksMerger, MergeProjectIds) +{ + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_THAT(systemTask1.projectPartIds, ElementsAre("ProjectPart1", "ProjectPart2")); +} + +TEST_F(PchTasksMerger, DontMergeProjectIds) +{ + Merger::mergePchTasks(systemTask1, systemTask3); + + ASSERT_THAT(systemTask1.projectPartIds, ElementsAre("ProjectPart1")); +} + +TEST_F(PchTasksMerger, MergeIncludeSearchPaths) +{ + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_THAT(systemTask1.systemIncludeSearchPaths, + ElementsAre(IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System}, + IncludeSearchPath{"/builtin/path", 3, IncludeSearchPathType::BuiltIn}, + IncludeSearchPath{"/builtin/path2", 3, IncludeSearchPathType::BuiltIn}, + IncludeSearchPath{"/framework/path", 1, IncludeSearchPathType::System})); +} + +TEST_F(PchTasksMerger, DontMergeIncludeSearchPaths) +{ + Merger::mergePchTasks(systemTask1, systemTask3); + + ASSERT_THAT(systemTask1.systemIncludeSearchPaths, + ElementsAre(IncludeSearchPath{"/system/path", 2, IncludeSearchPathType::System}, + IncludeSearchPath{"/builtin/path2", 3, IncludeSearchPathType::BuiltIn}, + IncludeSearchPath{"/framework/path", 1, IncludeSearchPathType::System})); +} + +TEST_F(PchTasksMerger, ChooseLanguageVersionFromFirstTask) +{ + systemTask2.languageVersion = Utils::LanguageVersion::CXX17; + + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_THAT(systemTask1.languageVersion, Utils::LanguageVersion::CXX17); +} + +TEST_F(PchTasksMerger, ChooseLanguageVersionFromSecondTask) +{ + systemTask1.languageVersion = Utils::LanguageVersion::CXX17; + + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_THAT(systemTask1.languageVersion, Utils::LanguageVersion::CXX17); +} + +TEST_F(PchTasksMerger, DontChooseLanguageVersion) +{ + systemTask3.languageVersion = Utils::LanguageVersion::CXX17; + + Merger::mergePchTasks(systemTask1, systemTask3); + + ASSERT_THAT(systemTask1.languageVersion, Utils::LanguageVersion::CXX98); +} + +TEST_F(PchTasksMerger, FirstTaskIsNotMergedAgain) +{ + systemTask1.isMerged = true; + + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_FALSE(systemTask2.isMerged); +} + +TEST_F(PchTasksMerger, DontMergeAlreadyMergedFirstTask) +{ + systemTask1.isMerged = true; + + bool isMerged = Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_FALSE(isMerged); +} + +TEST_F(PchTasksMerger, SecondTaskIsNotMergedAgain) +{ + systemTask2.isMerged = true; + + Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_THAT(systemTask1.includes, ElementsAre(1, 2)); +} + +TEST_F(PchTasksMerger, DontMergeAlreadyMergedSecondTask) +{ + systemTask2.isMerged = true; + + bool isMerged = Merger::mergePchTasks(systemTask1, systemTask2); + + ASSERT_FALSE(isMerged); +} + +} // namespace diff --git a/tests/unit/unittest/usedmacrofilter-test.cpp b/tests/unit/unittest/usedmacrofilter-test.cpp index bc5b79cccf5..b02cd7e5e02 100644 --- a/tests/unit/unittest/usedmacrofilter-test.cpp +++ b/tests/unit/unittest/usedmacrofilter-test.cpp @@ -46,7 +46,7 @@ protected: {3, SourceType::ProjectInclude, 0}, {4, SourceType::TopSystemInclude, 0}, {5, SourceType::TopProjectInclude, 0}}; - UsedMacros usedMacros{{"YI", 1}, {"ER", 2}, {"SAN", 3}, {"SE", 4}, {"WU", 5}}; + UsedMacros usedMacros{{"YI", 1}, {"ER", 2}, {"LIU", 2}, {"QI", 3}, {"SAN", 3}, {"SE", 4}, {"WU", 5}}; CompilerMacros compileMacros{{"YI", "1", 1}, {"ER", "2", 2}, {"SAN", "3", 3}, @@ -57,36 +57,35 @@ protected: TEST_F(UsedMacroFilter, SystemIncludes) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); ASSERT_THAT(filter.systemIncludes, ElementsAre(FilePathId{2}, FilePathId{4})); } TEST_F(UsedMacroFilter, ProjectIncludes) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); ASSERT_THAT(filter.projectIncludes, ElementsAre(FilePathId{3}, FilePathId{5})); } TEST_F(UsedMacroFilter, TopSystemIncludes) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); ASSERT_THAT(filter.topSystemIncludes, ElementsAre(FilePathId{4})); } TEST_F(UsedMacroFilter, TopProjectIncludes) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); ASSERT_THAT(filter.topProjectIncludes, ElementsAre(FilePathId{5})); } - TEST_F(UsedMacroFilter, AllIncludes) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); ASSERT_THAT(filter.allIncludes, ElementsAre(FilePathId{1}, FilePathId{2}, FilePathId{3}, FilePathId{4}, FilePathId{5})); @@ -94,36 +93,36 @@ TEST_F(UsedMacroFilter, AllIncludes) TEST_F(UsedMacroFilter, SystemUsedMacros) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); - ASSERT_THAT(filter.systemUsedMacros, ElementsAre(UsedMacro{"ER", 2}, UsedMacro{"SE", 4})); + ASSERT_THAT(filter.systemUsedMacros, ElementsAre("ER", "SE", "LIU")); } TEST_F(UsedMacroFilter, ProjectUsedMacros) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); - ASSERT_THAT(filter.projectUsedMacros, ElementsAre(UsedMacro{"WU", 5}, UsedMacro{"SAN", 3})); + ASSERT_THAT(filter.projectUsedMacros, ElementsAre("QI", "WU", "SAN")); } TEST_F(UsedMacroFilter, SystemCompileMacros) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); - - filter.filter(compileMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); ASSERT_THAT(filter.systemCompilerMacros, - ElementsAre(CompilerMacro{"ER", "2", 2}, CompilerMacro{"SE", "4", 4})); + ElementsAre(CompilerMacro{"ER", "2", 2}, + CompilerMacro{"SE", "4", 4}, + CompilerMacro{"LIU"})); } TEST_F(UsedMacroFilter, ProjectCompileMacros) { - ClangBackEnd::UsedMacroFilter filter(includes, usedMacros); - - filter.filter(compileMacros); + ClangBackEnd::UsedMacroFilter filter(includes, usedMacros, compileMacros); ASSERT_THAT(filter.projectCompilerMacros, - ElementsAre(CompilerMacro{"WU", "5", 5}, CompilerMacro{"SAN", "3", 3})); + ElementsAre(CompilerMacro{"QI"}, + CompilerMacro{"WU", "5", 5}, + CompilerMacro{"SAN", "3", 3})); } } // namespace From 6ab9078e872fd7e31979c849ef2640eb400f480c Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Mon, 4 Feb 2019 15:06:18 +0100 Subject: [PATCH 09/19] Project tree: Do not pretend to support moving files via Drag & Drop We claimed to support the move action, so on Windows the user could drag files away to e.g. the Desktop or the Explorer. This resulted in inconsistent state, because we do not (cannot?) handle an "external drop" in any of our build system managers. Instead, we now support only the copy action, which leaves our project sources untouched. This is also more likely to be what the user wants. Fixes: QTCREATORBUG-14494 Change-Id: Ie327780768f1ac68d6dbe95c3daa4aa4dc3f0f41 Reviewed-by: Alessandro Portale --- src/plugins/projectexplorer/projectmodels.cpp | 2 +- src/plugins/texteditor/texteditor.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/projectexplorer/projectmodels.cpp b/src/plugins/projectexplorer/projectmodels.cpp index ad79e82975d..d90e3495244 100644 --- a/src/plugins/projectexplorer/projectmodels.cpp +++ b/src/plugins/projectexplorer/projectmodels.cpp @@ -398,7 +398,7 @@ bool FlatModel::trimEmptyDirectories(WrapperNode *parent) Qt::DropActions FlatModel::supportedDragActions() const { - return Qt::MoveAction; + return Qt::CopyAction; } QStringList FlatModel::mimeTypes() const diff --git a/src/plugins/texteditor/texteditor.cpp b/src/plugins/texteditor/texteditor.cpp index aeefcb39cf6..c506d71377f 100644 --- a/src/plugins/texteditor/texteditor.cpp +++ b/src/plugins/texteditor/texteditor.cpp @@ -8568,7 +8568,7 @@ void TextEditorLinkLabel::mouseMoveEvent(QMouseEvent *event) data->addFile(m_link.targetFileName, m_link.targetLine, m_link.targetColumn); auto drag = new QDrag(this); drag->setMimeData(data); - drag->exec(Qt::MoveAction); + drag->exec(Qt::CopyAction); } void TextEditorLinkLabel::mouseReleaseEvent(QMouseEvent *event) From 9e8ae42343aff6d6429cbbb784105905c896d3a5 Mon Sep 17 00:00:00 2001 From: David Schulz Date: Tue, 5 Feb 2019 13:23:34 +0100 Subject: [PATCH 10/19] Core: consistent InfoBar member prefix Change-Id: Ie0cfda51b3936d7745c7876181e6d4b5242860ff Reviewed-by: Eike Ziller --- src/plugins/coreplugin/infobar.cpp | 30 +++++++++++++++--------------- src/plugins/coreplugin/infobar.h | 10 +++++----- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/plugins/coreplugin/infobar.cpp b/src/plugins/coreplugin/infobar.cpp index 53c0cc4abe5..827f1e8337a 100644 --- a/src/plugins/coreplugin/infobar.cpp +++ b/src/plugins/coreplugin/infobar.cpp @@ -48,15 +48,15 @@ QSettings *InfoBar::m_settings = nullptr; Utils::Theme *InfoBar::m_theme = nullptr; InfoBarEntry::InfoBarEntry(Id _id, const QString &_infoText, GlobalSuppressionMode _globalSuppression) - : id(_id) - , infoText(_infoText) - , globalSuppression(_globalSuppression) + : m_id(_id) + , m_infoText(_infoText) + , m_globalSuppression(_globalSuppression) { } void InfoBarEntry::setCustomButtonInfo(const QString &_buttonText, CallBack callBack) { - buttonText = _buttonText; + m_buttonText = _buttonText; m_buttonCallBack = callBack; } @@ -69,14 +69,14 @@ void InfoBarEntry::setCancelButtonInfo(CallBack callBack) void InfoBarEntry::setCancelButtonInfo(const QString &_cancelButtonText, CallBack callBack) { m_useCancelButton = true; - cancelButtonText = _cancelButtonText; + m_cancelButtonText = _cancelButtonText; m_cancelButtonCallBack = callBack; } void InfoBarEntry::removeCancelButton() { m_useCancelButton = false; - cancelButtonText.clear(); + m_cancelButtonText.clear(); m_cancelButtonCallBack = nullptr; } @@ -94,14 +94,14 @@ void InfoBar::addInfo(const InfoBarEntry &info) void InfoBar::removeInfo(Id id) { const int size = m_infoBarEntries.size(); - Utils::erase(m_infoBarEntries, Utils::equal(&InfoBarEntry::id, id)); + Utils::erase(m_infoBarEntries, Utils::equal(&InfoBarEntry::m_id, id)); if (size != m_infoBarEntries.size()) emit changed(); } bool InfoBar::containsInfo(Id id) const { - return Utils::anyOf(m_infoBarEntries, Utils::equal(&InfoBarEntry::id, id)); + return Utils::anyOf(m_infoBarEntries, Utils::equal(&InfoBarEntry::m_id, id)); } // Remove and suppress id @@ -240,7 +240,7 @@ void InfoBarDisplay::update() vbox->setMargin(0); vbox->addLayout(hbox); - QLabel *infoWidgetLabel = new QLabel(info.infoText); + QLabel *infoWidgetLabel = new QLabel(info.m_infoText); infoWidgetLabel->setWordWrap(true); hbox->addWidget(infoWidgetLabel); @@ -270,17 +270,17 @@ void InfoBarDisplay::update() m_isShowingDetailsWidget = false; } - if (!info.buttonText.isEmpty()) { + if (!info.m_buttonText.isEmpty()) { auto infoWidgetButton = new QToolButton; - infoWidgetButton->setText(info.buttonText); + infoWidgetButton->setText(info.m_buttonText); connect(infoWidgetButton, &QAbstractButton::clicked, [info]() { info.m_buttonCallBack(); }); hbox->addWidget(infoWidgetButton); } - const Id id = info.id; + const Id id = info.m_id; QToolButton *infoWidgetSuppressButton = nullptr; - if (info.globalSuppression == InfoBarEntry::GlobalSuppressionEnabled) { + if (info.m_globalSuppression == InfoBarEntry::GlobalSuppressionEnabled) { infoWidgetSuppressButton = new QToolButton; infoWidgetSuppressButton->setText(tr("Do Not Show Again")); connect(infoWidgetSuppressButton, &QAbstractButton::clicked, this, [this, id] { @@ -301,7 +301,7 @@ void InfoBarDisplay::update() }); } - if (info.cancelButtonText.isEmpty()) { + if (info.m_cancelButtonText.isEmpty()) { if (infoWidgetCloseButton) { infoWidgetCloseButton->setAutoRaise(true); infoWidgetCloseButton->setIcon(Utils::Icons::CLOSE_FOREGROUND.icon()); @@ -314,7 +314,7 @@ void InfoBarDisplay::update() if (infoWidgetCloseButton) hbox->addWidget(infoWidgetCloseButton); } else { - infoWidgetCloseButton->setText(info.cancelButtonText); + infoWidgetCloseButton->setText(info.m_cancelButtonText); hbox->addWidget(infoWidgetCloseButton); if (infoWidgetSuppressButton) hbox->addWidget(infoWidgetSuppressButton); diff --git a/src/plugins/coreplugin/infobar.h b/src/plugins/coreplugin/infobar.h index f3fbea6a024..ccd235cedc9 100644 --- a/src/plugins/coreplugin/infobar.h +++ b/src/plugins/coreplugin/infobar.h @@ -67,13 +67,13 @@ public: void setDetailsWidgetCreator(const DetailsWidgetCreator &creator); private: - Id id; - QString infoText; - QString buttonText; + Id m_id; + QString m_infoText; + QString m_buttonText; CallBack m_buttonCallBack; - QString cancelButtonText; + QString m_cancelButtonText; CallBack m_cancelButtonCallBack; - GlobalSuppressionMode globalSuppression; + GlobalSuppressionMode m_globalSuppression; DetailsWidgetCreator m_detailsWidgetCreator; bool m_useCancelButton = true; friend class InfoBar; From 113260c8d0b0ba264cc6b363b29d1101602a9842 Mon Sep 17 00:00:00 2001 From: Christian Stenger Date: Tue, 5 Feb 2019 14:06:07 +0100 Subject: [PATCH 11/19] AutoTest: Count blacklisted tests correctly Amends 0af0f58a60. Change-Id: I646b98fda5d1292a46a85e11a09506875e8a1606 Reviewed-by: Oliver Wolff --- src/plugins/autotest/testresultspane.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plugins/autotest/testresultspane.cpp b/src/plugins/autotest/testresultspane.cpp index 0c4b9eac652..c036ad431a3 100644 --- a/src/plugins/autotest/testresultspane.cpp +++ b/src/plugins/autotest/testresultspane.cpp @@ -471,7 +471,9 @@ void TestResultsPane::updateSummaryLabel() if (count) labelText += ", " + QString::number(count) + ' ' + tr("fatals"); count = m_model->resultTypeCount(Result::BlacklistedFail) - + m_model->resultTypeCount(Result::BlacklistedPass); + + m_model->resultTypeCount(Result::BlacklistedXFail) + + m_model->resultTypeCount(Result::BlacklistedPass) + + m_model->resultTypeCount(Result::BlacklistedXPass); if (count) labelText += ", " + QString::number(count) + ' ' + tr("blacklisted"); count = m_model->resultTypeCount(Result::Skip); From 4599d1ad4d47185a242e8909d61039265c290098 Mon Sep 17 00:00:00 2001 From: Ivan Donchevskii Date: Fri, 1 Feb 2019 12:24:24 +0100 Subject: [PATCH 12/19] Clang: Improve indexing performance on Windows Do not use delayed template parsing (it's too slow). Change-Id: I473c92394321920239fd90e6a37f0ce517f94e6a Reviewed-by: Marco Bubke --- .../source/collectmacrossourcefilecallbacks.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp index e8e77f2e231..d0d107e45e2 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp +++ b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp @@ -45,6 +45,7 @@ bool CollectMacrosSourceFileCallbacks::handleBeginSource(clang::CompilerInstance compilerInstance.getPreprocessorPtr(), m_sourcesManager); + compilerInstance.getLangOpts().DelayedTemplateParsing = false; compilerInstance.getPreprocessorPtr()->SetSuppressIncludeNotFoundError(true); compilerInstance.getPreprocessorPtr()->addPPCallbacks(std::move(callbacks)); From a0254ea7ee28ecf4ca9a5cd8ca5c9a75ff823482 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 6 Feb 2019 11:29:49 +0100 Subject: [PATCH 13/19] Kit settings page: Fix inconsistent labels The widget expected the KitInformation display names to have a colon at the end, which not all of them did. Instead, add the colon in the widget, because it's not really part of the name. Change-Id: I87d613031b462903bf4039eb7f8bdb99c15e37d6 Reviewed-by: Christian Stenger --- .../cmakeprojectmanager/cmakekitconfigwidget.cpp | 4 ++-- src/plugins/debugger/debuggerkitconfigwidget.cpp | 2 +- .../projectexplorer/kitinformationconfigwidget.cpp | 10 +++++----- src/plugins/projectexplorer/kitmanagerconfigwidget.cpp | 2 +- .../qmakeprojectmanager/qmakekitconfigwidget.cpp | 2 +- src/plugins/qtsupport/qtkitconfigwidget.cpp | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/plugins/cmakeprojectmanager/cmakekitconfigwidget.cpp b/src/plugins/cmakeprojectmanager/cmakekitconfigwidget.cpp index 01db2bd56c5..d264f1324f9 100644 --- a/src/plugins/cmakeprojectmanager/cmakekitconfigwidget.cpp +++ b/src/plugins/cmakeprojectmanager/cmakekitconfigwidget.cpp @@ -97,7 +97,7 @@ CMakeKitConfigWidget::~CMakeKitConfigWidget() QString CMakeKitConfigWidget::displayName() const { - return tr("CMake Tool:"); + return tr("CMake Tool"); } void CMakeKitConfigWidget::makeReadOnly() @@ -230,7 +230,7 @@ CMakeGeneratorKitConfigWidget::~CMakeGeneratorKitConfigWidget() QString CMakeGeneratorKitConfigWidget::displayName() const { - return tr("CMake generator:"); + return tr("CMake generator"); } void CMakeGeneratorKitConfigWidget::makeReadOnly() diff --git a/src/plugins/debugger/debuggerkitconfigwidget.cpp b/src/plugins/debugger/debuggerkitconfigwidget.cpp index 1486e7a08cf..d95a2861ac5 100644 --- a/src/plugins/debugger/debuggerkitconfigwidget.cpp +++ b/src/plugins/debugger/debuggerkitconfigwidget.cpp @@ -93,7 +93,7 @@ QString DebuggerKitConfigWidget::toolTip() const QString DebuggerKitConfigWidget::displayName() const { - return tr("Debugger:"); + return tr("Debugger"); } void DebuggerKitConfigWidget::makeReadOnly() diff --git a/src/plugins/projectexplorer/kitinformationconfigwidget.cpp b/src/plugins/projectexplorer/kitinformationconfigwidget.cpp index b1fcbcb6be4..a366d84646a 100644 --- a/src/plugins/projectexplorer/kitinformationconfigwidget.cpp +++ b/src/plugins/projectexplorer/kitinformationconfigwidget.cpp @@ -82,7 +82,7 @@ SysRootInformationConfigWidget::~SysRootInformationConfigWidget() QString SysRootInformationConfigWidget::displayName() const { - return tr("Sysroot:"); + return tr("Sysroot"); } QString SysRootInformationConfigWidget::toolTip() const @@ -177,7 +177,7 @@ ToolChainInformationConfigWidget::~ToolChainInformationConfigWidget() QString ToolChainInformationConfigWidget::displayName() const { - return tr("Compiler:"); + return tr("Compiler"); } QString ToolChainInformationConfigWidget::toolTip() const @@ -285,7 +285,7 @@ QWidget *DeviceTypeInformationConfigWidget::mainWidget() const QString DeviceTypeInformationConfigWidget::displayName() const { - return tr("Device type:"); + return tr("Device type"); } QString DeviceTypeInformationConfigWidget::toolTip() const @@ -358,7 +358,7 @@ QWidget *DeviceInformationConfigWidget::mainWidget() const QString DeviceInformationConfigWidget::displayName() const { - return tr("Device:"); + return tr("Device"); } QString DeviceInformationConfigWidget::toolTip() const @@ -437,7 +437,7 @@ QWidget *KitEnvironmentConfigWidget::mainWidget() const QString KitEnvironmentConfigWidget::displayName() const { - return tr("Environment:"); + return tr("Environment"); } QString KitEnvironmentConfigWidget::toolTip() const diff --git a/src/plugins/projectexplorer/kitmanagerconfigwidget.cpp b/src/plugins/projectexplorer/kitmanagerconfigwidget.cpp index c9b8391bd90..473c8455e25 100644 --- a/src/plugins/projectexplorer/kitmanagerconfigwidget.cpp +++ b/src/plugins/projectexplorer/kitmanagerconfigwidget.cpp @@ -216,7 +216,7 @@ void KitManagerConfigWidget::addConfigWidget(KitConfigWidget *widget) QTC_ASSERT(widget, return); QTC_ASSERT(!m_widgets.contains(widget), return); - QString name = widget->displayName(); + const QString name = widget->displayName() + ':'; QString toolTip = widget->toolTip(); auto action = new QAction(tr("Mark as Mutable"), nullptr); diff --git a/src/plugins/qmakeprojectmanager/qmakekitconfigwidget.cpp b/src/plugins/qmakeprojectmanager/qmakekitconfigwidget.cpp index 3cfcfaaa599..ba90baf6f16 100644 --- a/src/plugins/qmakeprojectmanager/qmakekitconfigwidget.cpp +++ b/src/plugins/qmakeprojectmanager/qmakekitconfigwidget.cpp @@ -55,7 +55,7 @@ QWidget *QmakeKitConfigWidget::mainWidget() const QString QmakeKitConfigWidget::displayName() const { - return tr("Qt mkspec:"); + return tr("Qt mkspec"); } QString QmakeKitConfigWidget::toolTip() const diff --git a/src/plugins/qtsupport/qtkitconfigwidget.cpp b/src/plugins/qtsupport/qtkitconfigwidget.cpp index 53c6421038c..8b32ba44ded 100644 --- a/src/plugins/qtsupport/qtkitconfigwidget.cpp +++ b/src/plugins/qtsupport/qtkitconfigwidget.cpp @@ -72,7 +72,7 @@ QtKitConfigWidget::~QtKitConfigWidget() QString QtKitConfigWidget::displayName() const { - return tr("Qt version:"); + return tr("Qt version"); } QString QtKitConfigWidget::toolTip() const From 14f66e0eb52da61cca253adb8168e012944f1c92 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Tue, 5 Feb 2019 15:17:35 +0100 Subject: [PATCH 14/19] Fix highlighting of regexp search results in editor The highlighting in the editor was still done with QRegExp, so if you used Perl regular expression features, highlighting in the editor was incorrect. Fixes: QTCREATORBUG-21940 Change-Id: I785f0b2413a291d9f06de5877b18067a30d58588 Reviewed-by: Marco Bubke --- src/plugins/texteditor/texteditor.cpp | 33 +++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/plugins/texteditor/texteditor.cpp b/src/plugins/texteditor/texteditor.cpp index 8b03581a403..e8190a60856 100644 --- a/src/plugins/texteditor/texteditor.cpp +++ b/src/plugins/texteditor/texteditor.cpp @@ -692,7 +692,8 @@ public: QTextCursor m_pendingLinkUpdate; QTextCursor m_lastLinkUpdate; - QRegExp m_searchExpr; + QRegularExpression m_searchExpr; + QString m_findText; FindFlags m_findFlags; void highlightSearchResults(const QTextBlock &block, const PaintEventData &data) const; QTimer m_delayedUpdateTimer; @@ -3663,7 +3664,7 @@ QTextBlock TextEditorWidgetPrivate::foldedBlockAt(const QPoint &pos, QRect *box) void TextEditorWidgetPrivate::highlightSearchResults(const QTextBlock &block, const PaintEventData &data) const { - if (m_searchExpr.isEmpty()) + if (m_searchExpr.pattern().isEmpty()) return; int blockPosition = block.position(); @@ -3682,10 +3683,11 @@ void TextEditorWidgetPrivate::highlightSearchResults(const QTextBlock &block, co .toTextCharFormat(C_SEARCH_RESULT).background().color().darker(120); while (idx < text.length()) { - idx = m_searchExpr.indexIn(text, idx + l); - if (idx < 0) + const QRegularExpressionMatch match = m_searchExpr.match(text, idx + 1); + if (!match.hasMatch()) break; - l = m_searchExpr.matchedLength(); + idx = match.capturedStart(); + l = match.capturedLength(); if (l == 0) break; if ((m_findFlags & FindWholeWords) @@ -4280,7 +4282,7 @@ void TextEditorWidgetPrivate::paintSearchResultOverlay(const PaintEventData &dat QPainter &painter) const { m_searchResultOverlay->clear(); - if (m_searchExpr.isEmpty()) + if (m_searchExpr.pattern().isEmpty()) return; const int margin = 5; @@ -5283,7 +5285,7 @@ void TextEditorWidgetPrivate::slotUpdateRequest(const QRect &r, int dy) m_extraArea->scroll(0, dy); } else if (r.width() > 4) { // wider than cursor width, not just cursor blinking m_extraArea->update(0, r.y(), m_extraArea->width(), r.height()); - if (!m_searchExpr.isEmpty()) { + if (!m_searchExpr.pattern().isEmpty()) { const int m = m_searchResultOverlay->dropShadowWidth(); q->viewport()->update(r.adjusted(-m, -m, m, m)); } @@ -6303,13 +6305,16 @@ void TextEditorWidgetPrivate::clearLink() void TextEditorWidgetPrivate::highlightSearchResultsSlot(const QString &txt, FindFlags findFlags) { - if (m_searchExpr.pattern() == txt) + const QString pattern = (findFlags & FindRegularExpression) ? txt + : QRegularExpression::escape(txt); + const QRegularExpression::PatternOptions options + = (findFlags & FindCaseSensitively) ? QRegularExpression::NoPatternOption + : QRegularExpression::CaseInsensitiveOption; + if (m_searchExpr.pattern() == pattern && m_searchExpr.patternOptions() == options) return; - m_searchExpr.setPattern(txt); - m_searchExpr.setPatternSyntax((findFlags & FindRegularExpression) ? - QRegExp::RegExp : QRegExp::FixedString); - m_searchExpr.setCaseSensitivity((findFlags & FindCaseSensitively) ? - Qt::CaseSensitive : Qt::CaseInsensitive); + m_searchExpr.setPattern(pattern); + m_searchExpr.setPatternOptions(options); + m_findText = txt; m_findFlags = findFlags; m_delayedUpdateTimer.start(50); @@ -6367,7 +6372,7 @@ void TextEditorWidgetPrivate::highlightSearchResultsInScrollBar() m_searchWatcher = nullptr; } - const QString &txt = m_searchExpr.pattern(); + const QString &txt = m_findText; if (txt.isEmpty()) return; From f973b4e94af1cfda7f15ddb4a006d5b5ce522667 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Mon, 4 Feb 2019 16:13:59 +0100 Subject: [PATCH 15/19] Help: Move finding the best documentation link to HelpItem Change-Id: I5032d380295d942cd4f334e4bf47e349dd5b0aea Reviewed-by: Jarek Kobus --- src/plugins/coreplugin/helpitem.cpp | 31 +++++++++++++++++++++++++++++ src/plugins/coreplugin/helpitem.h | 1 + src/plugins/help/helpplugin.cpp | 28 +------------------------- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/src/plugins/coreplugin/helpitem.cpp b/src/plugins/coreplugin/helpitem.cpp index ec5ead12e07..268c00ce67b 100644 --- a/src/plugins/coreplugin/helpitem.cpp +++ b/src/plugins/coreplugin/helpitem.cpp @@ -173,3 +173,34 @@ const QMap &HelpItem::links() const } return *m_helpLinks; } + +static QUrl findBestLink(const QMap &links) +{ + if (links.isEmpty()) + return QUrl(); + if (links.size() == 1) + return links.first(); + QUrl source = links.first(); + // workaround to show the latest Qt version + int version = 0; + QRegExp exp("(\\d+)"); + foreach (const QUrl &link, links) { + const QString &authority = link.authority(); + if (authority.startsWith("com.trolltech.") + || authority.startsWith("org.qt-project.")) { + if (exp.indexIn(authority) >= 0) { + const int tmpVersion = exp.cap(1).toInt(); + if (tmpVersion > version) { + source = link; + version = tmpVersion; + } + } + } + } + return source; +} + +const QUrl HelpItem::bestLink() const +{ + return findBestLink(links()); +} diff --git a/src/plugins/coreplugin/helpitem.h b/src/plugins/coreplugin/helpitem.h index 794eaca8f31..5468659b2a4 100644 --- a/src/plugins/coreplugin/helpitem.h +++ b/src/plugins/coreplugin/helpitem.h @@ -78,6 +78,7 @@ public: QString extractContent(bool extended) const; const QMap &links() const; + const QUrl bestLink() const; private: QUrl m_helpUrl; diff --git a/src/plugins/help/helpplugin.cpp b/src/plugins/help/helpplugin.cpp index df16af3101c..ada46f3b3ec 100644 --- a/src/plugins/help/helpplugin.cpp +++ b/src/plugins/help/helpplugin.cpp @@ -620,30 +620,6 @@ HelpViewer *HelpPluginPrivate::viewerForContextHelp() return viewerForHelpViewerLocation(LocalHelpManager::contextHelpOption()); } -static QUrl findBestLink(const QMap &links) -{ - if (links.isEmpty()) - return QUrl(); - QUrl source = links.constBegin().value(); - // workaround to show the latest Qt version - int version = 0; - QRegExp exp("(\\d+)"); - foreach (const QUrl &link, links) { - const QString &authority = link.authority(); - if (authority.startsWith("com.trolltech.") - || authority.startsWith("org.qt-project.")) { - if (exp.indexIn(authority) >= 0) { - const int tmpVersion = exp.cap(1).toInt(); - if (tmpVersion > version) { - source = link; - version = tmpVersion; - } - } - } - } - return source; -} - void HelpPluginPrivate::requestContextHelp() { // Find out what to show @@ -660,9 +636,7 @@ void HelpPluginPrivate::requestContextHelp() void HelpPluginPrivate::showContextHelp(const HelpItem &contextHelp) { - const QMap &links = contextHelp.links(); - - const QUrl source = findBestLink(links); + const QUrl source = contextHelp.bestLink(); if (!source.isValid()) { // No link found or no context object HelpViewer *viewer = showHelpUrl(QUrl(Help::Constants::AboutBlank), From 7d64c9e3ec9988bc0f88cd847677174c3a974aad Mon Sep 17 00:00:00 2001 From: Ivan Donchevskii Date: Wed, 30 Jan 2019 09:07:45 +0100 Subject: [PATCH 16/19] Clang: Fix some issues with running indexer over Qt Creator - fix qDebug channels on Windows - fix the number of perameters in the sql statement - fix nullptr access - speed up preprocessor a little bit Change-Id: Ic9b32fbcc6b409c4064c4f522b94391cbff8654e Reviewed-by: Marco Bubke --- src/libs/clangsupport/commandlinebuilder.h | 2 +- src/libs/clangsupport/projectpartcontainer.h | 3 ++- .../clangpchmanagerbackendmain.cpp | 13 +++++++++++ .../source/collectbuilddependencyaction.h | 1 + .../clangrefactoringbackendmain.cpp | 13 +++++++++++ .../source/clangtool.h | 2 -- .../source/symbolstorage.h | 6 +++-- .../source/symbolsvisitorbase.h | 3 ++- .../builddependencycollector-test.cpp | 5 ----- .../unit/unittest/commandlinebuilder-test.cpp | 22 +++++++++---------- tests/unit/unittest/pchcreator-test.cpp | 4 ++-- tests/unit/unittest/symbolindexer-test.cpp | 16 +++++++------- 12 files changed, 57 insertions(+), 33 deletions(-) diff --git a/src/libs/clangsupport/commandlinebuilder.h b/src/libs/clangsupport/commandlinebuilder.h index df116197ab1..9b5bced9328 100644 --- a/src/libs/clangsupport/commandlinebuilder.h +++ b/src/libs/clangsupport/commandlinebuilder.h @@ -270,7 +270,7 @@ public: void addNoStdIncAndNoStdLibInc() { commandLine.emplace_back("-nostdinc"); - commandLine.emplace_back("-nostdlibinc"); + commandLine.emplace_back("-nostdinc++"); } public: diff --git a/src/libs/clangsupport/projectpartcontainer.h b/src/libs/clangsupport/projectpartcontainer.h index 41ae071961d..06065911372 100644 --- a/src/libs/clangsupport/projectpartcontainer.h +++ b/src/libs/clangsupport/projectpartcontainer.h @@ -161,5 +161,6 @@ public: using ProjectPartContainers = std::vector; -QDebug operator<<(QDebug debug, const ProjectPartContainer &container); +CLANGSUPPORT_EXPORT QDebug operator<<(QDebug debug, const ProjectPartContainer &container); + } // namespace ClangBackEnd diff --git a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp index 5c38a3d8413..c4e8e085ce2 100644 --- a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp +++ b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp @@ -55,6 +55,7 @@ #include #include #include +#include #include using namespace std::chrono_literals; @@ -214,8 +215,20 @@ struct Data // because we have a cycle dependency ClangBackEnd::CallDoInMainThreadAfterFinished::Yes}; }; +#ifdef Q_OS_WIN +static void messageOutput(QtMsgType type, const QMessageLogContext &, const QString &msg) +{ + std::wcout << msg.toStdWString() << std::endl; + if (type == QtFatalMsg) + abort(); +} +#endif + int main(int argc, char *argv[]) { +#ifdef Q_OS_WIN + qInstallMessageHandler(messageOutput); +#endif try { QCoreApplication::setOrganizationName(QStringLiteral("QtProject")); QCoreApplication::setOrganizationDomain(QStringLiteral("qt-project.org")); diff --git a/src/tools/clangpchmanagerbackend/source/collectbuilddependencyaction.h b/src/tools/clangpchmanagerbackend/source/collectbuilddependencyaction.h index ff861f174f0..03d27f9b21e 100644 --- a/src/tools/clangpchmanagerbackend/source/collectbuilddependencyaction.h +++ b/src/tools/clangpchmanagerbackend/source/collectbuilddependencyaction.h @@ -58,6 +58,7 @@ public: auto &preprocessor = compilerInstance.getPreprocessor(); preprocessor.SetSuppressIncludeNotFoundError(true); + preprocessor.SetMacroExpansionOnlyInDirectives(); auto macroPreprocessorCallbacks = new CollectBuildDependencyPreprocessorCallbacks( m_buildDependency, diff --git a/src/tools/clangrefactoringbackend/clangrefactoringbackendmain.cpp b/src/tools/clangrefactoringbackend/clangrefactoringbackendmain.cpp index 0d483bbeaf1..23a127c17f2 100644 --- a/src/tools/clangrefactoringbackend/clangrefactoringbackendmain.cpp +++ b/src/tools/clangrefactoringbackend/clangrefactoringbackendmain.cpp @@ -39,6 +39,7 @@ #include #include +#include using namespace std::chrono_literals; @@ -97,8 +98,20 @@ struct Data // because we have a cycle dependency SymbolIndexing symbolIndexing{database, filePathCache, generatedFiles, [&] (int progress, int total) { clangCodeModelServer.setProgress(progress, total); }}; }; +#ifdef Q_OS_WIN +static void messageOutput(QtMsgType type, const QMessageLogContext &, const QString &msg) +{ + std::wcout << msg.toStdWString() << std::endl; + if (type == QtFatalMsg) + abort(); +} +#endif + int main(int argc, char *argv[]) { +#ifdef Q_OS_WIN + qInstallMessageHandler(messageOutput); +#endif try { QCoreApplication::setOrganizationName(QStringLiteral("QtProject")); QCoreApplication::setOrganizationDomain(QStringLiteral("qt-project.org")); diff --git a/src/tools/clangrefactoringbackend/source/clangtool.h b/src/tools/clangrefactoringbackend/source/clangtool.h index b9d366859e3..8b4e47df682 100644 --- a/src/tools/clangrefactoringbackend/source/clangtool.h +++ b/src/tools/clangrefactoringbackend/source/clangtool.h @@ -95,8 +95,6 @@ public: bool isClean() const; - - private: RefactoringCompilationDatabase m_compilationDatabase; std::vector m_fileContents; diff --git a/src/tools/clangrefactoringbackend/source/symbolstorage.h b/src/tools/clangrefactoringbackend/source/symbolstorage.h index f2993f9a43c..f1fdbc7c02e 100644 --- a/src/tools/clangrefactoringbackend/source/symbolstorage.h +++ b/src/tools/clangrefactoringbackend/source/symbolstorage.h @@ -341,12 +341,14 @@ public: m_database}; mutable ReadStatement m_getProjectPartArtefactsBySourceId{ "SELECT toolChainArguments, compilerMacros, systemIncludeSearchPaths, " - "projectIncludeSearchPaths, projectPartId FROM projectParts WHERE projectPartId = (SELECT " + "projectIncludeSearchPaths, projectPartId, language, languageVersion, languageExtension " + "FROM projectParts WHERE projectPartId = (SELECT " "projectPartId FROM projectPartsSources WHERE sourceId = ?)", m_database}; mutable ReadStatement m_getProjectPartArtefactsByProjectPartName{ "SELECT toolChainArguments, compilerMacros, systemIncludeSearchPaths, " - "projectIncludeSearchPaths, projectPartId FROM projectParts WHERE projectPartName = ?", + "projectIncludeSearchPaths, projectPartId, language, languageVersion, languageExtension " + "FROM projectParts WHERE projectPartName = ?", m_database}; mutable ReadStatement m_getPrecompiledHeader{ "SELECT projectPchPath, projectPchBuildTime FROM precompiledHeaders WHERE projectPartId = ?", diff --git a/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h b/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h index c833f73afbe..ff2142f40e2 100644 --- a/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h +++ b/src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h @@ -68,7 +68,8 @@ public: bool isAlreadyParsed(clang::FileID fileId) { const clang::FileEntry *fileEntry = m_sourceManager->getFileEntryForID(fileId); - + if (!fileEntry) + return false; return m_sourcesManager.alreadyParsed(filePathId(fileEntry), fileEntry->getModificationTime()); } diff --git a/tests/unit/unittest/builddependencycollector-test.cpp b/tests/unit/unittest/builddependencycollector-test.cpp index 4631a883766..3c9ea84d32b 100644 --- a/tests/unit/unittest/builddependencycollector-test.cpp +++ b/tests/unit/unittest/builddependencycollector-test.cpp @@ -435,9 +435,7 @@ TEST_F(BuildDependencyCollector, CollectUsedMacrosWithExternalDefine) ElementsAre(Eq(UsedMacro{"DEFINED", fileId}), Eq(UsedMacro{"IF_DEFINE", fileId}), Eq(UsedMacro{"__clang__", fileId}), - Eq(UsedMacro{"CLASS_EXPORT", fileId}), Eq(UsedMacro{"IF_NOT_DEFINE", fileId}), - Eq(UsedMacro{"MACRO_EXPANSION", fileId}), Eq(UsedMacro{"COMPILER_ARGUMENT", fileId}))); } @@ -452,9 +450,7 @@ TEST_F(BuildDependencyCollector, CollectUsedMacrosWithoutExternalDefine) ElementsAre(Eq(UsedMacro{"DEFINED", fileId}), Eq(UsedMacro{"IF_DEFINE", fileId}), Eq(UsedMacro{"__clang__", fileId}), - Eq(UsedMacro{"CLASS_EXPORT", fileId}), Eq(UsedMacro{"IF_NOT_DEFINE", fileId}), - Eq(UsedMacro{"MACRO_EXPANSION", fileId}), Eq(UsedMacro{"COMPILER_ARGUMENT", fileId}))); } @@ -639,7 +635,6 @@ TEST_F(BuildDependencyCollector, Create) SourceType::TopProjectInclude))), Field(&BuildDependency::usedMacros, UnorderedElementsAre( - UsedMacro{"DEFINE", id(TESTDATA_DIR "/builddependencycollector/project/macros.h")}, UsedMacro{"IFDEF", id(TESTDATA_DIR "/builddependencycollector/project/macros.h")}, UsedMacro{"DEFINED", id(TESTDATA_DIR "/builddependencycollector/project/macros.h")})), diff --git a/tests/unit/unittest/commandlinebuilder-test.cpp b/tests/unit/unittest/commandlinebuilder-test.cpp index c3b60df3694..c3dad70de3c 100644 --- a/tests/unit/unittest/commandlinebuilder-test.cpp +++ b/tests/unit/unittest/commandlinebuilder-test.cpp @@ -136,7 +136,7 @@ TYPED_TEST(CommandLineBuilder, CTask) ASSERT_THAT( builder.commandLine, - ElementsAre("clang", "-x", "c-header", "-std=c11", "-nostdinc", "-nostdlibinc", "/source/file.c")); + ElementsAre("clang", "-x", "c-header", "-std=c11", "-nostdinc", "-nostdinc++", "/source/file.c")); } TYPED_TEST(CommandLineBuilder, ObjectiveCTask) @@ -153,7 +153,7 @@ TYPED_TEST(CommandLineBuilder, ObjectiveCTask) "objective-c-header", "-std=c11", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "/source/file.c")); } @@ -170,7 +170,7 @@ TYPED_TEST(CommandLineBuilder, CppTask) "c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "/source/file.cpp")); } @@ -188,7 +188,7 @@ TYPED_TEST(CommandLineBuilder, ObjectiveCppTask) "objective-c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "/source/file.cpp")); } @@ -420,7 +420,7 @@ TYPED_TEST(CommandLineBuilder, IncludesOrder) "c++-header", "-std=c++11", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-I", "/include/foo", "-I", @@ -441,7 +441,7 @@ TYPED_TEST(CommandLineBuilder, EmptySourceFile) Builder builder{this->emptyProjectInfo, {}, {}}; ASSERT_THAT(builder.commandLine, - ElementsAre("clang++", "-x", "c++-header", "-std=c++98", "-nostdinc", "-nostdlibinc")); + ElementsAre("clang++", "-x", "c++-header", "-std=c++98", "-nostdinc", "-nostdinc++")); } TYPED_TEST(CommandLineBuilder, SourceFile) @@ -454,7 +454,7 @@ TYPED_TEST(CommandLineBuilder, SourceFile) "c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "/source/file.cpp")); } @@ -469,7 +469,7 @@ TYPED_TEST(CommandLineBuilder, EmptyOutputFile) "c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "/source/file.cpp")); } @@ -483,7 +483,7 @@ TYPED_TEST(CommandLineBuilder, OutputFile) "c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-o", "/output/file.o", "/source/file.cpp")); @@ -499,7 +499,7 @@ TYPED_TEST(CommandLineBuilder, IncludePchPath) "c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-Xclang", "-include-pch", "-Xclang", @@ -521,7 +521,7 @@ TYPED_TEST(CommandLineBuilder, CompilerMacros) "c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DER=2", "-DYI=1")); } diff --git a/tests/unit/unittest/pchcreator-test.cpp b/tests/unit/unittest/pchcreator-test.cpp index 0da712d9836..26ca9965c2f 100644 --- a/tests/unit/unittest/pchcreator-test.cpp +++ b/tests/unit/unittest/pchcreator-test.cpp @@ -157,7 +157,7 @@ TEST_F(PchCreator, CreateProjectPartClangCompilerArguments) "c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-I", TESTDATA_DIR "/builddependencycollector/project", "-isystem", @@ -183,7 +183,7 @@ TEST_F(PchCreator, CreateProjectPartClangCompilerArgumentsWithSystemPch) "c++-header", "-std=c++98", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-I", TESTDATA_DIR "/builddependencycollector/project", "-isystem", diff --git a/tests/unit/unittest/symbolindexer-test.cpp b/tests/unit/unittest/symbolindexer-test.cpp index 4f112d45298..104c17b11b3 100644 --- a/tests/unit/unittest/symbolindexer-test.cpp +++ b/tests/unit/unittest/symbolindexer-test.cpp @@ -269,7 +269,7 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsAddFilesInCollector) "c++-header", "-std=c++14", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DBAR=1", "-DFOO=1", "-I", @@ -299,7 +299,7 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsAddFilesWithPrecompiledHeaderInColl "c++-header", "-std=c++14", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DBAR=1", "-DFOO=1", "-I", @@ -332,7 +332,7 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsAddFilesWithoutPrecompiledHeaderInC "c++-header", "-std=c++14", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DBAR=1", "-DFOO=1", "-I", @@ -512,7 +512,7 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsInOrderWithoutProjectPartArtifact) "c++-header", "-std=c++14", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DBAR=1", "-DFOO=1", "-I", @@ -564,7 +564,7 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsInOrderWithProjectPartArtifact) "c++-header", "-std=c++14", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DBAR=1", "-DFOO=1", "-I", @@ -620,7 +620,7 @@ TEST_F(SymbolIndexer, UpdateChangedPathCallsInOrder) "c++-header", "-std=c++14", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DBAR=1", "-DFOO=1", "-I", @@ -682,7 +682,7 @@ TEST_F(SymbolIndexer, UpdateChangedPathIsUsingPrecompiledHeader) "c++-header", "-std=c++14", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DBAR=1", "-DFOO=1", "-I", @@ -717,7 +717,7 @@ TEST_F(SymbolIndexer, UpdateChangedPathIsNotUsingPrecompiledHeaderIfItNotExists) "c++-header", "-std=c++14", "-nostdinc", - "-nostdlibinc", + "-nostdinc++", "-DBAR=1", "-DFOO=1", "-I", From a60cd0ba30b8c66f1f34d2e79365f94b4e332cf6 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Wed, 6 Feb 2019 09:34:00 +0100 Subject: [PATCH 17/19] ClangTools: Remove "in file included from" trace for header diagnostics ...as this does not add much value but rather noise. Change-Id: I0bb8807c3f4df4114ea135bea74cde3fd21ab090 Reviewed-by: Ivan Donchevskii --- src/plugins/clangtools/clangtoolslogfilereader.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/plugins/clangtools/clangtoolslogfilereader.cpp b/src/plugins/clangtools/clangtoolslogfilereader.cpp index d1e98fac87b..d2213d3304e 100644 --- a/src/plugins/clangtools/clangtoolslogfilereader.cpp +++ b/src/plugins/clangtools/clangtoolslogfilereader.cpp @@ -25,6 +25,8 @@ #include "clangtoolslogfilereader.h" +#include + #include #include #include @@ -147,6 +149,10 @@ static Diagnostic buildDiagnostic(const CXDiagnostic cxDiagnostic, clang_disposeDiagnosticSet(cxChildDiagnostics); }); + using CppTools::ProjectFile; + const bool isHeaderFile = ProjectFile::isHeader( + ProjectFile::classify(diagnostic.location.filePath)); + for (unsigned i = 0; i < clang_getNumDiagnosticsInSet(cxChildDiagnostics); ++i) { CXDiagnostic cxDiagnostic = clang_getDiagnosticInSet(cxChildDiagnostics, i); Utils::ExecuteOnDestruction cleanUpDiagnostic([&]() { @@ -156,6 +162,9 @@ static Diagnostic buildDiagnostic(const CXDiagnostic cxDiagnostic, if (diagnosticStep.isValid()) continue; + if (isHeaderFile && diagnosticStep.message.contains("in file included from")) + continue; + if (isInvalidDiagnosticLocation(diagnostic, diagnosticStep, nativeFilePath)) return diagnostic; From 9cca455fc760dbcfc5edbbe69f0d751bbdea4ed5 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Wed, 6 Feb 2019 10:01:01 +0100 Subject: [PATCH 18/19] ClangTools: Generalize some variables names Change-Id: I81f9fa2c2c0d91c78050db0004114d22779550cf Reviewed-by: Ivan Donchevskii --- .../clangtools/clangtoolsdiagnosticview.cpp | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/plugins/clangtools/clangtoolsdiagnosticview.cpp b/src/plugins/clangtools/clangtoolsdiagnosticview.cpp index b10d26dd328..7469aa618b1 100644 --- a/src/plugins/clangtools/clangtoolsdiagnosticview.cpp +++ b/src/plugins/clangtools/clangtoolsdiagnosticview.cpp @@ -154,32 +154,32 @@ QModelIndex DiagnosticView::getIndex(const QModelIndex &index, Direction directi { QModelIndex parentIndex = index.parent(); - // Use direct sibling for level 2 and 3 items is possible + // Use direct sibling for level 2 and 3 items if possible if (parentIndex.isValid()) { - const QModelIndex nextIndex = index.sibling(index.row() + direction, index.column()); - if (nextIndex.isValid()) - return nextIndex; + const QModelIndex followingIndex = index.sibling(index.row() + direction, index.column()); + if (followingIndex.isValid()) + return followingIndex; } // Last level 3 item? Continue on level 2 item if (parentIndex.parent().isValid()) return getIndex(parentIndex, direction); - // Find next/previous level 2 item - QModelIndex nextTopIndex = getTopLevelIndex(parentIndex.isValid() ? parentIndex : index, - direction); - while (!model()->hasChildren(nextTopIndex)) - nextTopIndex = getTopLevelIndex(nextTopIndex, direction); - return model()->index(direction == Next ? 0 : model()->rowCount(nextTopIndex) - 1, + // Find following level 2 item + QModelIndex followingTopIndex = getTopLevelIndex(parentIndex.isValid() ? parentIndex : index, + direction); + while (!model()->hasChildren(followingTopIndex)) + followingTopIndex = getTopLevelIndex(followingTopIndex, direction); + return model()->index(direction == Next ? 0 : model()->rowCount(followingTopIndex) - 1, 0, - nextTopIndex); + followingTopIndex); } QModelIndex DiagnosticView::getTopLevelIndex(const QModelIndex &index, Direction direction) const { - QModelIndex below = index.sibling(index.row() + direction, 0); - if (below.isValid()) - return below; + QModelIndex following = index.sibling(index.row() + direction, 0); + if (following.isValid()) + return following; return model()->index(direction == Next ? 0 : model()->rowCount(index) - 1, 0); } From 0f40b1fb5c3597a265400eefd94a51890077bbb3 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Wed, 6 Feb 2019 10:10:36 +0100 Subject: [PATCH 19/19] ClangTools: Fix "Go to next/previous diagnostic" action ...for some corner cases: (1) Prev: If the very first diagnostic is selected, select the very last diagnostic instead of doing nothing. (2) Prev: If the first child diagnostic is selected, select the main diagnostic and not the previous main diagnostic. (3) Next: In case a file path item is selected, select the very first diagnostic for that file path and not the next file path. Change-Id: If5ec573911a92c5cfcc1f49ef26ed4dcf82d034b Reviewed-by: Ivan Donchevskii --- .../clangtools/clangtoolsdiagnosticview.cpp | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/plugins/clangtools/clangtoolsdiagnosticview.cpp b/src/plugins/clangtools/clangtoolsdiagnosticview.cpp index 7469aa618b1..07bc7f397f4 100644 --- a/src/plugins/clangtools/clangtoolsdiagnosticview.cpp +++ b/src/plugins/clangtools/clangtoolsdiagnosticview.cpp @@ -152,27 +152,29 @@ void DiagnosticView::goBack() QModelIndex DiagnosticView::getIndex(const QModelIndex &index, Direction direction) const { - QModelIndex parentIndex = index.parent(); + const QModelIndex parentIndex = index.parent(); + QModelIndex followingTopIndex = index; - // Use direct sibling for level 2 and 3 items if possible if (parentIndex.isValid()) { - const QModelIndex followingIndex = index.sibling(index.row() + direction, index.column()); + // Use direct sibling for level 2 and 3 items + const QModelIndex followingIndex = index.sibling(index.row() + direction, 0); if (followingIndex.isValid()) return followingIndex; + + // First/Last level 3 item? Continue on level 2 item. + if (parentIndex.parent().isValid()) + return direction == Previous ? parentIndex : getIndex(parentIndex, direction); + + followingTopIndex = getTopLevelIndex(parentIndex, direction); } - // Last level 3 item? Continue on level 2 item - if (parentIndex.parent().isValid()) - return getIndex(parentIndex, direction); - - // Find following level 2 item - QModelIndex followingTopIndex = getTopLevelIndex(parentIndex.isValid() ? parentIndex : index, - direction); + // Skip top level items without children while (!model()->hasChildren(followingTopIndex)) followingTopIndex = getTopLevelIndex(followingTopIndex, direction); - return model()->index(direction == Next ? 0 : model()->rowCount(followingTopIndex) - 1, - 0, - followingTopIndex); + + // Select first/last level 2 item + const int row = direction == Next ? 0 : model()->rowCount(followingTopIndex) - 1; + return model()->index(row, 0, followingTopIndex); } QModelIndex DiagnosticView::getTopLevelIndex(const QModelIndex &index, Direction direction) const @@ -180,7 +182,8 @@ QModelIndex DiagnosticView::getTopLevelIndex(const QModelIndex &index, Direction QModelIndex following = index.sibling(index.row() + direction, 0); if (following.isValid()) return following; - return model()->index(direction == Next ? 0 : model()->rowCount(index) - 1, 0); + const int row = direction == Next ? 0 : model()->rowCount() - 1; + return model()->index(row, 0); } QList DiagnosticView::customActions() const