Remove QMimeMagicRule's pimpl

It didn't make the class implicitly shared, but required an
additional heap allocation on construction and copy, as it
used 'only' QScopedPointer. As it's private API the pimpl
is also not needed for BC reasons.

So inline the data members, and some trivial accessors.

As a by-product of removing the copy special member functions,
we gain nothrow move special member functions.

Interestingly, the memory layout of a QList<QMimeMagicRule>
(replacing which is the topic of a future patch) doesn't
change due to this change, because the type that formerly
fit QList very well now is too large. But copying the type
outside QList now no longer allocates memory.

Saves more than 2.5KiB in text size on optimized GCC 5.3
Linux AMD64 builds.

Change-Id: Ie3588cb5693227da6f1bfa196db924e075a750b3
Reviewed-by: David Faure <david.faure@kdab.com>

2 files changed, 95 insertions, 151 deletions

diff --git a/src/corelib/mimetypes/qmimemagicrule.cpp b/src/corelib/mimetypes/qmimemagicrule.cpp
index 1b8acc0f6e..09473caa78 100644
--- a/src/corelib/mimetypes/qmimemagicrule.cpp
+++ b/src/corelib/mimetypes/qmimemagicrule.cpp
@@ -82,36 +82,17 @@ QByteArray QMimeMagicRule::typeName(QMimeMagicRule::Type theType)
     return magicRuleTypes_string + magicRuleTypes_indices[theType];
 }
 
-class QMimeMagicRulePrivate
-{
-public:
-    bool operator==(const QMimeMagicRulePrivate &other) const;
-
-    QMimeMagicRule::Type type;
-    QByteArray value;
-    int startPos;
-    int endPos;
-    QByteArray mask;
-
-    QByteArray pattern;
-    quint32 number;
-    quint32 numberMask;
-
-    typedef bool (*MatchFunction)(const QMimeMagicRulePrivate *d, const QByteArray &data);
-    MatchFunction matchFunction;
-};
-
-bool QMimeMagicRulePrivate::operator==(const QMimeMagicRulePrivate &other) const
+bool QMimeMagicRule::operator==(const QMimeMagicRule &other) const
 {
-    return type == other.type &&
-           value == other.value &&
-           startPos == other.startPos &&
-           endPos == other.endPos &&
-           mask == other.mask &&
-           pattern == other.pattern &&
-           number == other.number &&
-           numberMask == other.numberMask &&
-           matchFunction == other.matchFunction;
+    return m_type == other.m_type &&
+           m_value == other.m_value &&
+           m_startPos == other.m_startPos &&
+           m_endPos == other.m_endPos &&
+           m_mask == other.m_mask &&
+           m_pattern == other.m_pattern &&
+           m_number == other.m_number &&
+           m_numberMask == other.m_numberMask &&
+           m_matchFunction == other.m_matchFunction;
 }
 
 // Used by both providers
@@ -164,23 +145,23 @@ bool QMimeMagicRule::matchSubstring(const char *dataPtr, int dataSize, int range
     return true;
 }
 
-static bool matchString(const QMimeMagicRulePrivate *d, const QByteArray &data)
+bool QMimeMagicRule::matchString(const QByteArray &data) const
 {
-    const int rangeLength = d->endPos - d->startPos + 1;
-    return QMimeMagicRule::matchSubstring(data.constData(), data.size(), d->startPos, rangeLength, d->pattern.size(), d->pattern.constData(), d->mask.constData());
+    const int rangeLength = m_endPos - m_startPos + 1;
+    return QMimeMagicRule::matchSubstring(data.constData(), data.size(), m_startPos, rangeLength, m_pattern.size(), m_pattern.constData(), m_mask.constData());
 }
 
 template <typename T>
-static bool matchNumber(const QMimeMagicRulePrivate *d, const QByteArray &data)
+bool QMimeMagicRule::matchNumber(const QByteArray &data) const
 {
-    const T value(d->number);
-    const T mask(d->numberMask);
+    const T value(m_number);
+    const T mask(m_numberMask);
 
-    //qDebug() << "matchNumber" << "0x" << QString::number(d->number, 16) << "size" << sizeof(T);
-    //qDebug() << "mask" << QString::number(d->numberMask, 16);
+    //qDebug() << "matchNumber" << "0x" << QString::number(m_number, 16) << "size" << sizeof(T);
+    //qDebug() << "mask" << QString::number(m_numberMask, 16);
 
-    const char *p = data.constData() + d->startPos;
-    const char *e = data.constData() + qMin(data.size() - int(sizeof(T)), d->endPos + 1);
+    const char *p = data.constData() + m_startPos;
+    const char *e = data.constData() + qMin(data.size() - int(sizeof(T)), m_endPos + 1);
     for ( ; p <= e; ++p) {
         if ((*reinterpret_cast<const T*>(p) & mask) == (value & mask))
             return true;
@@ -242,105 +223,102 @@ static inline QByteArray makePattern(const QByteArray &value)
 //  <match value="must be converted with BinHex" type="string" offset="11"/>
 //  <match value="0x9501" type="big16" offset="0:64"/>
 
-QMimeMagicRule::QMimeMagicRule(const QString &typeStr,
-                               const QByteArray &theValue,
+QMimeMagicRule::QMimeMagicRule(const QString &type,
+                               const QByteArray &value,
                                const QString &offsets,
-                               const QByteArray &theMask,
-                               QString *errorString) :
-    d(new QMimeMagicRulePrivate)
+                               const QByteArray &mask,
+                               QString *errorString)
+    : m_type(QMimeMagicRule::type(type.toLatin1())),
+      m_value(value),
+      m_mask(mask),
+      m_matchFunction(nullptr)
 {
-    d->value = theValue;
-    d->mask = theMask;
-    d->matchFunction = 0;
-
-    d->type = QMimeMagicRule::type(typeStr.toLatin1());
-    if (d->type == Invalid) {
-        *errorString = QStringLiteral("Type %s is not supported").arg(typeStr);
-    }
+    if (m_type == Invalid)
+        *errorString = QStringLiteral("Type %s is not supported").arg(type);
 
     // Parse for offset as "1" or "1:10"
     const int colonIndex = offsets.indexOf(QLatin1Char(':'));
     const QString startPosStr = colonIndex == -1 ? offsets : offsets.mid(0, colonIndex);
     const QString endPosStr   = colonIndex == -1 ? offsets : offsets.mid(colonIndex + 1);
-    if (!QMimeTypeParserBase::parseNumber(startPosStr, &d->startPos, errorString) ||
-        !QMimeTypeParserBase::parseNumber(endPosStr, &d->endPos, errorString)) {
-        d->type = Invalid;
+    if (!QMimeTypeParserBase::parseNumber(startPosStr, &m_startPos, errorString) ||
+        !QMimeTypeParserBase::parseNumber(endPosStr, &m_endPos, errorString)) {
+        m_type = Invalid;
         return;
     }
 
-    if (d->value.isEmpty()) {
-        d->type = Invalid;
+    if (m_value.isEmpty()) {
+        m_type = Invalid;
         if (errorString)
             *errorString = QLatin1String("Invalid empty magic rule value");
         return;
     }
 
-    if (d->type >= Host16 && d->type <= Byte) {
+    if (m_type >= Host16 && m_type <= Byte) {
         bool ok;
-        d->number = d->value.toUInt(&ok, 0); // autodetect
+        m_number = m_value.toUInt(&ok, 0); // autodetect base
         if (!ok) {
-            d->type = Invalid;
+            m_type = Invalid;
             if (errorString)
                 *errorString = QString::fromLatin1("Invalid magic rule value \"%1\"").arg(
-                        QString::fromLatin1(d->value));
+                        QString::fromLatin1(m_value));
             return;
         }
-        d->numberMask = !d->mask.isEmpty() ? d->mask.toUInt(&ok, 0) : 0; // autodetect
+        m_numberMask = !m_mask.isEmpty() ? m_mask.toUInt(&ok, 0) : 0; // autodetect base
     }
 
-    switch (d->type) {
+    switch (m_type) {
     case String:
-        d->pattern = makePattern(d->value);
-        d->pattern.squeeze();
-        if (!d->mask.isEmpty()) {
-            if (d->mask.size() < 4 || !d->mask.startsWith("0x")) {
-                d->type = Invalid;
+        m_pattern = makePattern(m_value);
+        m_pattern.squeeze();
+        if (!m_mask.isEmpty()) {
+            if (m_mask.size() < 4 || !m_mask.startsWith("0x")) {
+                m_type = Invalid;
                 if (errorString)
                     *errorString = QString::fromLatin1("Invalid magic rule mask \"%1\"").arg(
-                            QString::fromLatin1(d->mask));
+                            QString::fromLatin1(m_mask));
                 return;
             }
             const QByteArray &tempMask = QByteArray::fromHex(QByteArray::fromRawData(
-                                                     d->mask.constData() + 2, d->mask.size() - 2));
-            if (tempMask.size() != d->pattern.size()) {
-                d->type = Invalid;
+                                                     m_mask.constData() + 2, m_mask.size() - 2));
+            if (tempMask.size() != m_pattern.size()) {
+                m_type = Invalid;
                 if (errorString)
                     *errorString = QString::fromLatin1("Invalid magic rule mask size \"%1\"").arg(
-                            QString::fromLatin1(d->mask));
+                            QString::fromLatin1(m_mask));
                 return;
             }
-            d->mask = tempMask;
+            m_mask = tempMask;
         } else {
-            d->mask.fill(char(-1), d->pattern.size());
+            m_mask.fill(char(-1), m_pattern.size());
         }
-        d->mask.squeeze();
-        d->matchFunction = matchString;
+        m_mask.squeeze();
+        m_matchFunction = &QMimeMagicRule::matchString;
         break;
     case Byte:
-        if (d->number <= quint8(-1)) {
-            if (d->numberMask == 0)
-                d->numberMask = quint8(-1);
-            d->matchFunction = matchNumber<quint8>;
+        if (m_number <= quint8(-1)) {
+            if (m_numberMask == 0)
+                m_numberMask = quint8(-1);
+            m_matchFunction = &QMimeMagicRule::matchNumber<quint8>;
         }
         break;
     case Big16:
     case Host16:
     case Little16:
-        if (d->number <= quint16(-1)) {
-            d->number = d->type == Little16 ? qFromLittleEndian<quint16>(d->number) : qFromBigEndian<quint16>(d->number);
-            if (d->numberMask == 0)
-                d->numberMask = quint16(-1);
-            d->matchFunction = matchNumber<quint16>;
+        if (m_number <= quint16(-1)) {
+            m_number = m_type == Little16 ? qFromLittleEndian<quint16>(m_number) : qFromBigEndian<quint16>(m_number);
+            if (m_numberMask == 0)
+                m_numberMask = quint16(-1);
+            m_matchFunction = &QMimeMagicRule::matchNumber<quint16>;
         }
         break;
     case Big32:
     case Host32:
     case Little32:
-        if (d->number <= quint32(-1)) {
-            d->number = d->type == Little32 ? qFromLittleEndian<quint32>(d->number) : qFromBigEndian<quint32>(d->number);
-            if (d->numberMask == 0)
-                d->numberMask = quint32(-1);
-            d->matchFunction = matchNumber<quint32>;
+        if (m_number <= quint32(-1)) {
+            m_number = m_type == Little32 ? qFromLittleEndian<quint32>(m_number) : qFromBigEndian<quint32>(m_number);
+            if (m_numberMask == 0)
+                m_numberMask = quint32(-1);
+            m_matchFunction = &QMimeMagicRule::matchNumber<quint32>;
         }
         break;
     default:
@@ -348,65 +326,19 @@ QMimeMagicRule::QMimeMagicRule(const QString &typeStr,
     }
 }
 
-QMimeMagicRule::QMimeMagicRule(const QMimeMagicRule &other) :
-    d(new QMimeMagicRulePrivate(*other.d))
-{
-}
-
-QMimeMagicRule::~QMimeMagicRule()
-{
-}
-
-QMimeMagicRule &QMimeMagicRule::operator=(const QMimeMagicRule &other)
-{
-    *d = *other.d;
-    return *this;
-}
-
-bool QMimeMagicRule::operator==(const QMimeMagicRule &other) const
-{
-    return d == other.d ||
-           *d == *other.d;
-}
-
-QMimeMagicRule::Type QMimeMagicRule::type() const
-{
-    return d->type;
-}
-
-QByteArray QMimeMagicRule::value() const
-{
-    return d->value;
-}
-
-int QMimeMagicRule::startPos() const
-{
-    return d->startPos;
-}
-
-int QMimeMagicRule::endPos() const
-{
-    return d->endPos;
-}
-
 QByteArray QMimeMagicRule::mask() const
 {
-    QByteArray result = d->mask;
-    if (d->type == String) {
+    QByteArray result = m_mask;
+    if (m_type == String) {
         // restore '0x'
         result = "0x" + result.toHex();
     }
     return result;
 }
 
-bool QMimeMagicRule::isValid() const
-{
-    return d->matchFunction;
-}
-
 bool QMimeMagicRule::matches(const QByteArray &data) const
 {
-    const bool ok = d->matchFunction && d->matchFunction(d.data(), data);
+    const bool ok = m_matchFunction && (this->*m_matchFunction)(data);
     if (!ok)
         return false;
 
diff --git a/src/corelib/mimetypes/qmimemagicrule_p.h b/src/corelib/mimetypes/qmimemagicrule_p.h
index 84c2fe2174..1a6ca81e11 100644
--- a/src/corelib/mimetypes/qmimemagicrule_p.h
+++ b/src/corelib/mimetypes/qmimemagicrule_p.h
@@ -61,7 +61,6 @@
 
 QT_BEGIN_NAMESPACE
 
-class QMimeMagicRulePrivate;
 class QMimeMagicRule
 {
 public:
@@ -69,20 +68,16 @@ public:
 
     QMimeMagicRule(const QString &typeStr, const QByteArray &value, const QString &offsets,
                    const QByteArray &mask, QString *errorString);
-    QMimeMagicRule(const QMimeMagicRule &other);
-    ~QMimeMagicRule();
-
-    QMimeMagicRule &operator=(const QMimeMagicRule &other);
 
     bool operator==(const QMimeMagicRule &other) const;
 
-    Type type() const;
-    QByteArray value() const;
-    int startPos() const;
-    int endPos() const;
+    Type type() const { return m_type; }
+    QByteArray value() const { return m_value; }
+    int startPos() const { return m_startPos; }
+    int endPos() const { return m_endPos; }
     QByteArray mask() const;
 
-    bool isValid() const;
+    bool isValid() const { return m_matchFunction != Q_NULLPTR; }
 
     bool matches(const QByteArray &data) const;
 
@@ -94,7 +89,24 @@ public:
     static bool matchSubstring(const char *dataPtr, int dataSize, int rangeStart, int rangeLength, int valueLength, const char *valueData, const char *mask);
 
 private:
-    const QScopedPointer<QMimeMagicRulePrivate> d;
+    Type m_type;
+    QByteArray m_value;
+    int m_startPos;
+    int m_endPos;
+    QByteArray m_mask;
+
+    QByteArray m_pattern;
+    quint32 m_number;
+    quint32 m_numberMask;
+
+    typedef bool (QMimeMagicRule::*MatchFunction)(const QByteArray &data) const;
+    MatchFunction m_matchFunction;
+
+private:
+    // match functions
+    bool matchString(const QByteArray &data) const;
+    template <typename T>
+    bool matchNumber(const QByteArray &data) const;
 };
 Q_DECLARE_TYPEINFO(QMimeMagicRule, Q_MOVABLE_TYPE);