From 1ac63f015818ca6bc381e0867199cc06e4acf156 Mon Sep 17 00:00:00 2001 From: malte_langkabel Date: Wed, 9 Aug 2017 10:59:47 +0200 Subject: [PATCH] logic: cxx indexer fixes * fixed comment locations have been saved to wrong database table * fixed usage of tryGetRealPathName on windows * used real filename in PreprocessorCallbacks, DiagnosticsConsumer and CommentHandler * fixed crash in IncludeValidation when only one source file was checked --- .../storage/sqlite/SqliteIndexStorage.cpp | 2 +- .../data/parser/cxx/CommentHandler.cpp | 29 +++++--- .../data/parser/cxx/CxxDiagnosticConsumer.cpp | 15 ++-- .../data/parser/cxx/PreprocessorCallbacks.cpp | 69 +++++++++++-------- .../data/parser/cxx/utilityCxxAstVisitor.cpp | 12 +++- .../data/parser/cxx/utilityCxxAstVisitor.h | 2 +- src/lib_cxx/project/IncludeValidation.cpp | 2 +- 7 files changed, 85 insertions(+), 46 deletions(-) diff --git a/src/lib/data/storage/sqlite/SqliteIndexStorage.cpp b/src/lib/data/storage/sqlite/SqliteIndexStorage.cpp index 78f342a8..037937d3 100644 --- a/src/lib/data/storage/sqlite/SqliteIndexStorage.cpp +++ b/src/lib/data/storage/sqlite/SqliteIndexStorage.cpp @@ -1027,7 +1027,7 @@ void SqliteIndexStorage::setupPrecompiledStatements() "LIMIT 1;" ); m_insertCommentLocationStmt = m_database.compileStatement( - "INSERT INTO source_location(id, file_node_id, start_line, start_column, end_line, end_column) VALUES(NULL, ?, ?, ?, ?, ?);" + "INSERT INTO comment_location(id, file_node_id, start_line, start_column, end_line, end_column) VALUES(NULL, ?, ?, ?, ?, ?);" ); m_checkErrorExistsStmt = m_database.compileStatement( "SELECT id FROM error WHERE " diff --git a/src/lib_cxx/data/parser/cxx/CommentHandler.cpp b/src/lib_cxx/data/parser/cxx/CommentHandler.cpp index 7eb9a0e7..d9febfb5 100644 --- a/src/lib_cxx/data/parser/cxx/CommentHandler.cpp +++ b/src/lib_cxx/data/parser/cxx/CommentHandler.cpp @@ -1,5 +1,6 @@ #include "data/parser/cxx/CommentHandler.h" +#include "data/parser/cxx/utilityCxxAstVisitor.h" #include "data/parser/ParseLocation.h" #include "data/parser/ParserClient.h" #include "utility/file/FileRegister.h" @@ -25,16 +26,26 @@ bool CommentHandler::HandleComment(clang::Preprocessor& preprocessor, clang::Sou const clang::PresumedLoc& presumedBegin = sourceManager.getPresumedLoc(sourceRange.getBegin(), false); const clang::PresumedLoc& presumedEnd = sourceManager.getPresumedLoc(sourceRange.getEnd(), false); - FilePath filePath = m_canonicalFilePathCache->getValue(presumedBegin.getFilename()); - if (m_fileRegister->hasFilePath(filePath) && !m_fileRegister->fileIsIndexed(filePath)) + clang::FileID fileId = sourceManager.getFileID(sourceRange.getBegin()); + + // find the location file + if (!fileId.isInvalid()) { - m_client->onCommentParsed(ParseLocation( - filePath, - presumedBegin.getLine(), - presumedBegin.getColumn(), - presumedEnd.getLine(), - presumedEnd.getColumn() - )); + const clang::FileEntry* fileEntry = sourceManager.getFileEntryForID(fileId); + if (fileEntry != NULL) + { + FilePath filePath = m_canonicalFilePathCache->getValue(utility::getFileNameOfFileEntry(fileEntry)); + if (m_fileRegister->hasFilePath(filePath) && !m_fileRegister->fileIsIndexed(filePath)) + { + m_client->onCommentParsed(ParseLocation( + filePath, + presumedBegin.getLine(), + presumedBegin.getColumn(), + presumedEnd.getLine(), + presumedEnd.getColumn() + )); + } + } } return false; diff --git a/src/lib_cxx/data/parser/cxx/CxxDiagnosticConsumer.cpp b/src/lib_cxx/data/parser/cxx/CxxDiagnosticConsumer.cpp index 47bd6df3..6aec559c 100644 --- a/src/lib_cxx/data/parser/cxx/CxxDiagnosticConsumer.cpp +++ b/src/lib_cxx/data/parser/cxx/CxxDiagnosticConsumer.cpp @@ -3,6 +3,7 @@ #include "clang/Basic/SourceManager.h" #include "clang/Tooling/Tooling.h" +#include "data/parser/cxx/utilityCxxAstVisitor.h" #include "data/parser/ParseLocation.h" #include "data/parser/ParserClient.h" #include "utility/file/FileRegister.h" @@ -74,11 +75,17 @@ void CxxDiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level lev if (info.getLocation().isValid() && info.hasSourceManager()) { const clang::SourceManager& sourceManager = info.getSourceManager(); - clang::PresumedLoc presumedLocation = sourceManager.getPresumedLoc(info.getLocation()); + { + const clang::PresumedLoc presumedLocation = sourceManager.getPresumedLoc(info.getLocation()); + line = presumedLocation.getLine(); + column = presumedLocation.getColumn(); + } - filePath = clang::tooling::getAbsolutePath(presumedLocation.getFilename()); - line = presumedLocation.getLine(); - column = presumedLocation.getColumn(); + const clang::FileEntry *fileEntry = sourceManager.getFileEntryForID(sourceManager.getFileID(info.getLocation())); + if (fileEntry) + { + filePath = m_canonicalFilePathCache->getValue(utility::getFileNameOfFileEntry(fileEntry)).str(); + } } ParseLocation location(m_canonicalFilePathCache->getValue(filePath), line, column); diff --git a/src/lib_cxx/data/parser/cxx/PreprocessorCallbacks.cpp b/src/lib_cxx/data/parser/cxx/PreprocessorCallbacks.cpp index 78b4860b..82a3f4fd 100644 --- a/src/lib_cxx/data/parser/cxx/PreprocessorCallbacks.cpp +++ b/src/lib_cxx/data/parser/cxx/PreprocessorCallbacks.cpp @@ -152,13 +152,19 @@ ParseLocation PreprocessorCallbacks::getParseLocation(const clang::Token& macroN const clang::SourceLocation& location = m_sourceManager.getSpellingLoc(macroNameTok.getLocation()); const clang::SourceLocation& endLocation = m_sourceManager.getSpellingLoc(macroNameTok.getEndLoc()); - return ParseLocation( - m_canonicalFilePathCache->getValue(m_sourceManager.getFilename(location).str()), - m_sourceManager.getSpellingLineNumber(location), - m_sourceManager.getSpellingColumnNumber(location), - m_sourceManager.getSpellingLineNumber(endLocation), - m_sourceManager.getSpellingColumnNumber(endLocation) - 1 - ); + const clang::FileEntry *fileEntry = m_sourceManager.getFileEntryForID(m_sourceManager.getFileID(location)); + if (fileEntry) + { + return ParseLocation( + m_canonicalFilePathCache->getValue(utility::getFileNameOfFileEntry(fileEntry)), + m_sourceManager.getSpellingLineNumber(location), + m_sourceManager.getSpellingColumnNumber(location), + m_sourceManager.getSpellingLineNumber(endLocation), + m_sourceManager.getSpellingColumnNumber(endLocation) - 1 + ); + } + + return ParseLocation(); } ParseLocation PreprocessorCallbacks::getParseLocation(const clang::MacroInfo* macroInfo) const @@ -166,30 +172,39 @@ ParseLocation PreprocessorCallbacks::getParseLocation(const clang::MacroInfo* ma clang::SourceLocation location = macroInfo->getDefinitionLoc(); clang::SourceLocation endLocation = macroInfo->getDefinitionEndLoc(); - return ParseLocation( - m_canonicalFilePathCache->getValue(m_sourceManager.getFilename(location).str()), - m_sourceManager.getSpellingLineNumber(location), - m_sourceManager.getSpellingColumnNumber(location), - m_sourceManager.getSpellingLineNumber(endLocation), - m_sourceManager.getSpellingColumnNumber(endLocation) - 1 - ); + const clang::FileEntry *fileEntry = m_sourceManager.getFileEntryForID(m_sourceManager.getFileID(location)); + if (fileEntry) + { + return ParseLocation( + m_canonicalFilePathCache->getValue(utility::getFileNameOfFileEntry(fileEntry)), + m_sourceManager.getSpellingLineNumber(location), + m_sourceManager.getSpellingColumnNumber(location), + m_sourceManager.getSpellingLineNumber(endLocation), + m_sourceManager.getSpellingColumnNumber(endLocation) - 1 + ); + } + + return ParseLocation(); } ParseLocation PreprocessorCallbacks::getParseLocation(const clang::SourceRange& sourceRange) const { - if (sourceRange.isInvalid()) + if (sourceRange.isValid()) { - return ParseLocation(); + const clang::PresumedLoc& presumedBegin = m_sourceManager.getPresumedLoc(sourceRange.getBegin(), false); + const clang::PresumedLoc& presumedEnd = m_sourceManager.getPresumedLoc(sourceRange.getEnd(), false); + + const clang::FileEntry *fileEntry = m_sourceManager.getFileEntryForID(m_sourceManager.getFileID(sourceRange.getBegin())); + if (fileEntry) + { + return ParseLocation( + m_canonicalFilePathCache->getValue(utility::getFileNameOfFileEntry(fileEntry)), + presumedBegin.getLine(), + presumedBegin.getColumn(), + presumedEnd.getLine(), + presumedEnd.getColumn() - 1 + ); + } } - - const clang::PresumedLoc& presumedBegin = m_sourceManager.getPresumedLoc(sourceRange.getBegin(), false); - const clang::PresumedLoc& presumedEnd = m_sourceManager.getPresumedLoc(sourceRange.getEnd(), false); - - return ParseLocation( - m_canonicalFilePathCache->getValue(presumedBegin.getFilename()), - presumedBegin.getLine(), - presumedBegin.getColumn(), - presumedEnd.getLine(), - presumedEnd.getColumn() - 1 - ); + return ParseLocation(); } diff --git a/src/lib_cxx/data/parser/cxx/utilityCxxAstVisitor.cpp b/src/lib_cxx/data/parser/cxx/utilityCxxAstVisitor.cpp index b7cc2fa1..b83c5413 100644 --- a/src/lib_cxx/data/parser/cxx/utilityCxxAstVisitor.cpp +++ b/src/lib_cxx/data/parser/cxx/utilityCxxAstVisitor.cpp @@ -3,6 +3,7 @@ #include #include +#include "utility/file/FilePath.h" bool utility::isImplicit(const clang::Decl* d) { @@ -72,12 +73,17 @@ SymbolKind utility::convertTagKind(clang::TagTypeKind tagKind) } } -clang::StringRef utility::getFileNameOfFileEntry(const clang::FileEntry* entry) +std::string utility::getFileNameOfFileEntry(const clang::FileEntry* entry) { - clang::StringRef fileName = entry->tryGetRealPathName(); - if (!fileName.size()) + std::string fileName = entry->tryGetRealPathName(); + if (fileName.empty()) { fileName = entry->getName(); } + else + { + fileName = FilePath(entry->getName().str()).parentDirectory().concat(FilePath(FilePath(fileName).fileName())).str(); + } + return fileName; } diff --git a/src/lib_cxx/data/parser/cxx/utilityCxxAstVisitor.h b/src/lib_cxx/data/parser/cxx/utilityCxxAstVisitor.h index a033ed31..a8b98dcd 100644 --- a/src/lib_cxx/data/parser/cxx/utilityCxxAstVisitor.h +++ b/src/lib_cxx/data/parser/cxx/utilityCxxAstVisitor.h @@ -11,7 +11,7 @@ namespace utility bool isImplicit(const clang::Decl* d); AccessKind convertAccessSpecifier(clang::AccessSpecifier access); SymbolKind convertTagKind(clang::TagTypeKind tagKind); - clang::StringRef getFileNameOfFileEntry(const clang::FileEntry* entry); + std::string getFileNameOfFileEntry(const clang::FileEntry* entry); } #endif // UTILITY_CXX_AST_VISITOR_H diff --git a/src/lib_cxx/project/IncludeValidation.cpp b/src/lib_cxx/project/IncludeValidation.cpp index 575f0839..f27b96d0 100644 --- a/src/lib_cxx/project/IncludeValidation.cpp +++ b/src/lib_cxx/project/IncludeValidation.cpp @@ -24,7 +24,7 @@ std::vector IncludeValidation::getUnresolvedIncludeDirectives( std::set processedFilePaths; std::set unresolvedIncludeDirectives; - quantileCount = std::min(quantileCount, sourceFilePaths.size()); + quantileCount = std::max(1, std::min(quantileCount, sourceFilePaths.size())); std::vector> quantiles; for (size_t i = 0; i < quantileCount; i++)