shiboken6/Clang parser: Refactor include checking

In the base class BaseVisitor of the clang parser, check whether the
file has changed by comparing the CXFile. If it has changed, convert
the file name to a QString and call visitLocation().

The code is then faster since the file name checks are only performed
when the CXFile changes. The check code operating on QString becomes
simpler and can be extended more easily.

Task-number: PYSIDE-802
Task-number: PYSIDE-1660
Pick-to: 6.2
Change-Id: I1dd22ef91a4a7b96d05507b43511313507281fd6
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

6 files changed, 81 insertions, 76 deletions

diff --git a/sources/shiboken6/ApiExtractor/clangparser/clangbuilder.cpp b/sources/shiboken6/ApiExtractor/clangparser/clangbuilder.cpp
index 1cd9937fa..6f5a4d47f 100644
--- a/sources/shiboken6/ApiExtractor/clangparser/clangbuilder.cpp
+++ b/sources/shiboken6/ApiExtractor/clangparser/clangbuilder.cpp
@@ -206,7 +206,7 @@ public:
     template <class Item>
     void qualifyTypeDef(const CXCursor &typeRefCursor, const QSharedPointer<Item> &item) const;
 
-    bool visitHeader(const char *cFileName) const;
+    bool visitHeader(const QString &fileName) const;
 
     void setFileName(const CXCursor &cursor, _CodeModelItem *item);
 
@@ -230,8 +230,8 @@ public:
     ArgumentModelItem m_currentArgument;
     VariableModelItem m_currentField;
     TemplateTypeAliasModelItem m_currentTemplateTypeAlias;
-    QByteArrayList m_systemIncludes; // files, like "memory"
-    QByteArrayList m_systemIncludePaths; // paths, like "/usr/include/Qt/"
+    QStringList m_systemIncludes; // files, like "memory"
+    QStringList m_systemIncludePaths; // paths, like "/usr/include/Qt/"
     QString m_usingTypeRef; // Base classes in "using Base::member;"
     bool m_withinUsingDeclaration = false;
 
@@ -809,101 +809,65 @@ Builder::~Builder()
     delete d;
 }
 
-static const char *cBaseName(const char *fileName)
+static QString baseName(QString path)
 {
-    const char *lastSlash = std::strrchr(fileName, '/');
+    qsizetype lastSlash = path.lastIndexOf(u'/');
 #ifdef Q_OS_WIN
-    if (lastSlash == nullptr)
-        lastSlash = std::strrchr(fileName, '\\');
+    if (lastSlash < 0)
+        lastSlash = path.lastIndexOf(u'\\');
 #endif
-    return lastSlash != nullptr ? (lastSlash + 1) : fileName;
+    if (lastSlash > 0)
+        path.remove(0, lastSlash + 1);
+    return path;
 }
 
-static inline bool cCompareFileName(const char *f1, const char *f2)
-{
-#ifdef Q_OS_WIN
-   return _stricmp(f1, f2) == 0;
-#else
-    return std::strcmp(f1, f2) == 0;
-#endif
-}
-
-#ifdef Q_OS_UNIX
-template<size_t N>
-static bool cStringStartsWith(const char *str, const char (&prefix)[N])
-{
-    return std::strncmp(prefix, str, N - 1) == 0;
-}
-#endif
-
-static bool cStringStartsWith(const char *str, const QByteArray &prefix)
-{
-    return std::strncmp(prefix.constData(), str, int(prefix.size())) == 0;
-}
-
-bool BuilderPrivate::visitHeader(const char *cFileName) const
+bool BuilderPrivate::visitHeader(const QString &fileName) const
 {
     // Resolve OpenGL typedefs although the header is considered a system header.
-    const char *baseName = cBaseName(cFileName);
-    if (cCompareFileName(baseName, "gl.h")
-        || cCompareFileName(baseName, "stdint.h") // Windows: int32_t, uint32_t
-        || cCompareFileName(baseName, "stddef.h")) { // size_t
+    const QString baseName = clang::baseName(fileName);
+    if (baseName == u"gl.h"
+        || baseName == u"stdint.h" // Windows: int32_t, uint32_t
+        || baseName == u"stddef.h") { // size_t
         return true;
     }
 #ifdef Q_OS_LINUX
-    if (cStringStartsWith(cFileName, "/usr/include/stdlib.h")
-        || cCompareFileName(baseName, "types.h")
-        || cCompareFileName(baseName, "stdint-intn.h") // int32_t
-        || cCompareFileName(baseName, "stdint-uintn.h")) { // uint32_t
+    if (fileName == u"/usr/include/stdlib.h"
+        || baseName == u"types.h"
+        || baseName == u"stdint-intn.h" // int32_t
+        || baseName == u"stdint-uintn.h") { // uint32_t
         return true;
     }
 #endif // Q_OS_LINUX
 #ifdef Q_OS_MACOS
     // Parse the following system headers to get the correct typdefs for types like
     // int32_t, which are used in the macOS implementation of OpenGL framework.
-    if (cCompareFileName(baseName, "gltypes.h")
-        || cStringStartsWith(cFileName, "/usr/include/_types")
-        || cStringStartsWith(cFileName, "/usr/include/_types")
-        || cStringStartsWith(cFileName, "/usr/include/sys/_types")) {
+    if (baseName == u"gltypes.h"
+        || fileName.startsWith(u"/usr/include/_types")
+        || fileName.startsWith(u"/usr/include/_types")
+        || fileName.startsWith(u"/usr/include/sys/_types")) {
         return true;
     }
 #endif // Q_OS_MACOS
-    if (baseName) {
-        for (const auto &systemInclude : m_systemIncludes) {
-            if (systemInclude == baseName)
-                return true;
-        }
+    for (const auto &systemInclude : m_systemIncludes) {
+        if (systemInclude == baseName)
+            return true;
     }
     for (const auto &systemIncludePath : m_systemIncludePaths) {
-        if (cStringStartsWith(cFileName, systemIncludePath))
+        if (fileName.startsWith(systemIncludePath))
             return true;
     }
     return false;
 }
 
-bool Builder::visitLocation(const CXSourceLocation &location) const
+bool Builder::visitLocation(const QString &fileName, LocationType locationType) const
 {
-    if (clang_Location_isInSystemHeader(location) == 0)
-        return true;
-    CXFile file; // void *
-    unsigned line;
-    unsigned column;
-    unsigned offset;
-    clang_getExpansionLocation(location, &file, &line, &column, &offset);
-    const CXString cxFileName = clang_getFileName(file);
-    // Has been observed to be 0 for invalid locations
-    bool result = false;
-    if (const char *cFileName = clang_getCString(cxFileName)) {
-        result = d->visitHeader(cFileName);
-        clang_disposeString(cxFileName);
-    }
-    return result;
+    return locationType != LocationType::System || d->visitHeader(fileName);
 }
 
-void Builder::setSystemIncludes(const QByteArrayList &systemIncludes)
+void Builder::setSystemIncludes(const QStringList &systemIncludes)
 {
     for (const auto &i : systemIncludes) {
-        if (i.endsWith('/'))
+        if (i.endsWith(u'/'))
             d->m_systemIncludePaths.append(i);
         else
             d->m_systemIncludes.append(i);
diff --git a/sources/shiboken6/ApiExtractor/clangparser/clangbuilder.h b/sources/shiboken6/ApiExtractor/clangparser/clangbuilder.h
index dc37dff0f..c84ffa356 100644
--- a/sources/shiboken6/ApiExtractor/clangparser/clangbuilder.h
+++ b/sources/shiboken6/ApiExtractor/clangparser/clangbuilder.h
@@ -44,9 +44,9 @@ public:
     Builder();
     ~Builder();
 
-    void setSystemIncludes(const QByteArrayList &systemIncludes);
+    void setSystemIncludes(const QStringList &systemIncludes);
 
-    bool visitLocation(const CXSourceLocation &location) const override;
+    bool visitLocation(const QString &fileName, LocationType locationType) const override;
 
     StartTokenResult startToken(const CXCursor &cursor) override;
     bool endToken(const CXCursor &cursor) override;
diff --git a/sources/shiboken6/ApiExtractor/clangparser/clangparser.cpp b/sources/shiboken6/ApiExtractor/clangparser/clangparser.cpp
index 8cf35641b..15be8f5a4 100644
--- a/sources/shiboken6/ApiExtractor/clangparser/clangparser.cpp
+++ b/sources/shiboken6/ApiExtractor/clangparser/clangparser.cpp
@@ -108,9 +108,9 @@ std::string_view SourceFileCache::getCodeSnippet(const CXCursor &cursor,
 BaseVisitor::BaseVisitor() = default;
 BaseVisitor::~BaseVisitor() = default;
 
-bool BaseVisitor::visitLocation(const CXSourceLocation &location) const
+bool BaseVisitor::visitLocation(const QString &, LocationType locationType) const
 {
-    return clang_Location_isFromMainFile(location) != 0;
+    return locationType != LocationType::System;
 }
 
 BaseVisitor::StartTokenResult BaseVisitor::cbHandleStartToken(const CXCursor &cursor)
@@ -148,6 +148,34 @@ std::string_view BaseVisitor::getCodeSnippet(const CXCursor &cursor)
     return result;
 }
 
+bool BaseVisitor::_handleVisitLocation(const CXSourceLocation &location)
+{
+    CXFile cxFile; // void *
+    unsigned line;
+    unsigned column;
+    unsigned offset;
+    clang_getExpansionLocation(location, &cxFile, &line, &column, &offset);
+
+    if (cxFile == m_currentCxFile) // Same file?
+        return m_visitCurrent;
+
+    const QString fileName = getFileName(cxFile);
+
+    LocationType locationType = LocationType::Unknown;
+    if (!fileName.isEmpty()) {
+        if (clang_Location_isFromMainFile(location) != 0)
+            locationType = LocationType::Main;
+        else if (clang_Location_isInSystemHeader(location) != 0)
+            locationType = LocationType::System;
+        else
+            locationType = LocationType::Other;
+    }
+
+    m_currentCxFile = cxFile;
+    m_visitCurrent = visitLocation(fileName, locationType);
+    return m_visitCurrent;
+}
+
 QString BaseVisitor::getCodeSnippetString(const CXCursor &cursor)
 {
     const std::string_view result = getCodeSnippet(cursor);
@@ -162,7 +190,7 @@ static CXChildVisitResult
     auto *bv = reinterpret_cast<BaseVisitor *>(clientData);
 
     const CXSourceLocation location = clang_getCursorLocation(cursor);
-    if (!bv->visitLocation(location))
+    if (!bv->_handleVisitLocation(location))
         return CXChildVisit_Continue;
 
     const BaseVisitor::StartTokenResult startResult = bv->cbHandleStartToken(cursor);
diff --git a/sources/shiboken6/ApiExtractor/clangparser/clangparser.h b/sources/shiboken6/ApiExtractor/clangparser/clangparser.h
index d95ada602..81b40ae6f 100644
--- a/sources/shiboken6/ApiExtractor/clangparser/clangparser.h
+++ b/sources/shiboken6/ApiExtractor/clangparser/clangparser.h
@@ -56,6 +56,14 @@ private:
     FileNameCache m_fileNameCache;
 };
 
+enum class LocationType
+{
+    Main, // Main header parsed for bindings
+    Other, // A header parsed for bindings
+    System,  // A system header
+    Unknown  // Clang internal
+};
+
 class BaseVisitor {
     Q_DISABLE_COPY(BaseVisitor)
 public:
@@ -68,7 +76,7 @@ public:
 
     // Whether location should be visited.
     // defaults to clang_Location_isFromMainFile()
-    virtual bool visitLocation(const CXSourceLocation &location) const;
+    virtual bool visitLocation(const QString &fileName, LocationType locationType) const;
 
     virtual StartTokenResult startToken(const CXCursor &cursor) = 0;
     virtual bool endToken(const CXCursor &cursor) = 0;
@@ -84,9 +92,14 @@ public:
     void setDiagnostics(const Diagnostics &d);
     void appendDiagnostic(const Diagnostic &d);
 
+    // For usage by the parser
+    bool _handleVisitLocation( const CXSourceLocation &location);
+
 private:
     SourceFileCache m_fileCache;
     Diagnostics m_diagnostics;
+    CXFile m_currentCxFile{};
+    bool m_visitCurrent = true;
 };
 
 bool parse(const QByteArrayList  &clangArgs,
diff --git a/sources/shiboken6/ApiExtractor/typedatabase.cpp b/sources/shiboken6/ApiExtractor/typedatabase.cpp
index 7c3fac762..4606848c3 100644
--- a/sources/shiboken6/ApiExtractor/typedatabase.cpp
+++ b/sources/shiboken6/ApiExtractor/typedatabase.cpp
@@ -251,7 +251,7 @@ IncludeList TypeDatabase::extraIncludes(const QString& className) const
 
 void TypeDatabase::addSystemInclude(const QString &name)
 {
-    m_systemIncludes.append(name.toUtf8());
+    m_systemIncludes.append(name);
 }
 
 // Add a lookup for the short name excluding inline namespaces
diff --git a/sources/shiboken6/ApiExtractor/typedatabase.h b/sources/shiboken6/ApiExtractor/typedatabase.h
index aaf174081..2eab21f32 100644
--- a/sources/shiboken6/ApiExtractor/typedatabase.h
+++ b/sources/shiboken6/ApiExtractor/typedatabase.h
@@ -121,7 +121,7 @@ public:
 
     IncludeList extraIncludes(const QString &className) const;
 
-    const QByteArrayList &systemIncludes() const { return m_systemIncludes; }
+    const QStringList &systemIncludes() const { return m_systemIncludes; }
     void addSystemInclude(const QString &name);
 
     void addInlineNamespaceLookups(const NamespaceTypeEntry *n);
@@ -251,7 +251,7 @@ private:
     QList<TypeRejection> m_rejections;
 
     QStringList m_dropTypeEntries;
-    QByteArrayList m_systemIncludes;
+    QStringList m_systemIncludes;
 };
 
 #ifndef QT_NO_DEBUG_STREAM