QmlDom: Don't pass ErrorMessage by value

When iterating, pass it by const ref. When "adding" pass it by rvalue
ref. In the few places where that clashes, copy it explicitly.

Coverity-Id: 417092
Change-Id: I93b2d671c38a2f44334929fd7ec9c2f1a18caac8
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>

14 files changed, 69 insertions, 56 deletions

diff --git a/src/qmldom/qqmldomastcreator.cpp b/src/qmldom/qqmldomastcreator.cpp
index 8025f30d4b..43405a96aa 100644
--- a/src/qmldom/qqmldomastcreator.cpp
+++ b/src/qmldom/qqmldomastcreator.cpp
@@ -395,10 +395,10 @@ bool QQmlDomAstCreator::visit(AST::UiPublicMember *el)
         FileLocations::addRegion(nodeStack.last().fileLocations, u"identifier",
                                  el->identifierToken);
         if (p.name == u"id")
-            qmlFile.addError(astParseErrors()
+            qmlFile.addError(std::move(astParseErrors()
                                      .warning(tr("id is a special attribute, that should not be "
                                                  "used as property name"))
-                                     .withPath(currentNodeEl().path));
+                                               .withPath(currentNodeEl().path)));
         if (p.isDefaultMember)
             FileLocations::addRegion(nodeStack.last().fileLocations, u"default",
                                      el->defaultToken());
@@ -768,11 +768,11 @@ bool QQmlDomAstCreator::visit(AST::UiObjectBinding *el)
     Path bPathFromOwner = current<QmlObject>().addBinding(
             Binding(toString(el->qualifiedId), value, bType), AddOption::KeepExisting, &bPtr);
     if (bPtr->name() == u"id")
-        qmlFile.addError(astParseErrors()
+        qmlFile.addError(std::move(astParseErrors()
                                  .warning(tr("id attributes should only be a lower case letter "
                                              "followed by letters, numbers or underscore, "
                                              "assuming they refer to an id property"))
-                                 .withPath(bPathFromOwner));
+                                           .withPath(bPathFromOwner)));
     pushEl(bPathFromOwner, *bPtr, el);
     FileLocations::addRegion(nodeStack.last().fileLocations, u"colon", el->colonToken);
     loadAnnotations(el);
@@ -833,24 +833,24 @@ bool QQmlDomAstCreator::visit(AST::UiScriptBinding *el)
                     QStringLiteral(uR"([[:lower:]][[:lower:][:upper:]0-9_]*)")));
             auto m = idRe.matchView(iExp->name);
             if (!m.hasMatch()) {
-                qmlFile.addError(
+                qmlFile.addError(std::move(
                         astParseErrors()
                                 .warning(tr("id attributes should only be a lower case letter "
                                             "followed by letters, numbers or underscore, not %1")
                                                  .arg(iExp->name))
-                                .withPath(pathFromOwner));
+                                .withPath(pathFromOwner)));
             }
         } else {
             pathFromOwner =
                     current<QmlObject>().addBinding(bindingV, AddOption::KeepExisting, &bindingPtr);
             Q_ASSERT_X(bindingPtr, className, "binding could not be retrieved");
-            qmlFile.addError(
+            qmlFile.addError(std::move(
                     astParseErrors()
                             .warning(tr("id attributes should only be a lower case letter "
                                         "followed by letters, numbers or underscore, not %1 "
                                         "%2, assuming they refer to a property")
                                              .arg(script->code(), script->astRelocatableDump()))
-                            .withPath(pathFromOwner));
+                            .withPath(pathFromOwner)));
         }
     } else {
         pathFromOwner =
@@ -931,10 +931,10 @@ bool QQmlDomAstCreator::visit(AST::UiArrayBinding *el)
     Path bindingPathFromOwner =
             current<QmlObject>().addBinding(bindingV, AddOption::KeepExisting, &bindingPtr);
     if (bindingV.name() == u"id")
-        qmlFile.addError(
+        qmlFile.addError(std::move(
                 astParseErrors()
                         .error(tr("id attributes should have only simple strings as values"))
-                        .withPath(bindingPathFromOwner));
+                        .withPath(bindingPathFromOwner)));
     pushEl(bindingPathFromOwner, *bindingPtr, el);
     FileLocations::addRegion(currentNodeEl().fileLocations, u"colon", el->colonToken);
     loadAnnotations(el);
diff --git a/src/qmldom/qqmldomelements.cpp b/src/qmldom/qqmldomelements.cpp
index 1dd5dba140..f448a7e1ae 100644
--- a/src/qmldom/qqmldomelements.cpp
+++ b/src/qmldom/qqmldomelements.cpp
@@ -1655,7 +1655,7 @@ void ScriptExpression::setCode(QString code, QString preCode, QString postCode)
             err.location.startLine -= m_localOffset.startLine;
             if (err.location.startLine == 1)
                 err.location.startColumn -= m_localOffset.startColumn;
-            addErrorLocal(err);
+            addErrorLocal(std::move(err));
         }
         m_ast = parser.rootNode();
         if (AST::Program *programPtr = AST::cast<AST::Program *>(m_ast)) {
diff --git a/src/qmldom/qqmldomerrormessage.cpp b/src/qmldom/qqmldomerrormessage.cpp
index 8760fa3df1..64dad6dcaf 100644
--- a/src/qmldom/qqmldomerrormessage.cpp
+++ b/src/qmldom/qqmldomerrormessage.cpp
@@ -287,6 +287,10 @@ struct StorableMsg {
         msg(e)
     {}
 
+    StorableMsg(ErrorMessage &&e):
+          msg(std::move(e))
+    {}
+
     ErrorMessage msg;
 };
 
@@ -296,12 +300,12 @@ static QHash<QLatin1String, StorableMsg> &registry()
     return r;
 }
 
-QLatin1String ErrorMessage::msg(const char *errorId, ErrorMessage err)
+QLatin1String ErrorMessage::msg(const char *errorId, ErrorMessage &&err)
 {
-    return msg(QLatin1String(errorId), err);
+    return msg(QLatin1String(errorId), std::move(err));
 }
 
-QLatin1String ErrorMessage::msg(QLatin1String errorId, ErrorMessage err)
+QLatin1String ErrorMessage::msg(QLatin1String errorId, ErrorMessage &&err)
 {
     bool doubleRegister = false;
     ErrorMessage old = myErrors().debug(u"dummy");
@@ -312,14 +316,14 @@ QLatin1String ErrorMessage::msg(QLatin1String errorId, ErrorMessage err)
             old = r[err.errorId].msg;
             doubleRegister = true;
         }
-        r[errorId] = StorableMsg{err.withErrorId(errorId)};
+        r[errorId] = StorableMsg{std::move(err.withErrorId(errorId))};
     }
     if (doubleRegister)
         defaultErrorHandler(myErrors().warning(tr("Double registration of error %1: (%2) vs (%3)").arg(errorId, err.withErrorId(errorId).toString(), old.toString())));
     return errorId;
 }
 
-void ErrorMessage::visitRegisteredMessages(function_ref<bool (ErrorMessage)> visitor)
+void ErrorMessage::visitRegisteredMessages(function_ref<bool(const ErrorMessage &)> visitor)
 {
     QHash<QLatin1String, StorableMsg> r;
     {
diff --git a/src/qmldom/qqmldomerrormessage_p.h b/src/qmldom/qqmldomerrormessage_p.h
index ad9c1d56ec..fb3da73418 100644
--- a/src/qmldom/qqmldomerrormessage_p.h
+++ b/src/qmldom/qqmldomerrormessage_p.h
@@ -99,9 +99,9 @@ class QMLDOM_EXPORT ErrorMessage { // reuse Some of the other DiagnosticMessages
 public:
     using Level = ErrorLevel;
     // error registry (usage is optional)
-    static QLatin1String msg(const char *errorId, ErrorMessage err);
-    static QLatin1String msg(QLatin1String errorId, ErrorMessage err);
-    static void visitRegisteredMessages(function_ref<bool(ErrorMessage)> visitor);
+    static QLatin1String msg(const char *errorId, ErrorMessage &&err);
+    static QLatin1String msg(QLatin1String errorId, ErrorMessage &&err);
+    static void visitRegisteredMessages(function_ref<bool (const ErrorMessage &)> visitor);
     [[nodiscard]] static ErrorMessage load(QLatin1String errorId);
     [[nodiscard]] static ErrorMessage load(const char *errorId);
     template<typename... T>
diff --git a/src/qmldom/qqmldomexternalitems.cpp b/src/qmldom/qqmldomexternalitems.cpp
index 1a44ef5596..4411d583df 100644
--- a/src/qmldom/qqmldomexternalitems.cpp
+++ b/src/qmldom/qqmldomexternalitems.cpp
@@ -196,9 +196,9 @@ void QmldirFile::setFromQmldir()
     bool hasErrors = false;
     for (auto const &el : m_qmldir.errors(uri().toString())) {
         ErrorMessage msg = myParsingErrors().errorMessage(el);
-        addErrorLocal(msg);
         if (msg.level == ErrorLevel::Error || msg.level == ErrorLevel::Fatal)
             hasErrors = true;
+        addErrorLocal(std::move(msg));
     }
     setIsValid(!hasErrors); // consider it valid also with errors?
     m_plugins = m_qmldir.plugins();
@@ -326,8 +326,11 @@ QmlFile::QmlFile(QString filePath, QString code, QDateTime lastDataUpdateAt, int
     lexer.setCode(code, /*lineno = */ 1, /*qmlMode=*/true);
     QQmlJS::Parser parser(m_engine.get());
     m_isValid = parser.parse();
-    for (DiagnosticMessage msg : parser.diagnosticMessages())
-        addErrorLocal(myParsingErrors().errorMessage(msg).withFile(filePath).withPath(m_path));
+    const auto diagnostics = parser.diagnosticMessages();
+    for (const DiagnosticMessage &msg : diagnostics) {
+        addErrorLocal(
+                std::move(myParsingErrors().errorMessage(msg).withFile(filePath).withPath(m_path)));
+    }
     m_ast = parser.ast();
 }
 
@@ -358,9 +361,9 @@ DomItem QmlFile::field(const DomItem &self, QStringView name) const
     return DomBase::field(self, name);
 }
 
-void QmlFile::addError(const DomItem &self, ErrorMessage msg)
+void QmlFile::addError(const DomItem &self, ErrorMessage &&msg)
 {
-    self.containingObject().addError(msg);
+    self.containingObject().addError(std::move(msg));
 }
 
 void QmlFile::writeOut(const DomItem &self, OutWriter &ow) const
diff --git a/src/qmldom/qqmldomexternalitems_p.h b/src/qmldom/qqmldomexternalitems_p.h
index b217108c9f..e9da66f013 100644
--- a/src/qmldom/qqmldomexternalitems_p.h
+++ b/src/qmldom/qqmldomexternalitems_p.h
@@ -260,7 +260,7 @@ public:
     {
         return std::static_pointer_cast<QmlFile>(doCopy(self));
     }
-    void addError(const DomItem &self, ErrorMessage msg) override;
+    void addError(const DomItem &self, ErrorMessage &&msg) override;
 
     const QMultiMap<QString, QmlComponent> &components() const & { return m_components; }
     void setComponents(const QMultiMap<QString, QmlComponent> &components)
diff --git a/src/qmldom/qqmldomitem.cpp b/src/qmldom/qqmldomitem.cpp
index 23cc37ddec..0214997eb8 100644
--- a/src/qmldom/qqmldomitem.cpp
+++ b/src/qmldom/qqmldomitem.cpp
@@ -1330,7 +1330,7 @@ DomItem::WriteOutCheckResult DomItem::performWriteOutChecks(const DomItem &origi
         } else {
             const auto iterateErrors = [&newFile](Sink s) {
                 newFile.iterateErrors(
-                        [s](DomItem, ErrorMessage msg) {
+                        [s](const DomItem &, const ErrorMessage &msg) {
                             s(u"\n  ");
                             msg.dump(s);
                             return true;
@@ -2467,12 +2467,14 @@ QDateTime DomItem::lastDataUpdateAt() const
         return QDateTime::fromMSecsSinceEpoch(0, QTimeZone::UTC);
 }
 
-void DomItem::addError(ErrorMessage msg) const
+void DomItem::addError(ErrorMessage &&msg) const
 {
     if (m_owner) {
         DomItem myOwner = owner();
         std::visit(
-                [this, &myOwner, &msg](auto &&ow) { ow->addError(myOwner, msg.withItem(*this)); },
+                [this, &myOwner, &msg](auto &&ow) {
+                    ow->addError(myOwner, std::move(msg.withItem(*this)));
+                },
                 *m_owner);
     } else
         defaultErrorHandler(msg.withItem(*this));
@@ -2481,7 +2483,7 @@ void DomItem::addError(ErrorMessage msg) const
 ErrorHandler DomItem::errorHandler() const
 {
     DomItem self = *this;
-    return [self](ErrorMessage m) mutable { self.addError(m); };
+    return [self](const ErrorMessage &m) { self.addError(ErrorMessage(m)); };
 }
 
 void DomItem::clearErrors(ErrorGroups groups, bool iterate) const
@@ -2496,8 +2498,9 @@ void DomItem::clearErrors(ErrorGroups groups, bool iterate) const
     }
 }
 
-bool DomItem::iterateErrors(function_ref<bool(const DomItem &, ErrorMessage)> visitor, bool iterate,
-                            Path inPath) const
+bool DomItem::iterateErrors(
+        function_ref<bool(const DomItem &, const ErrorMessage &)> visitor, bool iterate,
+        Path inPath) const
 {
     if (m_owner) {
         DomItem ow = owner();
@@ -3287,12 +3290,12 @@ void OwningItem::refreshedDataAt(QDateTime tNew)
         m_lastDataUpdateAt = tNew;
 }
 
-void OwningItem::addError(const DomItem &, ErrorMessage msg)
+void OwningItem::addError(const DomItem &, ErrorMessage &&msg)
 {
-    addErrorLocal(msg);
+    addErrorLocal(std::move(msg));
 }
 
-void OwningItem::addErrorLocal(ErrorMessage msg)
+void OwningItem::addErrorLocal(ErrorMessage &&msg)
 {
     QMutexLocker l(mutex());
     quint32 &c = m_errorsCounts[msg];
@@ -3314,7 +3317,7 @@ void OwningItem::clearErrors(ErrorGroups groups)
 }
 
 bool OwningItem::iterateErrors(
-        const DomItem &self, function_ref<bool(const DomItem &, ErrorMessage)> visitor,
+        const DomItem &self, function_ref<bool(const DomItem &, const ErrorMessage &)> visitor,
         Path inPath)
 {
     QMultiMap<Path, ErrorMessage> myErrors;
@@ -3370,7 +3373,7 @@ bool operator==(const DomItem &o1, const DomItem &o2)
 ErrorHandler MutableDomItem::errorHandler()
 {
     MutableDomItem self;
-    return [&self](ErrorMessage m) { self.addError(m); };
+    return [&self](const ErrorMessage &m) { self.addError(ErrorMessage(m)); };
 }
 
 MutableDomItem MutableDomItem::addPrototypePath(Path prototypePath)
diff --git a/src/qmldom/qqmldomitem_p.h b/src/qmldom/qqmldomitem_p.h
index d3b2c2e29c..0f65850e57 100644
--- a/src/qmldom/qqmldomitem_p.h
+++ b/src/qmldom/qqmldomitem_p.h
@@ -1069,12 +1069,12 @@ public:
     QDateTime frozenAt() const;
     QDateTime lastDataUpdateAt() const;
 
-    void addError(ErrorMessage msg) const;
+    void addError(ErrorMessage &&msg) const;
     ErrorHandler errorHandler() const;
     void clearErrors(ErrorGroups groups = ErrorGroups({}), bool iterate = true) const;
     // return false if a quick exit was requested
     bool iterateErrors(
-            function_ref<bool(const DomItem &source, ErrorMessage msg)> visitor, bool iterate,
+            function_ref<bool (const DomItem &, const ErrorMessage &)> visitor, bool iterate,
             Path inPath = Path()) const;
 
     bool iterateSubOwners(function_ref<bool(const DomItem &owner)> visitor) const;
@@ -1450,13 +1450,13 @@ public:
     virtual bool freeze();
     QDateTime frozenAt() const;
 
-    virtual void addError(const DomItem &self, ErrorMessage msg);
-    void addErrorLocal(ErrorMessage msg);
+    virtual void addError(const DomItem &self, ErrorMessage &&msg);
+    void addErrorLocal(ErrorMessage &&msg);
     void clearErrors(ErrorGroups groups = ErrorGroups({}));
     // return false if a quick exit was requested
     bool iterateErrors(
             const DomItem &self,
-            function_ref<bool(const DomItem &source, ErrorMessage msg)> visitor,
+            function_ref<bool(const DomItem &source, const ErrorMessage &msg)> visitor,
             Path inPath = Path());
     QMultiMap<Path, ErrorMessage> localErrors() const {
         QMutexLocker l(mutex());
@@ -1683,7 +1683,7 @@ public:
     QDateTime frozenAt() { return m_owner.frozenAt(); }
     QDateTime lastDataUpdateAt() { return m_owner.lastDataUpdateAt(); }
 
-    void addError(ErrorMessage msg) { item().addError(msg); }
+    void addError(ErrorMessage &&msg) { item().addError(std::move(msg)); }
     ErrorHandler errorHandler();
 
     // convenience setters
diff --git a/src/qmldom/qqmldommoduleindex.cpp b/src/qmldom/qqmldommoduleindex.cpp
index 3f78e9144f..2826f72437 100644
--- a/src/qmldom/qqmldommoduleindex.cpp
+++ b/src/qmldom/qqmldommoduleindex.cpp
@@ -246,12 +246,12 @@ QList<DomItem> ModuleIndex::exportsWithNameAndMinorVersion(const DomItem &self,
                 undef.append(exportItem);
             } else {
                 if (majorVersion() < 0)
-                    self.addError(myVersioningErrors()
+                    self.addError(std::move(myVersioningErrors()
                                           .error(tr("Module %1 (unversioned) has versioned entries "
                                                     "for '%2' from %3")
                                                          .arg(uri(), name,
                                                               source.canonicalPath().toString()))
-                                          .withPath(myPath));
+                                                    .withPath(myPath)));
                 if ((versionPtr->majorVersion == majorVersion()
                      || versionPtr->majorVersion == Version::Undefined)
                     && versionPtr->minorVersion >= vNow
@@ -266,11 +266,11 @@ QList<DomItem> ModuleIndex::exportsWithNameAndMinorVersion(const DomItem &self,
     }
     if (!undef.isEmpty()) {
         if (!res.isEmpty()) {
-            self.addError(myVersioningErrors()
+            self.addError(std::move(myVersioningErrors()
                                   .error(tr("Module %1 (major version %2) has versioned and "
                                             "unversioned entries for '%3'")
                                                  .arg(uri(), QString::number(majorVersion()), name))
-                                  .withPath(myPath));
+                                            .withPath(myPath)));
             return res + undef;
         } else {
             return undef;
diff --git a/src/qmldom/qqmldomoutwriter.cpp b/src/qmldom/qqmldomoutwriter.cpp
index 405a281fbb..715a850948 100644
--- a/src/qmldom/qqmldomoutwriter.cpp
+++ b/src/qmldom/qqmldomoutwriter.cpp
@@ -186,7 +186,7 @@ DomItem OutWriter::updatedFile(const DomItem &qmlFile)
                                         targetExpr.item()
                                                 .copy(exprPtr, targetExpr.canonicalPath())
                                                 .iterateErrors(
-                                                        [s](DomItem, ErrorMessage msg) {
+                                                        [s](const DomItem &, const ErrorMessage &msg) {
                                                             s(u"\n  ");
                                                             msg.dump(s);
                                                             return true;
diff --git a/src/qmldom/qqmldomtop.cpp b/src/qmldom/qqmldomtop.cpp
index 7f64148f1c..93dfbfe009 100644
--- a/src/qmldom/qqmldomtop.cpp
+++ b/src/qmldom/qqmldomtop.cpp
@@ -427,7 +427,7 @@ void DomUniverse::execQueue()
                 } else {
                     QString errs;
                     DomItem qmlFileObj = env.copy(qmlFile);
-                    qmlFile->iterateErrors(qmlFileObj, [&errs](DomItem, ErrorMessage m) {
+                    qmlFile->iterateErrors(qmlFileObj, [&errs](const DomItem &, const ErrorMessage &m) {
                         errs += m.toString();
                         errs += u"\n";
                         return true;
@@ -465,8 +465,11 @@ void DomUniverse::execQueue()
                 Q_ASSERT(false);
             }
         }
-        for (const ErrorMessage &m : messages)
-            newValue.addError(m);
+
+        for (auto it = messages.begin(), end = messages.end(); it != end; ++it)
+            newValue.addError(std::move(*it));
+        messages.clear();
+
         // to do: tell observers?
         // execute callback
         if (t.callback) {
@@ -1126,7 +1129,7 @@ bool DomEnvironment::iterateDirectSubpaths(const DomItem &self, DirectVisitor vi
                 Path::Field(Fields::loadInfo),
                 [this](const DomItem &map, QString pStr) {
                     bool hasErrors = false;
-                    Path p = Path::fromString(pStr, [&hasErrors](ErrorMessage m) {
+                    Path p = Path::fromString(pStr, [&hasErrors](const ErrorMessage &m) {
                         switch (m.level) {
                         case ErrorLevel::Debug:
                         case ErrorLevel::Info:
diff --git a/src/qmldom/qqmldomtop_p.h b/src/qmldom/qqmldomtop_p.h
index 6d1637c859..4f477d842a 100644
--- a/src/qmldom/qqmldomtop_p.h
+++ b/src/qmldom/qqmldomtop_p.h
@@ -539,9 +539,9 @@ public:
     {
         return std::static_pointer_cast<LoadInfo>(doCopy(self));
     }
-    void addError(const DomItem &self, ErrorMessage msg) override
+    void addError(const DomItem &self, ErrorMessage &&msg) override
     {
-        self.path(elementCanonicalPath()).addError(msg);
+        self.path(elementCanonicalPath()).addError(std::move(msg));
     }
 
     void addEndCallback(const DomItem &self, std::function<void(Path, const DomItem &, const DomItem &)> callback);
diff --git a/src/qmldom/qqmldomtypesreader.cpp b/src/qmldom/qqmldomtypesreader.cpp
index ce0f3b8c05..4b29899d5b 100644
--- a/src/qmldom/qqmldomtypesreader.cpp
+++ b/src/qmldom/qqmldomtypesreader.cpp
@@ -249,7 +249,7 @@ bool QmltypesReader::parse()
     return m_isValid;
 }
 
-void QmltypesReader::addError(ErrorMessage message)
+void QmltypesReader::addError(ErrorMessage &&message)
 {
     if (message.file.isEmpty())
         message.file = qmltypesFile().canonicalFilePath();
diff --git a/src/qmldom/qqmldomtypesreader_p.h b/src/qmldom/qqmldomtypesreader_p.h
index d1650d726d..38bca2e02d 100644
--- a/src/qmldom/qqmldomtypesreader_p.h
+++ b/src/qmldom/qqmldomtypesreader_p.h
@@ -40,7 +40,7 @@ public:
     bool parse();
     // static void read
 private:
-    void addError(ErrorMessage message);
+    void addError(ErrorMessage &&message);
 
     void insertProperty(QQmlJSScope::Ptr jsScope, const QQmlJSMetaProperty &property,
                         QMap<int, QmlObject> &objs);
@@ -53,7 +53,7 @@ private:
     DomItem &qmltypesFile() { return m_qmltypesFile; }
     ErrorHandler handler()
     {
-        return [this](ErrorMessage m) { this->addError(m); };
+        return [this](const ErrorMessage &m) { this->addError(ErrorMessage(m)); };
     }
 
 private: