diff --git a/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h b/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h index d72691a70a7..62a406f0da2 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h +++ b/src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h @@ -49,8 +49,10 @@ public: FilePathIds &sourceFiles, UsedDefines &usedDefines, FilePathCachingInterface &filePathCache, - const clang::SourceManager &sourceManager) + const clang::SourceManager &sourceManager, + std::shared_ptr &&preprocessor) : SymbolsVisitorBase(filePathCache, sourceManager), + m_preprocessor(std::move(preprocessor)), m_symbolEntries(symbolEntries), m_sourceLocationEntries(sourceLocationEntries), m_sourceFiles(sourceFiles), @@ -114,9 +116,7 @@ public: void MacroDefined(const clang::Token ¯oNameToken, const clang::MacroDirective *macroDirective) override { - addMacroAsSymbol(macroNameToken, - firstMacroInfo(macroDirective), - SymbolType::MacroDefinition); + addMacroAsSymbol(macroNameToken, firstMacroInfo(macroDirective), SymbolType::MacroDefinition); } void MacroUndefined(const clang::Token ¯oNameToken, @@ -139,19 +139,66 @@ public: SymbolType::MacroUsage); } + void EndOfMainFile() override + { + filterOutHeaderGuards(); + mergeUsedDefines(); + filterOutExports(); + } + + void filterOutHeaderGuards() + { + auto partitionPoint = std::stable_partition(m_maybeUsedDefines.begin(), + m_maybeUsedDefines.end(), + [&] (const UsedDefine &usedDefine) { + llvm::StringRef id{usedDefine.defineName.data(), usedDefine.defineName.size()}; + clang::IdentifierInfo &identifierInfo = m_preprocessor->getIdentifierTable().get(id); + clang::MacroInfo *macroInfo = m_preprocessor->getMacroInfo(&identifierInfo); + return !macroInfo || !macroInfo->isUsedForHeaderGuard(); + }); + + m_maybeUsedDefines.erase(partitionPoint, m_maybeUsedDefines.end()); + } + + void filterOutExports() + { + auto partitionPoint = std::stable_partition(m_usedDefines.begin(), + m_usedDefines.end(), + [&] (const UsedDefine &usedDefine) { + return !usedDefine.defineName.contains("EXPORT"); + }); + + m_usedDefines.erase(partitionPoint, m_usedDefines.end()); + } + + void mergeUsedDefines() + { + m_usedDefines.reserve(m_usedDefines.size() + m_maybeUsedDefines.size()); + auto insertionPoint = m_usedDefines.insert(m_usedDefines.end(), + m_maybeUsedDefines.begin(), + m_maybeUsedDefines.end()); + std::inplace_merge(m_usedDefines.begin(), insertionPoint, m_usedDefines.end()); + } + + static void addUsedDefine(UsedDefine &&usedDefine, UsedDefines &usedDefines) + { + auto found = std::lower_bound(usedDefines.begin(), + usedDefines.end(), usedDefine); + + if (found == usedDefines.end() || *found != usedDefine) + usedDefines.insert(found, std::move(usedDefine)); + } + void addUsedDefine(const clang::Token ¯oNameToken, const clang::MacroDefinition ¯oDefinition) { clang::MacroInfo *macroInfo = macroDefinition.getMacroInfo(); - if (macroInfo) { - UsedDefine usedDefine{macroNameToken.getIdentifierInfo()->getName(), - filePathId(macroNameToken.getLocation())}; - auto found = std::lower_bound(m_usedDefines.begin(), - m_usedDefines.end(), usedDefine); - - if (found == m_usedDefines.end() || *found != usedDefine) - m_usedDefines.insert(found, std::move(usedDefine)); - } + UsedDefine usedDefine{macroNameToken.getIdentifierInfo()->getName(), + filePathId(macroNameToken.getLocation())}; + if (macroInfo) + addUsedDefine(std::move(usedDefine), m_usedDefines); + else + addUsedDefine(std::move(usedDefine), m_maybeUsedDefines); } static const clang::MacroInfo *firstMacroInfo(const clang::MacroDirective *macroDirective) @@ -176,7 +223,6 @@ public: clang::SourceLocation sourceLocation = macroNameToken.getLocation(); if (macroInfo && sourceLocation.isFileID()) { FilePathId fileId = filePathId(sourceLocation); - auto macroName = macroNameToken.getIdentifierInfo()->getName(); if (fileId.isValid()) { auto macroName = macroNameToken.getIdentifierInfo()->getName(); SymbolIndex globalId = toSymbolIndex(macroInfo); @@ -212,6 +258,8 @@ public: } private: + UsedDefines m_maybeUsedDefines; + std::shared_ptr m_preprocessor; SymbolEntries &m_symbolEntries; SourceLocationEntries &m_sourceLocationEntries; FilePathIds &m_sourceFiles; diff --git a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp index ad147a94cfd..317d93bdc3c 100644 --- a/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp +++ b/src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp @@ -33,12 +33,14 @@ namespace ClangBackEnd { bool CollectMacrosSourceFileCallbacks::handleBeginSource(clang::CompilerInstance &compilerInstance) { - auto callbacks = std::make_unique(m_symbolEntries, - m_sourceLocationEntries, - m_sourceFiles, - m_usedDefines, - m_filePathCache, - compilerInstance.getSourceManager()); + auto callbacks = std::make_unique( + m_symbolEntries, + m_sourceLocationEntries, + m_sourceFiles, + m_usedDefines, + m_filePathCache, + compilerInstance.getSourceManager(), + compilerInstance.getPreprocessorPtr()); compilerInstance.getPreprocessorPtr()->addPPCallbacks(std::move(callbacks)); diff --git a/tests/unit/unittest/data/symbolscollector_defines.h b/tests/unit/unittest/data/symbolscollector_defines.h index 91ee38cdf98..6cb2ee91ef9 100644 --- a/tests/unit/unittest/data/symbolscollector_defines.h +++ b/tests/unit/unittest/data/symbolscollector_defines.h @@ -33,4 +33,10 @@ void foo(MACRO_EXPANSION); #undef UN_DEFINE +#define CLASS_EXPORT + +struct CLASS_EXPORT Foo +{ +}; + #endif // SYMBOLSCOLLECTOR_DEFINES_H diff --git a/tests/unit/unittest/symbolscollector-test.cpp b/tests/unit/unittest/symbolscollector-test.cpp index b04464521ea..777d48b38d6 100644 --- a/tests/unit/unittest/symbolscollector-test.cpp +++ b/tests/unit/unittest/symbolscollector-test.cpp @@ -83,6 +83,17 @@ MATCHER_P2(HasLineColumn, line, column, && entry.lineColumn.column == column; } +MATCHER_P(HasSymbolName, symbolName, + std::string(negation ? "hasn't" : "has") + + " symbol name: " + + symbolName + ) +{ + const SymbolEntry &entry = arg.second; + + return entry.symbolName == symbolName; +} + class SymbolsCollector : public testing::Test { protected: @@ -115,8 +126,7 @@ TEST_F(SymbolsCollector, CollectSymbolName) collector.collectSymbols(); ASSERT_THAT(collector.symbols(), - Contains( - Pair(_, Field(&SymbolEntry::symbolName, "x")))); + Contains(HasSymbolName("x"))); } TEST_F(SymbolsCollector, SymbolMatchesLocation) @@ -203,8 +213,7 @@ TEST_F(SymbolsCollector, DISABLED_ON_WINDOWS(CollectInUnsavedFile)) collector.collectSymbols(); ASSERT_THAT(collector.symbols(), - Contains( - Pair(_, Field(&SymbolEntry::symbolName, "function")))); + Contains(HasSymbolName("function"))); } TEST_F(SymbolsCollector, SourceFiles) @@ -297,7 +306,7 @@ TEST_F(SymbolsCollector, DontCollectSourceFilesAfterFilesAreCleared) ASSERT_THAT(collector.sourceFiles(), IsEmpty()); } -TEST_F(SymbolsCollector, CollectDefines) +TEST_F(SymbolsCollector, CollectUsedDefinesWithExternalDefine) { auto fileId = filePathId(TESTDATA_DIR "/symbolscollector_defines.h"); collector.addFiles({fileId}, {"cc", "-DCOMPILER_ARGUMENT"}); @@ -313,6 +322,44 @@ TEST_F(SymbolsCollector, CollectDefines) Eq(UsedDefine{"COMPILER_ARGUMENT", fileId}))); } +TEST_F(SymbolsCollector, CollectUsedDefinesWithoutExternalDefine) +{ + auto fileId = filePathId(TESTDATA_DIR "/symbolscollector_defines.h"); + collector.addFiles({fileId}, {"cc"}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.usedDefines(), + ElementsAre(Eq(UsedDefine{"DEFINED", fileId}), + Eq(UsedDefine{"IF_DEFINE", fileId}), + Eq(UsedDefine{"__clang__", fileId}), + Eq(UsedDefine{"IF_NOT_DEFINE", fileId}), + Eq(UsedDefine{"MACRO_EXPANSION", fileId}), + Eq(UsedDefine{"COMPILER_ARGUMENT", fileId}))); +} + +TEST_F(SymbolsCollector, DontCollectHeaderGuards) +{ + auto fileId = filePathId(TESTDATA_DIR "/symbolscollector_defines.h"); + collector.addFiles({fileId}, {"cc"}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.usedDefines(), + Not(Contains(Eq(UsedDefine{"SYMBOLSCOLLECTOR_DEFINES_H", fileId})))); +} + +TEST_F(SymbolsCollector, DontCollectDynamicLibraryExports) +{ + auto fileId = filePathId(TESTDATA_DIR "/symbolscollector_defines.h"); + collector.addFiles({fileId}, {"cc"}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.usedDefines(), + Not(Contains(Eq(UsedDefine{"CLASS_EXPORT", fileId})))); +} + TEST_F(SymbolsCollector, CollectMacroDefinitionSourceLocation) { auto fileId = filePathId(TESTDATA_DIR "/symbolscollector_defines.h"); @@ -412,4 +459,37 @@ TEST_F(SymbolsCollector, CollectMacroUsageBuiltInSourceLocation) Contains(IsSourceLocationEntry(symbolId("__clang__"), fileId, 29, 9, SymbolType::MacroUsage))); } +TEST_F(SymbolsCollector, CollectMacroDefinitionSymbols) +{ + auto fileId = filePathId(TESTDATA_DIR "/symbolscollector_defines.h"); + collector.addFiles({fileId}, {"cc"}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.symbols(), + Contains(HasSymbolName("IF_NOT_DEFINE"))); +} + +TEST_F(SymbolsCollector, CollectMacroBuiltInSymbols) +{ + auto fileId = filePathId(TESTDATA_DIR "/symbolscollector_defines.h"); + collector.addFiles({fileId}, {"cc"}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.symbols(), + Contains(HasSymbolName("__clang__"))); +} + +TEST_F(SymbolsCollector, CollectMacroCompilerArgumentSymbols) +{ + auto fileId = filePathId(TESTDATA_DIR "/symbolscollector_defines.h"); + collector.addFiles({fileId}, {"cc", "-DCOMPILER_ARGUMENT"}); + + collector.collectSymbols(); + + ASSERT_THAT(collector.symbols(), + Contains(HasSymbolName("COMPILER_ARGUMENT"))); +} + }