ClassView: Optimize operator== of internal data

Make all of the fields of SymbolLocation and SymbolInformation
const members. Optimize a bit operator== of these structures:
check first if hashes are non-equal - in this case return
false early. Fix hash type of SymbolInformation for porting
to Qt6 purpose.

Task-number: QTCREATORBUG-25317
Task-number: QTCREATORBUG-24098
Change-Id: I769f99ff3157093e9f10ee3929bc7f6eb83f34e3
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/classview/classviewmanager.cpp

@@ -317,8 +317,7 @@ void Manager::gotoLocations(const QList<QVariant> &list)
         return;
 
     // Default to first known location
-    SymbolLocation loc = *locations.constBegin();
+    auto locationIt = locations.constBegin();
-
     if (locations.size() > 1) {
         // The symbol has multiple locations. Check if we are already at one location,
         // and if so, cycle to the "next" one
@@ -329,20 +328,19 @@ void Manager::gotoLocations(const QList<QVariant> &list)
             int line;
             int column;
             textEditor->convertPosition(textEditor->position(), &line, &column);
-            SymbolLocation current(fileName, line, column);
+            const SymbolLocation current(fileName, line, column);
-            QSet<SymbolLocation>::const_iterator it = locations.constFind(current);
+            if (auto it = locations.constFind(current), end = locations.constEnd(); it != end) {
-            QSet<SymbolLocation>::const_iterator end = locations.constEnd();
-            if (it != end) {
                 // we already are at the symbol, cycle to next location
                 ++it;
                 if (it == end)
                     it = locations.constBegin();
-                loc = *it;
+                locationIt = it;
             }
         }
     }
+    const SymbolLocation &location = *locationIt;
     // line is 1-based, column is 0-based
-    gotoLocation(loc.fileName(), loc.line(), loc.column() - 1);
+    gotoLocation(location.fileName(), location.line(), location.column() - 1);
 }
 
 /*!

src/plugins/classview/classviewparser.cpp

@@ -479,6 +479,7 @@ void Parser::requestCurrentState()
 {
     d->timer.stop();
 
+    // TODO: we need to have a fresh SessionManager data here, which we could pass to parse()
     const ParserTreeItem::ConstPtr newRoot = parse();
     {
         QWriteLocker locker(&d->rootItemLocker);

src/plugins/classview/classviewsymbolinformation.cpp

@@ -49,13 +49,16 @@ SymbolInformation::SymbolInformation() :
 }
 
 SymbolInformation::SymbolInformation(const QString &valueName, const QString &valueType,
-                                     int valueIconType) :
+                                     int valueIconType)
-    m_iconType(valueIconType),
+    : m_iconType(valueIconType)
-    m_name(valueName),
+#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
-    m_type(valueType)
+    , m_hash(qHashMulti(0, valueIconType, valueName, valueType))
+#else
+    , m_hash(qHash(qMakePair(valueIconType, qMakePair(valueName, valueType))))
+#endif
+    , m_name(valueName)
+    , m_type(valueType)
 {
-    // calculate hash
-    m_hash = qHash(qMakePair(m_iconType, qMakePair(m_name, m_type)));
 }
 
 /*!
@@ -94,7 +97,8 @@ int SymbolInformation::iconTypeSortOrder() const
 
     static QHash<int, int> sortOrder;
 
-    // initialization
+    // TODO: Check if this static initialization is OK when SymbolInformation object are
+    // instantiated in different threads.
     if (sortOrder.isEmpty()) {
         for (int i : IconSortOrder)
             sortOrder.insert(i, sortOrder.count());

src/plugins/classview/classviewsymbolinformation.h

@@ -25,6 +25,8 @@
 
 #pragma once
 
+#include <utils/porting.h>
+
 #include <QMetaType>
 #include <QString>
 
@@ -47,17 +49,17 @@ public:
     inline uint hash() const { return m_hash; }
     inline bool operator==(const SymbolInformation &other) const
     {
-        return iconType() == other.iconType() && name() == other.name()
+        return hash() == other.hash() && iconType() == other.iconType() && name() == other.name()
             && type() == other.type();
     }
 
     int iconTypeSortOrder() const;
 
 private:
-    int m_iconType; //!< icon type
+    const int m_iconType;
-    uint m_hash;    //!< precalculated hash value - to speed up qHash
+    const Utils::QHashValueType m_hash; // precalculated hash value - to speed up qHash
-    QString m_name; //!< symbol name (e.g. SymbolInformation)
+    const QString m_name;               // symbol name (e.g. SymbolInformation)
-    QString m_type; //!< symbol type (e.g. (int char))
+    const QString m_type;               // symbol type (e.g. (int char))
 
 };
 

src/plugins/classview/classviewsymbollocation.cpp

@@ -45,20 +45,16 @@ SymbolLocation::SymbolLocation() :
 {
 }
 
-SymbolLocation::SymbolLocation(QString file, int lineNumber, int columnNumber) :
+SymbolLocation::SymbolLocation(const QString &file, int lineNumber, int columnNumber)
-    m_fileName(file),
+    : m_fileName(file)
-    m_line(lineNumber),
+    , m_line(lineNumber)
-    m_column(columnNumber)
+    , m_column(qMax(columnNumber, 0))
-{
-    if (m_column < 0)
-        m_column = 0;
-
-    // pre-computate hash value
 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
-    m_hash = qHashMulti(0, m_fileName, m_line, m_column);
+    , m_hash(qHashMulti(0, m_fileName, m_line, m_column))
 #else
-    m_hash = qHash(qMakePair(m_fileName, qMakePair(m_line, m_column)));
+    , m_hash(qHash(qMakePair(m_fileName, qMakePair(m_line, m_column))))
 #endif
+{
 }
 
 } // namespace Internal

src/plugins/classview/classviewsymbollocation.h

@@ -40,7 +40,7 @@ public:
     SymbolLocation();
 
     //! Constructor
-    explicit SymbolLocation(QString file, int lineNumber = 0, int columnNumber = 0);
+    explicit SymbolLocation(const QString &file, int lineNumber = 0, int columnNumber = 0);
 
     inline const QString &fileName() const { return m_fileName; }
     inline int line() const { return m_line; }
@@ -48,15 +48,15 @@ public:
     inline Utils::QHashValueType hash() const { return m_hash; }
     inline bool operator==(const SymbolLocation &other) const
     {
-        return line() == other.line() && column() == other.column()
+        return hash() == other.hash() && line() == other.line() && column() == other.column()
             && fileName() == other.fileName();
     }
 
 private:
-    QString m_fileName; //!< file name
+    const QString m_fileName;
-    int m_line;         //!< line number
+    const int m_line;
-    int m_column;       //!< column
+    const int m_column;
-    Utils::QHashValueType m_hash;   //!< precalculated hash value for the object, to speed up qHash
+    const Utils::QHashValueType m_hash; // precalculated hash value - to speed up qHash
 };
 
 //! qHash overload for QHash/QSet