diff --git a/src/tools/clangpchmanagerbackend/source/collectincludesaction.h b/src/tools/clangpchmanagerbackend/source/collectincludesaction.h index 9469e16fb0a..923cf5bfc0c 100644 --- a/src/tools/clangpchmanagerbackend/source/collectincludesaction.h +++ b/src/tools/clangpchmanagerbackend/source/collectincludesaction.h @@ -40,10 +40,12 @@ class CollectIncludesAction final : public clang::PreprocessOnlyAction { public: CollectIncludesAction(FilePathIds &includeIds, + FilePathIds &topIncludeIds, FilePathCachingInterface &filePathCache, std::vector &excludedIncludeUID, std::vector &alreadyIncludedFileUIDs) : m_includeIds(includeIds), + m_topIncludeIds(topIncludeIds), m_filePathCache(filePathCache), m_excludedIncludeUID(excludedIncludeUID), m_alreadyIncludedFileUIDs(alreadyIncludedFileUIDs) @@ -61,6 +63,7 @@ public: auto macroPreprocessorCallbacks = new CollectIncludesPreprocessorCallbacks( headerSearch, m_includeIds, + m_topIncludeIds, m_filePathCache, m_excludedIncludeUID, m_alreadyIncludedFileUIDs, @@ -81,6 +84,7 @@ public: private: FilePathIds &m_includeIds; + FilePathIds &m_topIncludeIds; FilePathCachingInterface &m_filePathCache; std::vector &m_excludedIncludeUID; std::vector &m_alreadyIncludedFileUIDs; diff --git a/src/tools/clangpchmanagerbackend/source/collectincludespreprocessorcallbacks.h b/src/tools/clangpchmanagerbackend/source/collectincludespreprocessorcallbacks.h index 92b3bbe2a0c..645d87e91d5 100644 --- a/src/tools/clangpchmanagerbackend/source/collectincludespreprocessorcallbacks.h +++ b/src/tools/clangpchmanagerbackend/source/collectincludespreprocessorcallbacks.h @@ -50,12 +50,14 @@ class CollectIncludesPreprocessorCallbacks final : public clang::PPCallbacks public: CollectIncludesPreprocessorCallbacks(clang::HeaderSearch &headerSearch, FilePathIds &includeIds, + FilePathIds &topIncludeIds, FilePathCachingInterface &filePathCache, const std::vector &excludedIncludeUID, std::vector &alreadyIncludedFileUIDs, clang::SourceManager &sourceManager) : m_headerSearch(headerSearch), m_includeIds(includeIds), + m_topIncludeIds(topIncludeIds), m_filePathCache(filePathCache), m_excludedIncludeUID(excludedIncludeUID), m_alreadyIncludedFileUIDs(alreadyIncludedFileUIDs), @@ -74,10 +76,8 @@ public: { if (!m_skipInclude && file) { auto fileUID = file->getUID(); - auto sourceFileID = m_sourceManager.getFileID(hashLocation); - auto sourceFileUID = m_sourceManager.getFileEntryForID(sourceFileID)->getUID(); - if (isNotInExcludedIncludeUID(fileUID) && isNotAlreadyIncluded(sourceFileUID).first) { - + auto sourceFileUID = m_sourceManager.getFileEntryForID(m_sourceManager.getFileID(hashLocation))->getUID(); + if (isNotInExcludedIncludeUID(fileUID)) { auto notAlreadyIncluded = isNotAlreadyIncluded(fileUID); if (notAlreadyIncluded.first) { m_alreadyIncludedFileUIDs.insert(notAlreadyIncluded.second, fileUID); @@ -85,6 +85,8 @@ public: if (!filePath.empty()) { FilePathId includeId = m_filePathCache.filePathId(filePath); m_includeIds.emplace_back(includeId); + if (isInExcludedIncludeUID(sourceFileUID)) + m_topIncludeIds.emplace_back(includeId); } } } @@ -132,9 +134,14 @@ public: bool isNotInExcludedIncludeUID(uint uid) const { - return !std::binary_search(m_excludedIncludeUID.begin(), - m_excludedIncludeUID.end(), - uid); + return !isInExcludedIncludeUID(uid); + } + + bool isInExcludedIncludeUID(uint uid) const + { + return std::binary_search(m_excludedIncludeUID.begin(), + m_excludedIncludeUID.end(), + uid); } std::pair::iterator> isNotAlreadyIncluded(uint uid) const @@ -154,6 +161,7 @@ public: private: clang::HeaderSearch &m_headerSearch; FilePathIds &m_includeIds; + FilePathIds &m_topIncludeIds; FilePathCachingInterface &m_filePathCache; const std::vector &m_excludedIncludeUID; std::vector &m_alreadyIncludedFileUIDs; diff --git a/src/tools/clangpchmanagerbackend/source/collectincludestoolaction.h b/src/tools/clangpchmanagerbackend/source/collectincludestoolaction.h index d8fa4ef24bf..e5c30829fa5 100644 --- a/src/tools/clangpchmanagerbackend/source/collectincludestoolaction.h +++ b/src/tools/clangpchmanagerbackend/source/collectincludestoolaction.h @@ -38,9 +38,11 @@ class CollectIncludesToolAction final : public clang::tooling::FrontendActionFac { public: CollectIncludesToolAction(FilePathIds &includeIds, + FilePathIds &topIncludeIds, FilePathCachingInterface &filePathCache, const Utils::PathStringVector &excludedIncludes) : m_includeIds(includeIds), + m_topIncludeIds(topIncludeIds), m_filePathCache(filePathCache), m_excludedIncludes(excludedIncludes) {} @@ -63,6 +65,7 @@ public: clang::FrontendAction *create() override { return new CollectIncludesAction(m_includeIds, + m_topIncludeIds, m_filePathCache, m_excludedIncludeUIDs, m_alreadyIncludedFileUIDs); @@ -74,7 +77,7 @@ public: fileUIDs.reserve(m_excludedIncludes.size()); for (const Utils::PathString &filePath : m_excludedIncludes) { - const clang::FileEntry *file = fileManager.getFile({filePath.data(), filePath.size()}); + const clang::FileEntry *file = fileManager.getFile({filePath.data(), filePath.size()}, true); if (file) fileUIDs.push_back(file->getUID()); @@ -89,6 +92,7 @@ private: std::vector m_alreadyIncludedFileUIDs; std::vector m_excludedIncludeUIDs; FilePathIds &m_includeIds; + FilePathIds &m_topIncludeIds; FilePathCachingInterface &m_filePathCache; const Utils::PathStringVector &m_excludedIncludes; }; diff --git a/src/tools/clangpchmanagerbackend/source/includecollector.cpp b/src/tools/clangpchmanagerbackend/source/includecollector.cpp index 6ac142e22b3..3c93de82b28 100644 --- a/src/tools/clangpchmanagerbackend/source/includecollector.cpp +++ b/src/tools/clangpchmanagerbackend/source/includecollector.cpp @@ -44,6 +44,7 @@ void IncludeCollector::collectIncludes() auto action = std::unique_ptr( new CollectIncludesToolAction(m_includeIds, + m_topIncludeIds, m_filePathCache, m_excludedIncludes)); @@ -76,6 +77,13 @@ FilePathIds IncludeCollector::takeIncludeIds() return std::move(m_includeIds); } +FilePathIds IncludeCollector::takeTopIncludeIds() +{ + std::sort(m_topIncludeIds.begin(), m_topIncludeIds.end()); + + return std::move(m_topIncludeIds); +} + } // namespace ClangBackEnd diff --git a/src/tools/clangpchmanagerbackend/source/includecollector.h b/src/tools/clangpchmanagerbackend/source/includecollector.h index 52e6e560dba..fc3c1a59910 100644 --- a/src/tools/clangpchmanagerbackend/source/includecollector.h +++ b/src/tools/clangpchmanagerbackend/source/includecollector.h @@ -41,10 +41,12 @@ public: void setExcludedIncludes(Utils::PathStringVector &&excludedIncludes); FilePathIds takeIncludeIds(); + FilePathIds takeTopIncludeIds(); private: Utils::PathStringVector m_excludedIncludes; FilePathIds m_includeIds; + FilePathIds m_topIncludeIds; Utils::SmallStringVector m_directories; FilePathCachingInterface &m_filePathCache; }; diff --git a/src/tools/clangpchmanagerbackend/source/pchcreator.cpp b/src/tools/clangpchmanagerbackend/source/pchcreator.cpp index c9809260bad..a73ee3ceecb 100644 --- a/src/tools/clangpchmanagerbackend/source/pchcreator.cpp +++ b/src/tools/clangpchmanagerbackend/source/pchcreator.cpp @@ -467,7 +467,7 @@ Utils::PathStringVector PchCreator::generateProjectPartHeaderAndSourcePaths( return includeAndSources; } -FilePathIds PchCreator::generateProjectPartPchIncludes( +std::pair PchCreator::generateProjectPartPchIncludes( const V2::ProjectPartContainer &projectPart) const { Utils::SmallString jointedFileContent = generateProjectPartHeaderAndSourcesContent(projectPart); @@ -492,7 +492,7 @@ FilePathIds PchCreator::generateProjectPartPchIncludes( jointFile->remove(); - return collector.takeIncludeIds(); + return {collector.takeIncludeIds(), collector.takeTopIncludeIds()}; } Utils::SmallString PchCreator::generateProjectPathPchHeaderFilePath( @@ -547,8 +547,10 @@ Utils::SmallStringVector PchCreator::generateProjectPartClangCompilerArguments( IdPaths PchCreator::generateProjectPartPch(const V2::ProjectPartContainer &projectPart) { long long lastModified = QDateTime::currentSecsSinceEpoch(); - auto includes = generateProjectPartPchIncludes(projectPart); - auto content = generatePchIncludeFileContent(includes); + FilePathIds allExternalIncludes; + FilePathIds topExternalIncludes; + std::tie(allExternalIncludes, topExternalIncludes) = generateProjectPartPchIncludes(projectPart); + auto content = generatePchIncludeFileContent(topExternalIncludes); auto pchIncludeFilePath = generateProjectPathPchHeaderFilePath(projectPart); auto pchFilePath = generateProjectPartPchFilePath(projectPart); generateFileWithContent(pchIncludeFilePath, content); @@ -556,7 +558,7 @@ IdPaths PchCreator::generateProjectPartPch(const V2::ProjectPartContainer &proje generatePch(generateProjectPartClangCompilerArguments(projectPart), {projectPart.projectPartId().clone(), std::move(pchFilePath), lastModified}); - return {projectPart.projectPartId().clone(), std::move(includes)}; + return {projectPart.projectPartId().clone(), std::move(allExternalIncludes)}; } void PchCreator::generatePchs() diff --git a/src/tools/clangpchmanagerbackend/source/pchcreator.h b/src/tools/clangpchmanagerbackend/source/pchcreator.h index c7b2b1a943a..62f3cfa5e9e 100644 --- a/src/tools/clangpchmanagerbackend/source/pchcreator.h +++ b/src/tools/clangpchmanagerbackend/source/pchcreator.h @@ -97,7 +97,7 @@ unittest_public: const V2::ProjectPartContainer &projectPart) const; Utils::PathStringVector generateProjectPartHeaderAndSourcePaths( const V2::ProjectPartContainer &projectPart) const; - FilePathIds generateProjectPartPchIncludes( + std::pair generateProjectPartPchIncludes( const V2::ProjectPartContainer &projectPart) const; Utils::SmallString generateProjectPathPchHeaderFilePath( const V2::ProjectPartContainer &projectPart) const; diff --git a/tests/unit/unittest/data/includecollector_indirect_external.h b/tests/unit/unittest/data/includecollector_indirect_external.h index 6f70f09beec..bfd11c5d74c 100644 --- a/tests/unit/unittest/data/includecollector_indirect_external.h +++ b/tests/unit/unittest/data/includecollector_indirect_external.h @@ -1 +1,3 @@ #pragma once + +#include "includecollector_indirect_external2.h" diff --git a/tests/unit/unittest/data/includecollector_indirect_external2.h b/tests/unit/unittest/data/includecollector_indirect_external2.h new file mode 100644 index 00000000000..3f59c932d39 --- /dev/null +++ b/tests/unit/unittest/data/includecollector_indirect_external2.h @@ -0,0 +1,2 @@ +#pragma once + diff --git a/tests/unit/unittest/includecollector-test.cpp b/tests/unit/unittest/includecollector-test.cpp index a9c925ef73e..0a823b52b7e 100644 --- a/tests/unit/unittest/includecollector-test.cpp +++ b/tests/unit/unittest/includecollector-test.cpp @@ -47,8 +47,21 @@ namespace { class IncludeCollector : public ::testing::Test { protected: - void SetUp(); - FilePathId id(const Utils::SmallStringView &path); + void SetUp() + { + collector.addFile(TESTDATA_DIR, "includecollector_main.cpp", "", {"cc", "includecollector_main.cpp"}); + collector.addFile(TESTDATA_DIR, "includecollector_main2.cpp", "", {"cc", "includecollector_main2.cpp"}); + + collector.addUnsavedFiles({{{TESTDATA_DIR, "includecollector_generated_file.h"}, "#pragma once", {}}}); + + collector.setExcludedIncludes(excludePaths.clone()); + emptyCollector.setExcludedIncludes(excludePaths.clone()); + } + + FilePathId id(const Utils::SmallStringView &path) + { + return filePathCache.filePathId(FilePathView{path}); + } protected: Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory}; @@ -56,8 +69,8 @@ protected: ClangBackEnd::FilePathCaching filePathCache{database}; ClangBackEnd::IncludeCollector collector{filePathCache}; ClangBackEnd::IncludeCollector emptyCollector{filePathCache}; - Utils::PathStringVector excludePaths = {TESTDATA_DIR "/includecollector_main.h", - TESTDATA_DIR "/includecollector_main2.h", + Utils::PathStringVector excludePaths = {TESTDATA_DIR "/includecollector_main.cpp", + TESTDATA_DIR "/includecollector_main2.cpp", TESTDATA_DIR "/includecollector_header1.h", TESTDATA_DIR "/includecollector_header2.h", TESTDATA_DIR "/includecollector_generated_file.h"}; @@ -69,7 +82,9 @@ TEST_F(IncludeCollector, IncludesExternalHeader) ASSERT_THAT(collector.takeIncludeIds(), AllOf(Contains(id(TESTDATA_DIR "/includecollector_external1.h")), - Contains(id(TESTDATA_DIR "/includecollector_external2.h")))); + Contains(id(TESTDATA_DIR "/includecollector_external2.h")), + Contains(id(TESTDATA_DIR "/includecollector_indirect_external.h")), + Contains(id(TESTDATA_DIR "/includecollector_indirect_external2.h")))); } TEST_F(IncludeCollector, DoesNotIncludesInternalHeader) @@ -86,7 +101,9 @@ TEST_F(IncludeCollector, NoDuplicate) ASSERT_THAT(collector.takeIncludeIds(), UnorderedElementsAre(id(TESTDATA_DIR "/includecollector_external1.h"), id(TESTDATA_DIR "/includecollector_external2.h"), - id(TESTDATA_DIR "/includecollector_external3.h"))); + id(TESTDATA_DIR "/includecollector_external3.h"), + id(TESTDATA_DIR "/includecollector_indirect_external.h"), + id(TESTDATA_DIR "/includecollector_indirect_external2.h"))); } TEST_F(IncludeCollector, IncludesAreSorted) @@ -94,7 +111,7 @@ TEST_F(IncludeCollector, IncludesAreSorted) collector.collectIncludes(); ASSERT_THAT(collector.takeIncludeIds(), - SizeIs(3)); + SizeIs(5)); } TEST_F(IncludeCollector, If) @@ -116,7 +133,9 @@ TEST_F(IncludeCollector, LocalPath) ASSERT_THAT(emptyCollector.takeIncludeIds(), UnorderedElementsAre(id(TESTDATA_DIR "/includecollector_external1.h"), id(TESTDATA_DIR "/includecollector_external2.h"), - id(TESTDATA_DIR "/includecollector_external3.h"))); + id(TESTDATA_DIR "/includecollector_external3.h"), + id(TESTDATA_DIR "/includecollector_indirect_external.h"), + id(TESTDATA_DIR "/includecollector_indirect_external2.h"))); } TEST_F(IncludeCollector, IgnoreMissingFile) @@ -126,23 +145,53 @@ TEST_F(IncludeCollector, IgnoreMissingFile) emptyCollector.collectIncludes(); ASSERT_THAT(emptyCollector.takeIncludeIds(), + UnorderedElementsAre(id(TESTDATA_DIR "/includecollector_external1.h"), + id(TESTDATA_DIR "/includecollector_indirect_external.h"), + id(TESTDATA_DIR "/includecollector_indirect_external2.h"))); +} + +TEST_F(IncludeCollector, IncludesOnlyTopExternalHeader) +{ + collector.collectIncludes(); + + ASSERT_THAT(collector.takeTopIncludeIds(), + UnorderedElementsAre(id(TESTDATA_DIR "/includecollector_external1.h"), + id(TESTDATA_DIR "/includecollector_external2.h"), + id(TESTDATA_DIR "/includecollector_external3.h"))); +} + +TEST_F(IncludeCollector, TopIncludeInIfMacro) +{ + emptyCollector.addFile(TESTDATA_DIR, "includecollector_if.cpp", "", {"cc", "includecollector_if.cpp"}); + emptyCollector.setExcludedIncludes({"includecollector_if.cpp"}); + + emptyCollector.collectIncludes(); + + ASSERT_THAT(emptyCollector.takeTopIncludeIds(), + ElementsAre(id(TESTDATA_DIR "/includecollector_true.h"))); +} + +TEST_F(IncludeCollector, TopIncludeWithLocalPath) +{ + emptyCollector.addFile(TESTDATA_DIR, "includecollector_main.cpp", "", {"cc", "includecollector_main.cpp"}); + + emptyCollector.collectIncludes(); + + ASSERT_THAT(emptyCollector.takeTopIncludeIds(), + UnorderedElementsAre(id(TESTDATA_DIR "/includecollector_external1.h"), + id(TESTDATA_DIR "/includecollector_external2.h"), + id(TESTDATA_DIR "/includecollector_external3.h"))); +} + +TEST_F(IncludeCollector, TopIncludesIgnoreMissingFile) +{ + emptyCollector.addFile(TESTDATA_DIR, "includecollector_missingfile.cpp", "", {"cc", "includecollector_missingfile.cpp"}); + emptyCollector.setExcludedIncludes({"includecollector_missingfile.cpp"}); + + emptyCollector.collectIncludes(); + + ASSERT_THAT(emptyCollector.takeTopIncludeIds(), UnorderedElementsAre(id(TESTDATA_DIR "/includecollector_external1.h"))); } -void IncludeCollector::SetUp() -{ - collector.addFile(TESTDATA_DIR, "includecollector_main.cpp", "", {"cc", "includecollector_main.cpp"}); - collector.addFile(TESTDATA_DIR, "includecollector_main2.cpp", "", {"cc", "includecollector_main2.cpp"}); - - collector.addUnsavedFiles({{{TESTDATA_DIR, "includecollector_generated_file.h"}, "#pragma once", {}}}); - - collector.setExcludedIncludes(excludePaths.clone()); - emptyCollector.setExcludedIncludes(excludePaths.clone()); -} - -FilePathId IncludeCollector::id(const Utils::SmallStringView &path) -{ - return filePathCache.filePathId(FilePathView{path}); -} - } diff --git a/tests/unit/unittest/pchcreator-test.cpp b/tests/unit/unittest/pchcreator-test.cpp index ef17f9ac511..a2b2cf84985 100644 --- a/tests/unit/unittest/pchcreator-test.cpp +++ b/tests/unit/unittest/pchcreator-test.cpp @@ -47,6 +47,7 @@ using ClangBackEnd::ProjectPartPch; using ClangBackEnd::V2::ProjectPartContainer; using ClangBackEnd::V2::FileContainer; using ClangBackEnd::FilePath; +using ClangBackEnd::FilePathIds; using ClangBackEnd::FilePathView; using Utils::PathString; @@ -215,19 +216,27 @@ TEST_F(PchCreator, CreateProjectPartHeaderAndSources) TEST_F(PchCreatorSlowTest, CreateProjectPartPchIncludes) { + using IncludePair = decltype(creator.generateProjectPartPchIncludes(projectPart1)); + auto includeIds = creator.generateProjectPartPchIncludes(projectPart1); ASSERT_THAT(includeIds, - AllOf(Contains(id(TESTDATA_DIR "/includecollector_external1.h")), - Contains(id(TESTDATA_DIR "/includecollector_external2.h")), - Contains(id(TESTDATA_DIR "/includecollector_header2.h")))); + AllOf( + Field(&IncludePair::first, + AllOf(Contains(id(TESTDATA_DIR "/includecollector_external1.h")), + Contains(id(TESTDATA_DIR "/includecollector_external2.h")), + Contains(id(TESTDATA_DIR "/includecollector_header2.h")))), + Field(&IncludePair::second, + AllOf(Contains(id(TESTDATA_DIR "/includecollector_external1.h")), + Contains(id(TESTDATA_DIR "/includecollector_external2.h")))))); } TEST_F(PchCreatorSlowTest, CreateProjectPartPchFileContent) { - auto includes = creator.generateProjectPartPchIncludes(projectPart1); + FilePathIds topExternalIncludes; + std::tie(std::ignore, topExternalIncludes) = creator.generateProjectPartPchIncludes(projectPart1); - auto content = creator.generatePchIncludeFileContent(includes); + auto content = creator.generatePchIncludeFileContent(topExternalIncludes); ASSERT_THAT(std::string(content), AllOf(HasSubstr("#include \"" TESTDATA_DIR "/includecollector_header2.h\"\n"), @@ -237,8 +246,9 @@ TEST_F(PchCreatorSlowTest, CreateProjectPartPchFileContent) TEST_F(PchCreatorSlowTest, CreateProjectPartPchIncludeFile) { - auto includeIds = creator.generateProjectPartPchIncludes(projectPart1); - auto content = creator.generatePchIncludeFileContent(includeIds); + FilePathIds topExternalIncludes; + std::tie(std::ignore, topExternalIncludes) = creator.generateProjectPartPchIncludes(projectPart1); + auto content = creator.generatePchIncludeFileContent(topExternalIncludes); auto pchIncludeFilePath = creator.generateProjectPathPchHeaderFilePath(projectPart1); auto file = creator.generateFileWithContent(pchIncludeFilePath, content); file->open(QIODevice::ReadOnly);