Move saveToDisk into SaveableUnitPointer

This way we can keep the flag mutilation closely local to the place
where we write the data. Also, SaveableUnitPointer doesn't need a full
CompilationUnit this way.

Change-Id: I01872e4c406cb2ccbaa1fa35325cc063b1e8a7df
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
 

4 files changed, 78 insertions, 76 deletions

diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp
index 2cb24dc113..a7e0909b16 100644
--- a/src/qml/compiler/qv4compileddata.cpp
+++ b/src/qml/compiler/qv4compileddata.cpp
@@ -42,7 +42,6 @@
 #include <private/qqmlirbuilder_p.h>
 #include <QCoreApplication>
 #include <QCryptographicHash>
-#include <QSaveFile>
 #include <QScopeGuard>
 #include <QFileInfo>
 
@@ -79,38 +78,6 @@ CompilationUnit::~CompilationUnit()
     imports = nullptr;
 }
 
-bool CompilationUnit::saveToDisk(const QString &outputFileName, QString *errorString) const
-{
-    errorString->clear();
-
-#if QT_CONFIG(temporaryfile)
-    // Foo.qml -> Foo.qmlc
-    QSaveFile cacheFile(outputFileName);
-    if (!cacheFile.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
-        *errorString = cacheFile.errorString();
-        return false;
-    }
-
-    SaveableUnitPointer saveable(this);
-    qint64 headerWritten = cacheFile.write(saveable.data<char>(), saveable.size());
-    if (headerWritten != saveable.size()) {
-        *errorString = cacheFile.errorString();
-        return false;
-    }
-
-    if (!cacheFile.commit()) {
-        *errorString = cacheFile.errorString();
-        return false;
-    }
-
-    return true;
-#else
-    Q_UNUSED(outputFileName)
-    *errorString = QStringLiteral("features.temporaryfile is disabled.");
-    return false;
-#endif // QT_CONFIG(temporaryfile)
-}
-
 void CompilationUnit::setUnitData(const Unit *unitData, const QmlUnit *qmlUnit,
                                   const QString &fileName, const QString &finalUrlString)
 {
diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h
index 73db6f54e6..933d91f2a6 100644
--- a/src/qml/compiler/qv4compileddata_p.h
+++ b/src/qml/compiler/qv4compileddata_p.h
@@ -52,6 +52,12 @@
 
 #include <QtCore/qstring.h>
 #include <QtCore/qcryptographichash.h>
+#include <QtCore/qscopeguard.h>
+
+#if QT_CONFIG(temporaryfile)
+#include <QtCore/qsavefile.h>
+#endif
+
 #include <QVector>
 #include <QStringList>
 #include <QHash>
@@ -1140,51 +1146,71 @@ private:
     QString m_finalUrlString; // initialized from data->finalUrlIndex
 
     Heap::Module *m_module = nullptr;
-
-public:
-    bool saveToDisk(const QString &outputFileName, QString *errorString) const;
 };
 
 class SaveableUnitPointer
 {
     Q_DISABLE_COPY_MOVE(SaveableUnitPointer)
 public:
-    SaveableUnitPointer(const CompilationUnit *unit, quint32 temporaryFlags = Unit::StaticData) :
-          unit(unit)
+    SaveableUnitPointer(const Unit *unit, quint32 temporaryFlags = Unit::StaticData) :
+          unit(unit),
+          temporaryFlags(temporaryFlags)
     {
-        quint32_le &unitFlags = mutableFlags();
-        quint32 origFlags = unitFlags;
-        unitFlags |= temporaryFlags;
-        changedFlags = origFlags ^ unitFlags;
     }
 
-    ~SaveableUnitPointer()
+    ~SaveableUnitPointer() = default;
+
+    template<typename Char>
+    bool saveToDisk(const std::function<bool(const Char *, quint32)> &writer) const
     {
-        mutableFlags() ^= changedFlags;
+        auto cleanup = qScopeGuard([this]() { mutableFlags() ^= temporaryFlags; });
+        mutableFlags() |= temporaryFlags;
+        return writer(data<Char>(), size());
     }
 
-    const CompilationUnit *operator->() const { return unit; }
-    const CompilationUnit &operator*() const { return *unit; }
-    operator const CompilationUnit *() { return unit; }
+    static bool writeDataToFile(const QString &outputFileName, const char *data, quint32 size,
+                                QString *errorString)
+    {
+#if QT_CONFIG(temporaryfile)
+        QSaveFile cacheFile(outputFileName);
+        if (!cacheFile.open(QIODevice::WriteOnly | QIODevice::Truncate)
+                || cacheFile.write(data, size) != size
+                || !cacheFile.commit()) {
+            *errorString = cacheFile.errorString();
+            return false;
+        }
+
+        errorString->clear();
+        return true;
+#else
+        Q_UNUSED(outputFileName)
+        *errorString = QStringLiteral("features.temporaryfile is disabled.");
+        return false;
+#endif
+    }
+
+private:
+    const Unit *unit;
+    quint32 temporaryFlags;
+
+    quint32_le &mutableFlags() const
+    {
+        return const_cast<Unit *>(unit)->flags;
+    }
 
     template<typename Char>
     const Char *data() const
     {
         Q_STATIC_ASSERT(sizeof(Char) == 1);
         const Char *dataPtr;
-        memcpy(&dataPtr, &unit->data, sizeof(dataPtr));
+        memcpy(&dataPtr, &unit, sizeof(dataPtr));
         return dataPtr;
     }
 
     quint32 size() const
     {
-        return unit->data->unitSize;
+        return unit->unitSize;
     }
-
-private:
-    quint32_le &mutableFlags() { return const_cast<Unit *>(unit->unitData())->flags; };
-    const CompilationUnit *unit;
-    quint32 changedFlags;
 };
 
 
diff --git a/src/qml/jsruntime/qv4executablecompilationunit.cpp b/src/qml/jsruntime/qv4executablecompilationunit.cpp
index 7867dff769..1e725cbef7 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit.cpp
+++ b/src/qml/jsruntime/qv4executablecompilationunit.cpp
@@ -687,7 +687,11 @@ bool ExecutableCompilationUnit::saveToDisk(const QUrl &unitUrl, QString *errorSt
         return false;
     }
 
-    return CompilationUnit::saveToDisk(localCacheFilePath(unitUrl), errorString);
+    return CompiledData::SaveableUnitPointer(unitData()).saveToDisk<char>(
+            [this, &unitUrl, errorString](const char *data, quint32 size) {
+        return CompiledData::SaveableUnitPointer::writeDataToFile(localCacheFilePath(unitUrl), data,
+                                                                  size, errorString);
+    });
 }
 
 /*!
diff --git a/tools/qmlcachegen/qmlcachegen.cpp b/tools/qmlcachegen/qmlcachegen.cpp
index ff6ba17612..a91c181233 100644
--- a/tools/qmlcachegen/qmlcachegen.cpp
+++ b/tools/qmlcachegen/qmlcachegen.cpp
@@ -233,7 +233,7 @@ static bool compileQmlFile(const QString &inputFileName, SaveFunction saveFuncti
         const quint32 saveFlags
                 = QV4::CompiledData::Unit::StaticData
                 | QV4::CompiledData::Unit::PendingTypeCompilation;
-        QV4::CompiledData::SaveableUnitPointer saveable(&irDocument.javaScriptCompilationUnit,
+        QV4::CompiledData::SaveableUnitPointer saveable(irDocument.javaScriptCompilationUnit.data,
                                                         saveFlags);
         if (!saveFunction(saveable, &error->message))
             return false;
@@ -327,7 +327,7 @@ static bool compileJSFile(const QString &inputFileName, const QString &inputFile
         }
     }
 
-    return saveFunction(QV4::CompiledData::SaveableUnitPointer(&unit), &error->message);
+    return saveFunction(QV4::CompiledData::SaveableUnitPointer(unit.data), &error->message);
 }
 
 static bool saveUnitAsCpp(const QString &inputFileName, const QString &outputFileName,
@@ -366,27 +366,28 @@ static bool saveUnitAsCpp(const QString &inputFileName, const QString &outputFil
     if (!writeStr(QByteArrayLiteral(" {\nextern const unsigned char qmlData alignas(16) [] = {\n")))
         return false;
 
-    QByteArray hexifiedData;
-    {
-        QTextStream stream(&hexifiedData);
-        const uchar *begin = unit.data<uchar>();
-        const uchar *end = begin + unit.size();
-        stream << hex;
-        int col = 0;
-        for (const uchar *data = begin; data < end; ++data, ++col) {
-            if (data > begin)
-                stream << ',';
-            if (col % 8 == 0) {
-                stream << '\n';
-                col = 0;
+    unit.saveToDisk<uchar>([&writeStr](const uchar *begin, quint32 size) {
+        QByteArray hexifiedData;
+        {
+            QTextStream stream(&hexifiedData);
+            const uchar *end = begin + size;
+            stream << hex;
+            int col = 0;
+            for (const uchar *data = begin; data < end; ++data, ++col) {
+                if (data > begin)
+                    stream << ',';
+                if (col % 8 == 0) {
+                    stream << '\n';
+                    col = 0;
+                }
+                stream << "0x" << *data;
             }
-            stream << "0x" << *data;
+            stream << '\n';
         }
-        stream << '\n';
-    };
+        return writeStr(hexifiedData);
+    });
+
 
-    if (!writeStr(hexifiedData))
-        return false;
 
     if (!writeStr("};\n}\n}\n"))
         return false;
@@ -527,7 +528,11 @@ int main(int argc, char **argv)
     } else {
         saveFunction = [outputFileName](const QV4::CompiledData::SaveableUnitPointer &unit,
                                         QString *errorString) {
-            return unit->saveToDisk(outputFileName, errorString);
+            return unit.saveToDisk<char>(
+                    [&outputFileName, errorString](const char *data, quint32 size) {
+                        return QV4::CompiledData::SaveableUnitPointer::writeDataToFile(
+                                outputFileName, data, size, errorString);
+            });
         };
     }