Commit 23cae9a9 authored by Tim Jenssen's avatar Tim Jenssen

Clang: Filter duplicates in clang query output

Because we can visit headers many times, we get results many times too.

Change-Id: I3bbe7d7a5d01c2580a4569bfe115f14a69edc8a7
Reviewed-by: Tim Jenssen's avatarTim Jenssen <tim.jenssen@qt.io>
parent 07c3a370
......@@ -47,6 +47,11 @@ public:
return sourceRangesContainer;
}
SourceRangesContainer &sourceRanges()
{
return sourceRangesContainer;
}
const std::vector<DynamicASTMatcherDiagnosticContainer> &diagnostics() const
{
return diagnosticContainers;
......
......@@ -54,6 +54,11 @@ public:
return m_sourceRangeWithTextContainers;
}
std::vector<SourceRangeWithTextContainer> &sourceRangeWithTextContainers()
{
return m_sourceRangeWithTextContainers;
}
std::vector<SourceRangeWithTextContainer> takeSourceRangeWithTextContainers()
{
return std::move(m_sourceRangeWithTextContainers);
......
......@@ -115,3 +115,20 @@ using SourceRangeWithTextContainers = std::vector<SourceRangeWithTextContainer>;
CMBIPC_EXPORT QDebug operator<<(QDebug debug, const SourceRangeWithTextContainer &container);
std::ostream &operator<<(std::ostream &os, const SourceRangeWithTextContainer &container);
} // namespace ClangBackEnd
namespace std
{
template<> struct hash<ClangBackEnd::SourceRangeWithTextContainer>
{
using argument_type = ClangBackEnd::SourceRangeWithTextContainer;
using result_type = std::size_t;
result_type operator()(const argument_type &container) const
{
const result_type h1{std::hash<uint>{}(container.fileHash())};
const result_type h2{std::hash<uint>{}(container.start().offset())};
const result_type h3{std::hash<uint>{}(container.end().offset())};
return h1 ^ (h2 << 8) ^ (h3 << 16);
}
};
}
......@@ -34,6 +34,7 @@ ClangQueryGatherer::ClangQueryGatherer(StringCache<Utils::PathString, std::mutex
std::vector<V2::FileContainer> &&unsaved,
Utils::SmallString &&query)
: m_filePathCache(filePathCache),
m_sourceRangeFilter(sources.size()),
m_sources(std::move(sources)),
m_unsaved(std::move(unsaved)),
m_query(std::move(query))
......@@ -125,7 +126,7 @@ std::vector<SourceRangesAndDiagnosticsForQueryMessage> ClangQueryGatherer::allCu
std::vector<SourceRangesAndDiagnosticsForQueryMessage> messages;
for (Future &future : m_sourceFutures)
messages.push_back(future.get());
messages.push_back(m_sourceRangeFilter.removeDuplicates(future.get()));
return messages;
}
......@@ -135,7 +136,7 @@ std::vector<SourceRangesAndDiagnosticsForQueryMessage> ClangQueryGatherer::finis
std::vector<SourceRangesAndDiagnosticsForQueryMessage> messages;
for (auto &&future : finishedFutures())
messages.push_back(future.get());
messages.push_back(m_sourceRangeFilter.removeDuplicates(future.get()));
return messages;
}
......
......@@ -25,6 +25,8 @@
#pragma once
#include "sourcerangefilter.h"
#include <sourcerangesanddiagnosticsforquerymessage.h>
#include <filecontainerv2.h>
#include <stringcache.h>
......@@ -68,6 +70,7 @@ protected:
private:
StringCache<Utils::PathString, std::mutex> *m_filePathCache = nullptr;
SourceRangeFilter m_sourceRangeFilter;
std::vector<V2::FileContainer> m_sources;
std::vector<V2::FileContainer> m_unsaved;
Utils::SmallString m_query;
......
INCLUDEPATH += $$PWD
HEADERS += \
$$PWD/clangrefactoringbackend_global.h
$$PWD/clangrefactoringbackend_global.h \
$$PWD/sourcerangefilter.h
!isEmpty(LIBTOOLING_LIBS) {
SOURCES += \
......@@ -33,3 +34,6 @@ HEADERS += \
$$PWD/locationsourcefilecallbacks.h \
$$PWD/clangquerygatherer.h
}
SOURCES += \
$$PWD/sourcerangefilter.cpp
......@@ -115,6 +115,11 @@ bool RefactoringServer::pollTimerIsActive() const
return m_pollTimer.isActive();
}
void RefactoringServer::setGathererProcessingSlotCount(uint count)
{
m_gatherer.setProcessingSlotCount(count);
}
void RefactoringServer::gatherSourceRangesAndDiagnosticsForQueryMessages(
std::vector<V2::FileContainer> &&sources,
std::vector<V2::FileContainer> &&unsaved,
......
......@@ -64,6 +64,8 @@ public:
bool pollTimerIsActive() const;
void setGathererProcessingSlotCount(uint count);
private:
void gatherSourceRangesAndDiagnosticsForQueryMessages(std::vector<V2::FileContainer> &&sources,
std::vector<V2::FileContainer> &&unsaved,
......
/****************************************************************************
**
** Copyright (C) 2017 The Qt Company Ltd.
** Contact: https://www.qt.io/licensing/
**
** This file is part of Qt Creator.
**
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and The Qt Company. For licensing terms
** and conditions see https://www.qt.io/terms-conditions. For further
** information use the contact form at https://www.qt.io/contact-us.
**
** GNU General Public License Usage
** Alternatively, this file may be used under the terms of the GNU
** General Public License version 3 as published by the Free Software
** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
** included in the packaging of this file. Please review the following
** information to ensure the GNU General Public License requirements will
** be met: https://www.gnu.org/licenses/gpl-3.0.html.
**
****************************************************************************/
#include "sourcerangefilter.h"
#include <algorithm>
namespace ClangBackEnd {
SourceRangeFilter::SourceRangeFilter(std::size_t sourcesCount)
{
m_collectedSourceRanges.reserve(sourcesCount);
}
SourceRangesAndDiagnosticsForQueryMessage SourceRangeFilter::removeDuplicates(SourceRangesAndDiagnosticsForQueryMessage &&message)
{
removeDuplicates(message.sourceRanges().sourceRangeWithTextContainers());
return std::move(message);
}
void SourceRangeFilter::removeDuplicates(SourceRangeWithTextContainers &sourceRanges)
{
auto partitionPoint = std::stable_partition(sourceRanges.begin(),
sourceRanges.end(),
[&] (const SourceRangeWithTextContainer &sourceRange) {
return m_collectedSourceRanges.find(sourceRange) == m_collectedSourceRanges.end();
});
sourceRanges.erase(partitionPoint, sourceRanges.end());
std::copy(sourceRanges.begin(),
sourceRanges.end(),
std::inserter(m_collectedSourceRanges, m_collectedSourceRanges.end()));
}
} // namespace ClangBackEnd
/****************************************************************************
**
** Copyright (C) 2017 The Qt Company Ltd.
** Contact: https://www.qt.io/licensing/
**
** This file is part of Qt Creator.
**
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and The Qt Company. For licensing terms
** and conditions see https://www.qt.io/terms-conditions. For further
** information use the contact form at https://www.qt.io/contact-us.
**
** GNU General Public License Usage
** Alternatively, this file may be used under the terms of the GNU
** General Public License version 3 as published by the Free Software
** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
** included in the packaging of this file. Please review the following
** information to ensure the GNU General Public License requirements will
** be met: https://www.gnu.org/licenses/gpl-3.0.html.
**
****************************************************************************/
#pragma once
#include <unordered_set>
#include <sourcerangesanddiagnosticsforquerymessage.h>
namespace ClangBackEnd {
class SourceRangeFilter
{
public:
SourceRangeFilter(std::size_t sourcesCount = 0);
SourceRangesAndDiagnosticsForQueryMessage
removeDuplicates(SourceRangesAndDiagnosticsForQueryMessage &&message);
void removeDuplicates(SourceRangeWithTextContainers &sourceRanges);
private:
std::unordered_set<SourceRangeWithTextContainer> m_collectedSourceRanges;
};
} // namespace ClangBackEnd
......@@ -38,6 +38,7 @@ using testing::AtLeast;
using testing::AtMost;
using testing::Contains;
using testing::Each;
using testing::ElementsAre;
using testing::Eq;
using testing::Ge;
using testing::IsEmpty;
......@@ -47,6 +48,7 @@ using testing::Pair;
using testing::PrintToString;
using testing::Property;
using testing::SizeIs;
using testing::UnorderedElementsAre;
using testing::_;
using ClangBackEnd::V2::FileContainer;
......@@ -72,10 +74,16 @@ protected:
protected:
ClangBackEnd::StringCache<Utils::PathString, std::mutex> filePathCache;
Utils::SmallString sourceContent{"#include \"query_simplefunction.h\"\nvoid f()\n {}"};
Utils::SmallString sourceContent{"#include \"query_simplefunction.h\"\nvoid f() {}"};
FileContainer source{{TESTDATA_DIR, "query_simplefunction.cpp"},
sourceContent.clone(),
{"cc", "query_simplefunction.cpp"}};
{"cc", TESTDATA_DIR"/query_simplefunction.cpp", "-I", TESTDATA_DIR}};
FileContainer source2{{TESTDATA_DIR, "query_simplefunction2.cpp"},
{},
{"cc", TESTDATA_DIR"/query_simplefunction2.cpp", "-I", TESTDATA_DIR}};
FileContainer source3{{TESTDATA_DIR, "query_simplefunction3.cpp"},
{},
{"cc", TESTDATA_DIR"/query_simplefunction3.cpp", "-I", TESTDATA_DIR}};
Utils::SmallString unsavedContent{"void f();"};
FileContainer unsaved{{TESTDATA_DIR, "query_simplefunction.h"},
unsavedContent.clone(),
......@@ -83,19 +91,19 @@ protected:
Utils::SmallString query{"functionDecl()"};
ClangBackEnd::ClangQueryGatherer gatherer{&filePathCache, {source.clone()}, {unsaved.clone()}, query.clone()};
ClangBackEnd::ClangQueryGatherer manyGatherer{&filePathCache,
{source.clone(), source.clone(), source.clone()},
{source3.clone(), source2.clone(), source.clone()},
{unsaved.clone()},
query.clone()};
};
TEST_F(ClangQueryGatherer, CreateSourceRangesAndDiagnostics)
{
auto sourceRangesAndDiagnostics = gatherer.createSourceRangesAndDiagnosticsForSource(&filePathCache, source.clone(), {}, query.clone());
auto sourceRangesAndDiagnostics = gatherer.createSourceRangesAndDiagnosticsForSource(&filePathCache, source.clone(), {unsaved}, query.clone());
ASSERT_THAT(sourceRangesAndDiagnostics,
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
Contains(IsSourceRangeWithText(2, 1, 3, 4, "void f()\n {}")))));
Contains(IsSourceRangeWithText(2, 1, 2, 12, "void f() {}")))));
}
TEST_F(ClangQueryGatherer, CreateSourceRangesAndDiagnosticssWithUnsavedContent)
......@@ -176,14 +184,18 @@ TEST_F(ClangQueryGatherer, AfterStartCreateSourceRangesAndDiagnosticsMessagesGet
manyGatherer.startCreateNextSourceRangesAndDiagnosticsMessages();
ASSERT_THAT(manyGatherer.allCurrentProcessedMessages(),
Each(
UnorderedElementsAre(
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
UnorderedElementsAre(IsSourceRangeWithText(1, 1, 1, 9, "void f();"),
IsSourceRangeWithText(2, 1, 2, 12, "void f() {}")))),
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
Contains(IsSourceRangeWithText(1, 1, 1, 9, "void f();"))))));
UnorderedElementsAre(
IsSourceRangeWithText(1, 1, 1, 13, "int header();"),
IsSourceRangeWithText(3, 1, 3, 15, "int function();"))))));
}
TEST_F(ClangQueryGatherer, GetFinishedMessages)
{
manyGatherer.startCreateNextSourceRangesAndDiagnosticsMessages();
......@@ -193,10 +205,17 @@ TEST_F(ClangQueryGatherer, GetFinishedMessages)
ASSERT_THAT(messages,
AllOf(SizeIs(2),
Each(
UnorderedElementsAre(
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
Contains(IsSourceRangeWithText(1, 1, 1, 9, "void f();")))))));
UnorderedElementsAre(
IsSourceRangeWithText(1, 1, 1, 9, "void f();"),
IsSourceRangeWithText(2, 1, 2, 12, "void f() {}")))),
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
UnorderedElementsAre(
IsSourceRangeWithText(1, 1, 1, 13, "int header();"),
IsSourceRangeWithText(3, 1, 3, 15, "int function();")))))));
}
TEST_F(ClangQueryGatherer, GetFinishedMessagesAfterSecondPass)
......@@ -211,10 +230,38 @@ TEST_F(ClangQueryGatherer, GetFinishedMessagesAfterSecondPass)
ASSERT_THAT(messages,
AllOf(SizeIs(1),
Each(
ElementsAre(
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
UnorderedElementsAre(
IsSourceRangeWithText(3, 1, 3, 15, "int function();")))))));
}
TEST_F(ClangQueryGatherer, FilterDuplicates)
{
manyGatherer.setProcessingSlotCount(3);
manyGatherer.startCreateNextSourceRangesAndDiagnosticsMessages();
manyGatherer.waitForFinished();
auto messages = manyGatherer.finishedMessages();
ASSERT_THAT(messages,
AllOf(SizeIs(3),
UnorderedElementsAre(
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
UnorderedElementsAre(
IsSourceRangeWithText(1, 1, 1, 9, "void f();"),
IsSourceRangeWithText(2, 1, 2, 12, "void f() {}")))),
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
UnorderedElementsAre(
IsSourceRangeWithText(1, 1, 1, 13, "int header();"),
IsSourceRangeWithText(3, 1, 3, 15, "int function();")))),
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
Contains(IsSourceRangeWithText(1, 1, 1, 9, "void f();")))))));
UnorderedElementsAre(
IsSourceRangeWithText(3, 1, 3, 15, "int function();")))))));
}
TEST_F(ClangQueryGatherer, AfterGetFinishedMessagesFuturesAreReduced)
......
#include "query_simplefunction2.h"
int function();
#include "query_simplefunction2.h"
int function();
......@@ -40,6 +40,7 @@ namespace {
using testing::AllOf;
using testing::Contains;
using testing::NiceMock;
using testing::Not;
using testing::Pair;
using testing::PrintToString;
using testing::Property;
......@@ -77,7 +78,8 @@ protected:
Utils::SmallString sourceContent{"void f()\n {}"};
FileContainer source{{TESTDATA_DIR, "query_simplefunction.cpp"},
sourceContent.clone(),
{"cc", "query_simplefunction.cpp"}};
{"cc", TESTDATA_DIR"/query_simplefunction.cpp"}};
int processingSlotCount = 2;
};
using RefactoringServerSlowTest = RefactoringServer;
......@@ -153,8 +155,12 @@ TEST_F(RefactoringServerSlowTest, RequestTwoSourceRangesAndDiagnosticsForQueryMe
sourceRangesAndDiagnosticsForQueryMessage(
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
Contains(IsSourceRangeWithText(1, 1, 2, 4, sourceContent))))))
.Times(2);
Contains(IsSourceRangeWithText(1, 1, 2, 4, sourceContent))))));
EXPECT_CALL(mockRefactoringClient,
sourceRangesAndDiagnosticsForQueryMessage(
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
Not(Contains(IsSourceRangeWithText(1, 1, 2, 4, sourceContent)))))));
refactoringServer.requestSourceRangesAndDiagnosticsForQueryMessage(std::move(requestSourceRangesAndDiagnosticsForQueryMessage));
}
......@@ -163,7 +169,7 @@ TEST_F(RefactoringServerVerySlowTest, RequestManySourceRangesAndDiagnosticsForQu
{
std::vector<FileContainer> sources;
std::fill_n(std::back_inserter(sources),
std::thread::hardware_concurrency() + 3,
processingSlotCount + 3,
source.clone());
RequestSourceRangesAndDiagnosticsForQueryMessage requestSourceRangesAndDiagnosticsForQueryMessage{"functionDecl()",
std::move(sources),
......@@ -173,8 +179,13 @@ TEST_F(RefactoringServerVerySlowTest, RequestManySourceRangesAndDiagnosticsForQu
sourceRangesAndDiagnosticsForQueryMessage(
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
Contains(IsSourceRangeWithText(1, 1, 2, 4, sourceContent))))))
.Times(std::thread::hardware_concurrency() + 3);
Contains(IsSourceRangeWithText(1, 1, 2, 4, sourceContent))))));
EXPECT_CALL(mockRefactoringClient,
sourceRangesAndDiagnosticsForQueryMessage(
Property(&SourceRangesAndDiagnosticsForQueryMessage::sourceRanges,
Property(&SourceRangesContainer::sourceRangeWithTextContainers,
Not(Contains(IsSourceRangeWithText(1, 1, 2, 4, sourceContent)))))))
.Times(processingSlotCount + 2);
refactoringServer.requestSourceRangesAndDiagnosticsForQueryMessage(std::move(requestSourceRangesAndDiagnosticsForQueryMessage));
}
......@@ -237,6 +248,7 @@ void RefactoringServer::SetUp()
void RefactoringServer::TearDown()
{
refactoringServer.setGathererProcessingSlotCount(uint(processingSlotCount));
refactoringServer.waitThatSourceRangesAndDiagnosticsForQueryMessagesAreFinished();
}
......
/****************************************************************************
**
** Copyright (C) 2017 The Qt Company Ltd.
** Contact: https://www.qt.io/licensing/
**
** This file is part of Qt Creator.
**
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and The Qt Company. For licensing terms
** and conditions see https://www.qt.io/terms-conditions. For further
** information use the contact form at https://www.qt.io/contact-us.
**
** GNU General Public License Usage
** Alternatively, this file may be used under the terms of the GNU
** General Public License version 3 as published by the Free Software
** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
** included in the packaging of this file. Please review the following
** information to ensure the GNU General Public License requirements will
** be met: https://www.gnu.org/licenses/gpl-3.0.html.
**
****************************************************************************/
#include "googletest.h"
#include <sourcerangefilter.h>
namespace {
using testing::ContainerEq;
using testing::IsEmpty;
using ClangBackEnd::SourceRangeWithTextContainer;
using ClangBackEnd::SourceRangeWithTextContainers;
using ClangBackEnd::SourceRangesAndDiagnosticsForQueryMessage;
class SourceRangeFilter : public ::testing::Test
{
protected:
protected:
SourceRangeWithTextContainers sourceRanges1{{1, 1, 1, 1, 2, 1, 4, "foo"},
{2, 1, 1, 1, 2, 1, 4, "foo"},
{1, 1, 1, 1, 2, 2, 5, "foo"}};
SourceRangeWithTextContainers sourceRanges2{{1, 1, 1, 1, 2, 1, 4, "foo"},
{3, 1, 1, 1, 2, 1, 4, "foo"},
{1, 1, 1, 1, 2, 2, 6, "foo"}};
SourceRangeWithTextContainers sourceRanges3{{3, 1, 1, 1, 2, 1, 4, "foo"},
{1, 1, 1, 1, 2, 2, 6, "foo"}};
SourceRangesAndDiagnosticsForQueryMessage message1{{{}, Utils::clone(sourceRanges1)}, {}};
SourceRangesAndDiagnosticsForQueryMessage message2{{{}, Utils::clone(sourceRanges2)}, {}};
ClangBackEnd::SourceRangeFilter filter{3};
};
TEST_F(SourceRangeFilter, DontChangeForFirstTime)
{
auto expectedSourceRanges = sourceRanges1;
filter.removeDuplicates(sourceRanges1);
ASSERT_THAT(sourceRanges1, ContainerEq(expectedSourceRanges));
}
TEST_F(SourceRangeFilter, DoNotFilterNonDuplicates)
{
SourceRangeWithTextContainers expectedSourceRanges = sourceRanges3;
filter.removeDuplicates(sourceRanges1);
filter.removeDuplicates(sourceRanges3);
ASSERT_THAT(sourceRanges3, ContainerEq(expectedSourceRanges));
}
TEST_F(SourceRangeFilter, FilterDuplicates)
{
filter.removeDuplicates(sourceRanges1);
filter.removeDuplicates(sourceRanges2);
ASSERT_THAT(sourceRanges2, ContainerEq(sourceRanges3));
}
TEST_F(SourceRangeFilter, FilterMoreDuplicates)
{
filter.removeDuplicates(sourceRanges1);
filter.removeDuplicates(sourceRanges2);
filter.removeDuplicates(sourceRanges3);
ASSERT_THAT(sourceRanges3, IsEmpty());
}
TEST_F(SourceRangeFilter, FilterDuplicatesFromMessage)
{
filter.removeDuplicates(std::move(message1));
auto filteredMessage = filter.removeDuplicates(std::move(message2));
ASSERT_THAT(filteredMessage.sourceRanges().sourceRangeWithTextContainers(),
ContainerEq(sourceRanges3));
}
}
......@@ -64,7 +64,8 @@ SOURCES += \
pchmanagerserver-test.cpp \
pchmanagerclientserverinprocess-test.cpp \
clangquerygatherer-test.cpp \
filepath-test.cpp
filepath-test.cpp \
sourcerangefilter-test.cpp
!isEmpty(LIBCLANG_LIBS) {
SOURCES += \
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment