diff options
| author | Friedemann Kleint <Friedemann.Kleint@qt.io> | 2020-07-07 15:52:42 +0200 |
|---|---|---|
| committer | Friedemann Kleint <Friedemann.Kleint@qt.io> | 2020-07-09 19:34:08 +0200 |
| commit | 3a0b9ebc9e2980af8dc5fbf4ac8bc6ddfd49c9d9 (patch) | |
| tree | a9a499b128090c2131db7590e934d83241d21cf8 | |
| parent | fb2dc48389c1099bef3aaed97f16ce2bd1b90fee (diff) | |
shiboken2/clangparser: Refactor code snippet extraction handling
Code snippets resulting from macro expansion have a 0 range.
Detect this first thing and return an empty snippet before
starting to convert file names and reading files.
For that purpose, use a CXFile instead of a QString
in SourceLocation. For all other cases, output a verbose warning.
Provide a function to obtain the file name from a CXFile
with caching in the builder.
Task-number: PYSIDE-1339
Task-number: PYSIDE-904
Change-Id: Ie30563f5b25d0d21b3a1ceb0c9da37cfc8d808dd
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Christian Tismer <tismer@stackless.com>
5 files changed, 104 insertions, 37 deletions
diff --git a/sources/shiboken2/ApiExtractor/clangparser/clangbuilder.cpp b/sources/shiboken2/ApiExtractor/clangparser/clangbuilder.cpp index 1eaa36540..263c0a0bb 100644 --- a/sources/shiboken2/ApiExtractor/clangparser/clangbuilder.cpp +++ b/sources/shiboken2/ApiExtractor/clangparser/clangbuilder.cpp @@ -114,16 +114,6 @@ static inline CodeModel::AccessPolicy accessPolicy(CX_CXXAccessSpecifier access) return result; } -static void setFileName(const CXCursor &cursor, _CodeModelItem *item) -{ - const SourceRange range = getCursorRange(cursor); - if (!range.first.file.isEmpty()) { // Has been observed to be 0 for invalid locations - item->setFileName(QDir::cleanPath(range.first.file)); - item->setStartPosition(int(range.first.line), int(range.first.column)); - item->setEndPosition(int(range.second.line), int(range.second.column)); - } -} - static bool isSigned(CXTypeKind kind) { switch (kind) { @@ -178,8 +168,8 @@ public: bool addClass(const CXCursor &cursor, CodeModel::ClassType t); FunctionModelItem createFunction(const CXCursor &cursor, - CodeModel::FunctionType t = CodeModel::Normal) const; - FunctionModelItem createMemberFunction(const CXCursor &cursor) const; + CodeModel::FunctionType t = CodeModel::Normal); + FunctionModelItem createMemberFunction(const CXCursor &cursor); void qualifyConstructor(const CXCursor &cursor); TypeInfo createTypeInfoHelper(const CXType &type) const; // uncashed TypeInfo createTypeInfo(const CXType &type) const; @@ -206,6 +196,8 @@ public: bool visitHeader(const char *cFileName) const; + void setFileName(const CXCursor &cursor, _CodeModelItem *item); + BaseVisitor *m_baseVisitor; CodeModel *m_model; @@ -285,7 +277,7 @@ static inline ExceptionSpecification exceptionSpecificationFromClang(int ce) } FunctionModelItem BuilderPrivate::createFunction(const CXCursor &cursor, - CodeModel::FunctionType t) const + CodeModel::FunctionType t) { QString name = getCursorSpelling(cursor); // Apply type fixes to "operator X &" -> "operator X&" @@ -334,7 +326,7 @@ static inline CodeModel::FunctionType functionTypeFromCursor(const CXCursor &cur return result; } -FunctionModelItem BuilderPrivate::createMemberFunction(const CXCursor &cursor) const +FunctionModelItem BuilderPrivate::createMemberFunction(const CXCursor &cursor) { const CodeModel::FunctionType functionType = m_currentFunctionType == CodeModel::Signal || m_currentFunctionType == CodeModel::Slot @@ -725,6 +717,17 @@ void BuilderPrivate::qualifyTypeDef(const CXCursor &typeRefCursor, const QShared } } +void BuilderPrivate::setFileName(const CXCursor &cursor, _CodeModelItem *item) +{ + const SourceRange range = getCursorRange(cursor); + QString file = m_baseVisitor->getFileName(range.first.file); + if (!file.isEmpty()) { // Has been observed to be 0 for invalid locations + item->setFileName(QDir::cleanPath(file)); + item->setStartPosition(int(range.first.line), int(range.first.column)); + item->setEndPosition(int(range.second.line), int(range.second.column)); + } +} + Builder::Builder() { d = new BuilderPrivate(this); @@ -937,7 +940,7 @@ BaseVisitor::StartTokenResult Builder::startToken(const CXCursor &cursor) kind = EnumClass; } d->m_currentEnum.reset(new _EnumModelItem(d->m_model, name)); - setFileName(cursor, d->m_currentEnum.data()); + d->setFileName(cursor, d->m_currentEnum.data()); d->m_currentEnum->setScope(d->m_scope); d->m_currentEnum->setEnumKind(kind); d->m_currentEnum->setSigned(isSigned(clang_getEnumDeclIntegerType(cursor).kind)); @@ -1024,7 +1027,7 @@ BaseVisitor::StartTokenResult Builder::startToken(const CXCursor &cursor) // in subsequent modules. NamespaceModelItem namespaceItem = parentNamespaceItem->findNamespace(name); namespaceItem.reset(new _NamespaceModelItem(d->m_model, name)); - setFileName(cursor, namespaceItem.data()); + d->setFileName(cursor, namespaceItem.data()); namespaceItem->setScope(d->m_scope); namespaceItem->setType(type); parentNamespaceItem->addNamespace(namespaceItem); diff --git a/sources/shiboken2/ApiExtractor/clangparser/clangparser.cpp b/sources/shiboken2/ApiExtractor/clangparser/clangparser.cpp index 6303d09e5..d0c5bc1b8 100644 --- a/sources/shiboken2/ApiExtractor/clangparser/clangparser.cpp +++ b/sources/shiboken2/ApiExtractor/clangparser/clangparser.cpp @@ -40,19 +40,49 @@ namespace clang { -SourceFileCache::Snippet SourceFileCache::getCodeSnippet(const CXCursor &cursor) +QString SourceFileCache::getFileName(CXFile file) +{ + auto it = m_fileNameCache.find(file); + if (it == m_fileNameCache.end()) + it = m_fileNameCache.insert(file, clang::getFileName(file)); + return it.value(); +} + +SourceFileCache::Snippet SourceFileCache::getCodeSnippet(const CXCursor &cursor, + QString *errorMessage) { Snippet result(nullptr, nullptr); + + if (errorMessage) + errorMessage->clear(); + const SourceRange range = getCursorRange(cursor); - if (range.first.file.isEmpty() || range.second.file != range.first.file) + // Quick check for equal locations: Frequently happens if the code is + // the result of a macro expansion + if (range.first == range.second) + return result; + + if (range.first.file != range.second.file) { + if (errorMessage) + *errorMessage = QStringLiteral("Range spans several files"); return result; - FileBufferCache::Iterator it = m_fileBufferCache.find(range.first.file); + } + + auto it = m_fileBufferCache.find(range.first.file); if (it == m_fileBufferCache.end()) { - QFile file(range.first.file); + const QString fileName = getFileName(range.first.file); + if (fileName.isEmpty()) { + if (errorMessage) + *errorMessage = QStringLiteral("Range has no file"); + return result; + } + QFile file(fileName); if (!file.open(QIODevice::ReadOnly)) { - qWarning().noquote().nospace() - << "Can't open " << QDir::toNativeSeparators(range.first.file) - << ": " << file.errorString(); + if (errorMessage) { + QTextStream str(errorMessage); + str << "Cannot open \"" << QDir::toNativeSeparators(fileName) + << "\": " << file.errorString(); + } return result; } it = m_fileBufferCache.insert(range.first.file, file.readAll()); @@ -60,10 +90,15 @@ SourceFileCache::Snippet SourceFileCache::getCodeSnippet(const CXCursor &cursor) const unsigned pos = range.first.offset; const unsigned end = range.second.offset; + Q_ASSERT(end > pos); const QByteArray &contents = it.value(); if (end >= unsigned(contents.size())) { - qWarning().noquote().nospace() << "Range end " << end << " is above size of " - << range.first.file << " (" << contents.size() << ')'; + if (errorMessage) { + QTextStream str(errorMessage); + str << "Range end " << end << " is above size of \"" + << QDir::toNativeSeparators(getFileName(range.first.file)) + << "\" (" << contents.size() << ')'; + } return result; } result.first = contents.constData() + pos; @@ -102,15 +137,21 @@ bool BaseVisitor::cbHandleEndToken(const CXCursor &cursor, StartTokenResult star BaseVisitor::CodeSnippet BaseVisitor::getCodeSnippet(const CXCursor &cursor) { - CodeSnippet result = m_fileCache.getCodeSnippet(cursor); - if (result.first == nullptr) - appendDiagnostic(Diagnostic(QStringLiteral("Unable to retrieve code snippet."), cursor, CXDiagnostic_Error)); + QString errorMessage; + CodeSnippet result = m_fileCache.getCodeSnippet(cursor, &errorMessage); + if (result.first == nullptr && !errorMessage.isEmpty()) { + QString message; + QTextStream str(&message); + str << "Unable to retrieve code snippet \"" << getCursorSpelling(cursor) + << "\": " << errorMessage; + appendDiagnostic(Diagnostic(message, cursor, CXDiagnostic_Error)); + } return result; } QString BaseVisitor::getCodeSnippetString(const CXCursor &cursor) { - CodeSnippet result = m_fileCache.getCodeSnippet(cursor); + CodeSnippet result = getCodeSnippet(cursor); return result.first != nullptr ? QString::fromUtf8(result.first, int(result.second - result.first)) : QString(); diff --git a/sources/shiboken2/ApiExtractor/clangparser/clangparser.h b/sources/shiboken2/ApiExtractor/clangparser/clangparser.h index 4248be853..825de331c 100644 --- a/sources/shiboken2/ApiExtractor/clangparser/clangparser.h +++ b/sources/shiboken2/ApiExtractor/clangparser/clangparser.h @@ -45,12 +45,15 @@ class SourceFileCache { public: using Snippet = QPair<const char *, const char *>; - Snippet getCodeSnippet(const CXCursor &cursor); + Snippet getCodeSnippet(const CXCursor &cursor, QString *errorMessage = nullptr); + QString getFileName(CXFile file); private: - using FileBufferCache = QHash<QString, QByteArray>; + using FileBufferCache = QHash<CXFile, QByteArray>; + using FileNameCache = QHash<CXFile, QString>; FileBufferCache m_fileBufferCache; + FileNameCache m_fileNameCache; }; class BaseVisitor { @@ -74,6 +77,8 @@ public: StartTokenResult cbHandleStartToken(const CXCursor &cursor); bool cbHandleEndToken(const CXCursor &cursor, StartTokenResult startResult); + QString getFileName(CXFile file) { return m_fileCache.getFileName(file); } + CodeSnippet getCodeSnippet(const CXCursor &cursor); QString getCodeSnippetString(const CXCursor &cursor); diff --git a/sources/shiboken2/ApiExtractor/clangparser/clangutils.cpp b/sources/shiboken2/ApiExtractor/clangparser/clangutils.cpp index df2476100..6bf2e3ab0 100644 --- a/sources/shiboken2/ApiExtractor/clangparser/clangutils.cpp +++ b/sources/shiboken2/ApiExtractor/clangparser/clangutils.cpp @@ -60,15 +60,25 @@ QtCompatHashFunctionType qHash(const CXType &ct, QtCompatHashFunctionType seed) namespace clang { +bool SourceLocation::equals(const SourceLocation &rhs) const +{ + return file == rhs.file && offset == rhs.offset; +} + SourceLocation getExpansionLocation(const CXSourceLocation &location) { SourceLocation result; - CXFile file; // void * - clang_getExpansionLocation(location, &file, &result.line, &result.column, &result.offset); + clang_getExpansionLocation(location, &result.file, &result.line, &result.column, &result.offset); + return result; +} + +QString getFileName(CXFile file) +{ + QString result; const CXString cxFileName = clang_getFileName(file); // Has been observed to be 0 for invalid locations if (const char *cFileName = clang_getCString(cxFileName)) - result.file = QString::fromUtf8(cFileName); + result = QString::fromUtf8(cFileName); clang_disposeString(cxFileName); return result; } @@ -226,7 +236,7 @@ QDebug operator<<(QDebug s, const SourceLocation &l) QDebugStateSaver saver(s); s.nospace(); s.noquote(); - s << QDir::toNativeSeparators(l.file) << ':' << l.line; + s << QDir::toNativeSeparators(clang::getFileName(l.file)) << ':' << l.line; if (l.column) s << ':' << l.column; return s; diff --git a/sources/shiboken2/ApiExtractor/clangparser/clangutils.h b/sources/shiboken2/ApiExtractor/clangparser/clangutils.h index 5f005bd5d..41d0af460 100644 --- a/sources/shiboken2/ApiExtractor/clangparser/clangutils.h +++ b/sources/shiboken2/ApiExtractor/clangparser/clangutils.h @@ -62,16 +62,24 @@ inline bool isCursorValid(const CXCursor &c) return c.kind < CXCursor_FirstInvalid || c.kind > CXCursor_LastInvalid; } +QString getFileName(CXFile file); // Uncached,see BaseVisitor for a cached version + struct SourceLocation { - int compare(const SourceLocation &rhs) const; + bool equals(const SourceLocation &rhs) const; - QString file; + CXFile file; unsigned line = 0; unsigned column = 0; unsigned offset = 0; }; +inline bool operator==(const SourceLocation &l1, const SourceLocation &l2) +{ return l1.equals(l2); } + +inline bool operator!=(const SourceLocation &l1, const SourceLocation &l2) +{ return !l1.equals(l2); } + SourceLocation getExpansionLocation(const CXSourceLocation &location); using SourceRange =QPair<SourceLocation, SourceLocation>; |