Clang: Don't filter source with missing includes

We do now tag and save them. So we can reliably track them.

Task-number: QTCREATORBUG-22035
Change-Id: I49aaeeb76150b7e2d77b863eeb0aedefc9ab50f4
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>
This commit is contained in:
Marco Bubke
2019-02-28 17:52:34 +01:00
parent 1a63297d9b
commit 3e1faec230
13 changed files with 187 additions and 105 deletions

View File

@@ -144,6 +144,7 @@ public:
const Sqlite::Column &sourceIdColumn = table.addColumn("sourceId", Sqlite::ColumnType::Integer);
table.addColumn("sourceType", Sqlite::ColumnType::Integer);
table.addColumn("pchCreationTimeStamp", Sqlite::ColumnType::Integer);
table.addColumn("hasMissingIncludes", Sqlite::ColumnType::Integer);
table.addUniqueIndex({sourceIdColumn, projectPartIdColumn});
table.addIndex({projectPartIdColumn});

View File

@@ -59,9 +59,11 @@ public:
projectPartId);
for (const SourceEntry &entry : sourceEntries) {
insertOrUpdateSourceTypeStatement.write(entry.sourceId.filePathId,
insertOrUpdateProjectPartsSourcesStatement.write(
entry.sourceId.filePathId,
projectPartId,
static_cast<uchar>(entry.sourceType));
static_cast<uchar>(entry.sourceType),
static_cast<uchar>(entry.hasMissingIncludes));
}
}
@@ -120,9 +122,8 @@ public:
SourceEntries fetchDependSources(FilePathId sourceId, int projectPartId) const override
{
return fetchSourceDependenciesStatement.template values<SourceEntry, 3>(300,
sourceId.filePathId,
projectPartId);
return fetchSourceDependenciesStatement.template values<SourceEntry, 4>(
300, sourceId.filePathId, projectPartId);
}
UsedMacros fetchUsedMacros(FilePathId sourceId) const override
@@ -242,20 +243,20 @@ public:
"DELETE FROM newSourceDependencies",
database
};
WriteStatement insertOrUpdateSourceTypeStatement{
WriteStatement insertOrUpdateProjectPartsSourcesStatement{
"INSERT INTO projectPartsSources(sourceId, projectPartId, "
"sourceType) VALUES (?001, ?002, ?003) ON CONFLICT(sourceId, "
"projectPartId) DO UPDATE SET sourceType = ?003",
"sourceType, hasMissingIncludes) VALUES (?001, ?002, ?003, ?004) ON "
"CONFLICT(sourceId, projectPartId) DO UPDATE SET sourceType = ?003, "
"hasMissingIncludes = ?004",
database};
mutable ReadStatement fetchSourceDependenciesStatement{
"WITH RECURSIVE collectedDependencies(sourceId) AS (VALUES(?) UNION "
"SELECT dependencySourceId FROM sourceDependencies, "
"collectedDependencies WHERE sourceDependencies.sourceId == "
"collectedDependencies.sourceId) SELECT sourceId, "
"pchCreationTimeStamp, sourceType FROM "
"pchCreationTimeStamp, sourceType, hasMissingIncludes FROM "
"collectedDependencies NATURAL JOIN projectPartsSources WHERE "
"projectPartId = ? ORDER BY "
"sourceId",
"projectPartId = ? ORDER BY sourceId",
database};
mutable ReadStatement fetchProjectPartIdStatement{
"SELECT projectPartId FROM projectParts WHERE projectPartName = ?",

View File

@@ -204,8 +204,8 @@ public:
void appendContainsMissingIncludes(const FilePathIds &dependentSourceFilesWithMissingIncludes)
{
auto split = m_containsMissingIncludes
.insert(m_containsMissingIncludes.end(),
auto split = m_containsMissingIncludes.insert(
m_containsMissingIncludes.end(),
dependentSourceFilesWithMissingIncludes.begin(),
dependentSourceFilesWithMissingIncludes.end());
std::inplace_merge(m_containsMissingIncludes.begin(),
@@ -217,11 +217,13 @@ public:
{
FilePathIds filteredDependentSourceFilesWithMissingIncludes;
filteredDependentSourceFilesWithMissingIncludes.reserve(dependentSourceFilesWithMissingIncludes.size());
std::set_difference(dependentSourceFilesWithMissingIncludes.begin(),
std::set_difference(
dependentSourceFilesWithMissingIncludes.begin(),
dependentSourceFilesWithMissingIncludes.end(),
m_containsMissingIncludes.begin(),
m_containsMissingIncludes.end(),
std::back_inserter(filteredDependentSourceFilesWithMissingIncludes));
std::back_inserter(
filteredDependentSourceFilesWithMissingIncludes));
dependentSourceFilesWithMissingIncludes = filteredDependentSourceFilesWithMissingIncludes;
}
@@ -265,8 +267,7 @@ public:
sourceDependencies);
}
void removeSourceWithMissingIncludesFromIncludes()
{
void removeSourceWithMissingIncludesFromSources() {
class Compare
{
public:
@@ -280,17 +281,16 @@ public:
}
};
auto &includes = m_buildDependency.sources;
SourceEntries newIncludes;
newIncludes.reserve(includes.size());
std::set_difference(includes.begin(),
includes.end(),
SourceEntryReferences sourcesWithMissingIncludes;
sourcesWithMissingIncludes.reserve(m_containsMissingIncludes.size());
std::set_intersection(m_buildDependency.sources.begin(),
m_buildDependency.sources.end(),
m_containsMissingIncludes.begin(),
m_containsMissingIncludes.end(),
std::back_inserter(newIncludes),
std::back_inserter(sourcesWithMissingIncludes),
Compare{});
m_buildDependency.sources = newIncludes;
for (SourceEntryReference entry : sourcesWithMissingIncludes)
entry.get().hasMissingIncludes = HasMissingIncludes::Yes;
}
SourceDependencies sourceDependenciesSortedByDependendFilePathId() const
@@ -311,7 +311,7 @@ public:
collectSourceWithMissingIncludes(m_containsMissingIncludes,
sourceDependenciesSortedByDependendFilePathId());
removeSourceWithMissingIncludesFromIncludes();
removeSourceWithMissingIncludesFromSources();
}
void ensureDirectory(const QString &directory, const QString &fileName)

View File

@@ -40,6 +40,8 @@ enum class SourceType : unsigned char {
Source
};
enum class HasMissingIncludes : unsigned char { No, Yes };
class TimeStamp
{
using int64 = long long;
@@ -108,20 +110,23 @@ class SourceEntry
using int64 = long long;
public:
SourceEntry(int sourceId, int64 pchCreationTimeStamp, int sourceType)
: pchCreationTimeStamp(pchCreationTimeStamp)
, sourceId(sourceId)
, sourceType(static_cast<SourceType>(sourceType))
{}
SourceEntry(int sourceId,
int64 pchCreationTimeStamp,
int sourceType,
int hasMissingIncludes)
: pchCreationTimeStamp(pchCreationTimeStamp), sourceId(sourceId),
sourceType(static_cast<SourceType>(sourceType)),
hasMissingIncludes(
static_cast<HasMissingIncludes>(hasMissingIncludes)) {}
SourceEntry(FilePathId sourceId, SourceType sourceType, TimeStamp pchCreationTimeStamp)
: pchCreationTimeStamp(pchCreationTimeStamp)
, sourceId(sourceId)
, sourceType(sourceType)
{}
SourceEntry(FilePathId sourceId,
SourceType sourceType,
TimeStamp pchCreationTimeStamp,
HasMissingIncludes hasMissingIncludes = HasMissingIncludes::No)
: pchCreationTimeStamp(pchCreationTimeStamp), sourceId(sourceId),
sourceType(sourceType), hasMissingIncludes(hasMissingIncludes) {}
friend bool operator<(SourceEntry first, SourceEntry second)
{
friend bool operator<(SourceEntry first, SourceEntry second) {
return first.sourceId < second.sourceId;
}
@@ -137,8 +142,10 @@ public:
TimeStamp pchCreationTimeStamp;
FilePathId sourceId;
SourceType sourceType = SourceType::UserInclude;
HasMissingIncludes hasMissingIncludes = HasMissingIncludes::No;
};
using SourceEntries = std::vector<SourceEntry>;
}
using SourceEntryReference = std::reference_wrapper<SourceEntry>;
using SourceEntryReferences = std::vector<SourceEntryReference>;
} // namespace ClangBackEnd

View File

@@ -87,23 +87,22 @@ public:
UsedMacroFilter(const SourceEntries &includes,
const UsedMacros &usedMacros,
const CompilerMacros &compilerMacros)
{
filterIncludes(includes);
const CompilerMacros &compilerMacros) {
filterSources(includes);
systemUsedMacros = filterUsedMarcos(usedMacros, systemIncludes);
projectUsedMacros = filterUsedMarcos(usedMacros, projectIncludes);
filter(compilerMacros);
}
void filterIncludes(const SourceEntries &includes)
{
systemIncludes.reserve(includes.size());
projectIncludes.reserve(includes.size());
topSystemIncludes.reserve(includes.size() / 10);
topProjectIncludes.reserve(includes.size() / 10);
void filterSources(const SourceEntries &sources) {
systemIncludes.reserve(sources.size());
projectIncludes.reserve(sources.size());
topSystemIncludes.reserve(sources.size() / 10);
topProjectIncludes.reserve(sources.size() / 10);
this->sources.reserve(sources.size());
for (SourceEntry include : includes)
filterInclude(include);
for (SourceEntry source : sources)
filterSource(source);
}
void filter(const CompilerMacros &compilerMacros)
@@ -121,29 +120,31 @@ public:
}
private:
void filterInclude(SourceEntry include)
{
switch (include.sourceType) {
void filterSource(SourceEntry source) {
if (source.hasMissingIncludes == HasMissingIncludes::Yes)
return;
switch (source.sourceType) {
case SourceType::TopSystemInclude:
topSystemIncludes.emplace_back(include.sourceId);
systemIncludes.emplace_back(include.sourceId);
topSystemIncludes.emplace_back(source.sourceId);
systemIncludes.emplace_back(source.sourceId);
break;
case SourceType::SystemInclude:
systemIncludes.emplace_back(include.sourceId);
systemIncludes.emplace_back(source.sourceId);
break;
case SourceType::TopProjectInclude:
topProjectIncludes.emplace_back(include.sourceId);
projectIncludes.emplace_back(include.sourceId);
topProjectIncludes.emplace_back(source.sourceId);
projectIncludes.emplace_back(source.sourceId);
break;
case SourceType::ProjectInclude:
projectIncludes.emplace_back(include.sourceId);
projectIncludes.emplace_back(source.sourceId);
break;
case SourceType::UserInclude:
case SourceType::Source:
break;
}
sources.emplace_back(include.sourceId);
sources.emplace_back(source.sourceId);
}
static Utils::SmallStringVector filterUsedMarcos(const UsedMacros &usedMacros,

View File

@@ -61,7 +61,7 @@ protected:
MockSqliteWriteStatement &deleteOutdatedSourceDependenciesStatement = storage.deleteOutdatedSourceDependenciesStatement;
MockSqliteWriteStatement &deleteNewSourceDependenciesStatement = storage.deleteNewSourceDependenciesStatement;
MockSqliteReadStatement &getLowestLastModifiedTimeOfDependencies = storage.getLowestLastModifiedTimeOfDependencies;
MockSqliteWriteStatement &insertOrUpdateSourceTypeStatement = storage.insertOrUpdateSourceTypeStatement;
MockSqliteWriteStatement &insertOrUpdateProjectPartsSourcesStatement = storage.insertOrUpdateProjectPartsSourcesStatement;
MockSqliteReadStatement &fetchSourceDependenciesStatement = storage.fetchSourceDependenciesStatement;
MockSqliteReadStatement &fetchProjectPartIdStatement = storage.fetchProjectPartIdStatement;
MockSqliteReadStatement &fetchUsedMacrosStatement = storage.fetchUsedMacrosStatement;
@@ -174,14 +174,14 @@ TEST_F(BuildDependenciesStorage, AddNewSourceDependenciesTable)
TEST_F(BuildDependenciesStorage, UpdateSources)
{
InSequence s;
SourceEntries entries{{1, SourceType::TopProjectInclude, 10},
SourceEntries entries{{1, SourceType::TopProjectInclude, 10, ClangBackEnd::HasMissingIncludes::Yes},
{2, SourceType::TopSystemInclude, 20}};
EXPECT_CALL(deleteAllProjectPartsSourcesWithProjectPartNameStatement, write(TypedEq<int>(22)));
EXPECT_CALL(insertOrUpdateSourceTypeStatement,
write(TypedEq<int>(1), TypedEq<int>(22), TypedEq<uchar>(0)));
EXPECT_CALL(insertOrUpdateSourceTypeStatement,
write(TypedEq<int>(2), TypedEq<int>(22), TypedEq<uchar>(1)));
EXPECT_CALL(insertOrUpdateProjectPartsSourcesStatement,
write(TypedEq<int>(1), TypedEq<int>(22), TypedEq<uchar>(0), TypedEq<uchar>(1)));
EXPECT_CALL(insertOrUpdateProjectPartsSourcesStatement,
write(TypedEq<int>(2), TypedEq<int>(22), TypedEq<uchar>(1), TypedEq<uchar>(0)));
storage.insertOrUpdateSources(entries, 22);
}

View File

@@ -47,6 +47,7 @@ using ClangBackEnd::BuildDependency;
using ClangBackEnd::FilePathId;
using ClangBackEnd::FilePathIds;
using ClangBackEnd::FilePathView;
using ClangBackEnd::HasMissingIncludes;
using ClangBackEnd::SourceDependency;
using ClangBackEnd::SourceType;
using ClangBackEnd::UsedMacro;
@@ -57,11 +58,26 @@ MATCHER_P2(HasSource,
sourceId,
sourceType,
std::string(negation ? "hasn't " : "has ")
+ PrintToString(ClangBackEnd::SourceEntry(sourceId, sourceType, -1)))
+ PrintToString(ClangBackEnd::SourceEntry(
sourceId, sourceType, -1, ClangBackEnd::HasMissingIncludes::No)))
{
const ClangBackEnd::SourceEntry &entry = arg;
return entry.sourceId == sourceId && entry.sourceType == sourceType;
return entry.sourceId == sourceId && entry.sourceType == sourceType
&& entry.hasMissingIncludes == ClangBackEnd::HasMissingIncludes::No;
}
MATCHER_P3(HasSource,
sourceId,
sourceType,
hasMissingIncludes,
std::string(negation ? "hasn't " : "has ")
+ PrintToString(ClangBackEnd::SourceEntry(sourceId, sourceType, -1, hasMissingIncludes)))
{
const ClangBackEnd::SourceEntry &entry = arg;
return entry.sourceId == sourceId && entry.sourceType == sourceType
&& entry.hasMissingIncludes == hasMissingIncludes;
}
class BuildDependencyCollector : public ::testing::Test
@@ -199,6 +215,7 @@ TEST_F(BuildDependencyCollector, NoDuplicate)
ASSERT_THAT(sources(collector.sourceEntries()),
UnorderedElementsAre(
id(TESTDATA_DIR "/builddependencycollector/project/main.cpp"),
id(TESTDATA_DIR "/builddependencycollector/project/main2.cpp"),
id(TESTDATA_DIR "/builddependencycollector/project/header1.h"),
id(TESTDATA_DIR "/builddependencycollector/project/header2.h"),
id(TESTDATA_DIR "/builddependencycollector/external/external1.h"),
@@ -214,6 +231,7 @@ TEST_F(BuildDependencyCollector, IncludesAreSorted)
ASSERT_THAT(sources(collector.sourceEntries()),
ElementsAre(id(TESTDATA_DIR "/builddependencycollector/project/main.cpp"),
id(TESTDATA_DIR "/builddependencycollector/project/main2.cpp"),
id(TESTDATA_DIR "/builddependencycollector/project/header1.h"),
id(TESTDATA_DIR "/builddependencycollector/project/header2.h"),
id(TESTDATA_DIR "/builddependencycollector/external/external3.h"),
@@ -262,6 +280,7 @@ TEST_F(BuildDependencyCollector, IgnoreMissingFile)
ASSERT_THAT(sources(emptyCollector.sourceEntries()),
UnorderedElementsAre(
id(TESTDATA_DIR "/builddependencycollector/project/missingfile.cpp"),
id(TESTDATA_DIR "/builddependencycollector/external/external1.h"),
id(TESTDATA_DIR "/builddependencycollector/external/indirect_external.h"),
id(TESTDATA_DIR "/builddependencycollector/external/indirect_external2.h")));
@@ -634,10 +653,37 @@ TEST_F(BuildDependencyCollector, MissingInclude)
emptyCollector.collect();
ASSERT_THAT(emptyCollector.sourceEntries(),
ElementsAre(
ASSERT_THAT(
emptyCollector.sourceEntries(),
UnorderedElementsAre(
HasSource(id(TESTDATA_DIR "/builddependencycollector/project/main5.cpp"),
SourceType::Source,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR "/builddependencycollector/project/missinginclude2.h"),
SourceType::ProjectInclude,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR
"/builddependencycollector/project/indirect_missinginclude.h"),
SourceType::ProjectInclude,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR
"/builddependencycollector/project/indirect_missinginclude3.h"),
SourceType::ProjectInclude,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR
"/builddependencycollector/project/indirect_missinginclude4.h"),
SourceType::ProjectInclude,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR "/builddependencycollector/project/missinginclude3.h"),
SourceType::ProjectInclude,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR
"/builddependencycollector/project/indirect_missinginclude2.h"),
SourceType::ProjectInclude,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR "/builddependencycollector/project/header1.h"),
SourceType::UserInclude)));
SourceType::UserInclude,
HasMissingIncludes::No)));
}
@@ -735,6 +781,12 @@ TEST_F(BuildDependencyCollector, Create)
Field(
&BuildDependency::sources,
UnorderedElementsAre(
HasSource(id(TESTDATA_DIR "/builddependencycollector/project/main4.cpp"),
SourceType::Source,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR "/builddependencycollector/project/missingfile.h"),
SourceType::UserInclude,
HasMissingIncludes::Yes),
HasSource(id(TESTDATA_DIR "/builddependencycollector/project/header1.h"),
SourceType::UserInclude),
HasSource(id(TESTDATA_DIR "/builddependencycollector/project/header2.h"),

View File

@@ -1179,7 +1179,7 @@ std::ostream &operator<<(std::ostream &out, const SlotUsage &slotUsage)
return out << "(" << slotUsage.free << ", " << slotUsage.used << ")";
}
const char *sourceTypeString(SourceType sourceType)
const char *typeToString(SourceType sourceType)
{
using ClangBackEnd::SymbolTag;
@@ -1201,9 +1201,24 @@ const char *sourceTypeString(SourceType sourceType)
return "";
}
const char *typeToString(HasMissingIncludes hasMissingIncludes)
{
using ClangBackEnd::SymbolTag;
switch (hasMissingIncludes) {
case HasMissingIncludes::No:
return "HasMissingIncludes::No";
case HasMissingIncludes::Yes:
return "HasMissingIncludes::Yes";
}
return "";
}
std::ostream &operator<<(std::ostream &out, const SourceEntry &entry)
{
return out << "(" << entry.sourceId << ", " << sourceTypeString(entry.sourceType) << ")";
return out << "(" << entry.sourceId << ", " << typeToString(entry.sourceType) << ", "
<< typeToString(entry.hasMissingIncludes) << ")";
}
const char *typeToString(IncludeSearchPathType type)

View File

@@ -187,8 +187,9 @@ MockSqliteReadStatement::value<SourceLocation, 3>(const long long &symbolId, con
}
template<>
SourceEntries
MockSqliteReadStatement::values<SourceEntry, 3>(std::size_t reserveSize, const int &filePathId, const int &projectPartId)
SourceEntries MockSqliteReadStatement::values<SourceEntry, 4>(std::size_t reserveSize,
const int &filePathId,
const int &projectPartId)
{
return valuesReturnSourceEntries(reserveSize, filePathId, projectPartId);
}

View File

@@ -256,8 +256,9 @@ Utils::optional<SourceLocation>
MockSqliteReadStatement::value<SourceLocation, 3>(const long long &symbolId, const int &locationKind);
template<>
SourceEntries
MockSqliteReadStatement::values<SourceEntry, 3>(std::size_t reserveSize, const int&, const int&);
SourceEntries MockSqliteReadStatement::values<SourceEntry, 4>(std::size_t reserveSize,
const int &,
const int &);
template <>
Utils::optional<Sources::SourceNameAndDirectoryId>

View File

@@ -109,7 +109,7 @@ public:
MOCK_METHOD2(write,
void (uchar, int));
MOCK_METHOD3(write, void(int, int, uchar));
MOCK_METHOD4(write, void(int, int, uchar, uchar));
MOCK_METHOD2(write,
void (long long, int));
MOCK_METHOD2(write, void(long long, Utils::SmallStringView));

View File

@@ -104,7 +104,8 @@ TEST_F(RefactoringDatabaseInitializer, AddProjectPartsSourcesTable)
EXPECT_CALL(mockDatabase,
execute(Eq("CREATE TABLE IF NOT EXISTS projectPartsSources(projectPartId INTEGER, "
"sourceId INTEGER, sourceType INTEGER, pchCreationTimeStamp INTEGER)")));
"sourceId INTEGER, sourceType INTEGER, pchCreationTimeStamp INTEGER, "
"hasMissingIncludes INTEGER)")));
EXPECT_CALL(mockDatabase, execute(Eq("CREATE UNIQUE INDEX IF NOT EXISTS index_projectPartsSources_sourceId_projectPartId ON projectPartsSources(sourceId, projectPartId)")));
EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_projectPartsSources_projectPartId ON projectPartsSources(projectPartId)")));
@@ -179,7 +180,8 @@ TEST_F(RefactoringDatabaseInitializer, CreateInTheContructor)
EXPECT_CALL(mockDatabase, execute(Eq("CREATE UNIQUE INDEX IF NOT EXISTS index_projectParts_projectPartName ON projectParts(projectPartName)")));
EXPECT_CALL(mockDatabase,
execute(Eq("CREATE TABLE IF NOT EXISTS projectPartsSources(projectPartId INTEGER, "
"sourceId INTEGER, sourceType INTEGER, pchCreationTimeStamp INTEGER)")));
"sourceId INTEGER, sourceType INTEGER, pchCreationTimeStamp INTEGER, "
"hasMissingIncludes INTEGER)")));
EXPECT_CALL(mockDatabase, execute(Eq("CREATE UNIQUE INDEX IF NOT EXISTS index_projectPartsSources_sourceId_projectPartId ON projectPartsSources(sourceId, projectPartId)")));
EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_projectPartsSources_projectPartId ON projectPartsSources(projectPartId)")));
EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS usedMacros(usedMacroId INTEGER PRIMARY KEY, sourceId INTEGER, macroName TEXT)")));

View File

@@ -46,7 +46,8 @@ protected:
{3, SourceType::ProjectInclude, 0},
{4, SourceType::TopSystemInclude, 0},
{5, SourceType::TopProjectInclude, 0},
{6, SourceType::Source, 0}};
{6, SourceType::Source, 0},
{7, SourceType::TopProjectInclude, 0, ClangBackEnd::HasMissingIncludes::Yes}};
UsedMacros usedMacros{{"YI", 1},
{"ER", 2},
{"SE", 2},