Don't call virtual method from Exception c'tor

In case a static s_shouldAssert is true, we were calling
virtual description() method from the c'tor of base Exception
class, which was reimplemented in subclasses.
This could cause a crash, since reimplementations
were using not initialized yet private fields.

We fix it by de-virtualizing the description() method
and instead we pass the description in overloaded, protected
c'tors. Original c'tors still use the default description
that was used for base Exception class.

In addition we make all private fields const, and
make showException method non-virtual, as it didn't have
any overrides.

Change-Id: I7ebae1809ab423e990276e500f9c0db569d10e2d
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/plugins/qmldesigner/designercore/exceptions/exception.cpp

@@ -99,33 +99,50 @@ bool Exception::warnAboutException()
 #endif
 }
 
-/*!
-    Constructs an exception. \a line uses the __LINE__ macro, \a function uses
-    the __FUNCTION__ or the Q_FUNC_INFO macro, and \a file uses
-    the __FILE__ macro.
-*/
-Exception::Exception(int line,
-              const QByteArray &_function,
-              const QByteArray &_file)
-  : m_line(line),
-    m_function(QString::fromUtf8(_function)),
-    m_file(QString::fromUtf8(_file))
-{
 #ifdef Q_OS_LINUX
+static QString getBackTrace()
+{
+    QString backTrace;
     void * array[50];
     int nSize = backtrace(array, 50);
     char ** symbols = backtrace_symbols(array, nSize);
 
     for (int i = 0; i < nSize; i++)
-    {
+        backTrace.append(QString("%1\n").arg(QLatin1String(symbols[i])));
-        m_backTrace.append(QString("%1\n").arg(QLatin1String(symbols[i])));
-    }
 
     free(symbols);
+
+    return backTrace;
+}
 #endif
 
+QString Exception::defaultDescription(int line, const QByteArray &function, const QByteArray &file)
+{
+    return QString(QStringLiteral("file: %1, function: %2, line: %3"))
+            .arg(QString::fromUtf8(file), QString::fromUtf8(function), QString::number(line));
+}
+
+/*!
+    Constructs an exception. \a line uses the __LINE__ macro, \a function uses
+    the __FUNCTION__ or the Q_FUNC_INFO macro, and \a file uses
+    the __FILE__ macro.
+*/
+Exception::Exception(int line, const QByteArray &function, const QByteArray &file)
+  : Exception(line, function, file, Exception::defaultDescription(line, function, file))
+{ }
+
+Exception::Exception(int line, const QByteArray &function,
+                     const QByteArray &file, const QString &description)
+  : m_line(line)
+  , m_function(QString::fromUtf8(function))
+  , m_file(QString::fromUtf8(file))
+  , m_description(description)
+  #ifdef Q_OS_LINUX
+  , m_backTrace(getBackTrace())
+  #endif
+{
     if (s_shouldAssert) {
-        qDebug() << description();
+        qDebug() << Exception::description();
         QTC_ASSERT(false, ;);
         Q_ASSERT(false);
     }
@@ -152,7 +169,7 @@ void Exception::createWarning() const
 */
 QString Exception::description() const
 {
-    return QString(QStringLiteral("file: %1, function: %2, line: %3")).arg(m_file, m_function, QString::number(m_line));
+    return m_description;
 }
 
 /*!

src/plugins/qmldesigner/designercore/exceptions/invalidargumentexception.cpp

@@ -35,6 +35,19 @@ argument.
 */
 namespace QmlDesigner {
 
+QString InvalidArgumentException::invalidArgumentDescription(int line,
+                                                             const QByteArray &function,
+                                                             const QByteArray &file,
+                                                             const QByteArray &argument)
+{
+    if (QString::fromUtf8(function) == QLatin1String("createNode")) {
+        return QCoreApplication::translate("QmlDesigner::InvalidArgumentException",
+                  "Failed to create item of type %1").arg(QString::fromUtf8(argument));
+    }
+
+    return Exception::defaultDescription(line, function, file);
+}
+
 /*!
     Constructs the exception for \a argument. \a line uses the __LINE__ macro,
     \a function uses the __FUNCTION__ or the Q_FUNC_INFO macro, and \a file uses
@@ -44,17 +57,21 @@ InvalidArgumentException::InvalidArgumentException(int line,
                                                    const QByteArray &function,
                                                    const QByteArray &file,
                                                    const QByteArray &argument)
- : Exception(line, function, file), m_argument(QString::fromUtf8(argument))
+    : InvalidArgumentException(line, function, file, argument,
+                               invalidArgumentDescription(line, function, file, argument))
 {
     createWarning();
 }
 
-QString InvalidArgumentException::description() const
+InvalidArgumentException::InvalidArgumentException(int line,
+                                                   const QByteArray &function,
+                                                   const QByteArray &file,
+                                                   const QByteArray &argument,
+                                                   const QString &description)
+    : Exception(line, function, file, description)
+    , m_argument(QString::fromUtf8(argument))
 {
-    if (function() == QLatin1String("createNode"))
+    createWarning();
-        return QCoreApplication::translate("QmlDesigner::InvalidArgumentException", "Failed to create item of type %1").arg(m_argument);
-
-    return Exception::description();
 }
 
 /*!

src/plugins/qmldesigner/designercore/exceptions/invalididexception.cpp

@@ -29,28 +29,40 @@
 
 namespace QmlDesigner {
 
-InvalidIdException::InvalidIdException(int line,
+static QString descriptionBasedOnReason(InvalidIdException::Reason reason)
-                                       const QByteArray &function,
-                                       const QByteArray &file,
-                                       const QByteArray &id,
-                                       Reason reason) :
-    InvalidArgumentException(line, function, file, "id"),
-    m_id(QString::fromUtf8(id))
 {
-    if (reason == InvalidCharacters)
+    if (reason == InvalidIdException::InvalidCharacters)
-        m_description = QCoreApplication::translate("InvalidIdException", "Only alphanumeric characters and underscore allowed.\nIds must begin with a lowercase letter.");
+        return QCoreApplication::translate("InvalidIdException",
-    else
+                                           "Only alphanumeric characters and underscore allowed.\n"
-        m_description = QCoreApplication::translate("InvalidIdException", "Ids have to be unique.");
+                                           "Ids must begin with a lowercase letter.");
+
+    return QCoreApplication::translate("InvalidIdException", "Ids have to be unique.");
+}
+
+static QString decorateDescriptionWithId(const QString &id, const QString &description)
+{
+    return QCoreApplication::translate("InvalidIdException", "Invalid Id: %1\n%2")
+            .arg(id, description);
 }
 
 InvalidIdException::InvalidIdException(int line,
                                        const QByteArray &function,
                                        const QByteArray &file,
                                        const QByteArray &id,
-                                       const QByteArray &description) :
+                                       Reason reason)
-    InvalidArgumentException(line, function, file, "id"),
+    : InvalidArgumentException(line, function, file, "id",
-    m_id(QString::fromUtf8(id)),
+                               decorateDescriptionWithId(QString::fromUtf8(id),
-    m_description(QString::fromUtf8(description))
+                                                         descriptionBasedOnReason(reason)))
+{ }
+
+InvalidIdException::InvalidIdException(int line,
+                                       const QByteArray &function,
+                                       const QByteArray &file,
+                                       const QByteArray &id,
+                                       const QByteArray &description)
+    : InvalidArgumentException(line, function, file, "id",
+                               decorateDescriptionWithId(QString::fromUtf8(id),
+                                                         QString::fromUtf8(description)))
 {
     createWarning();
 }
@@ -60,9 +72,4 @@ QString InvalidIdException::type() const
     return QLatin1String("InvalidIdException");
 }
 
-QString InvalidIdException::description() const
-{
-    return QCoreApplication::translate("InvalidIdException", "Invalid Id: %1\n%2").arg(m_id, m_description);
-}
-
 }

src/plugins/qmldesigner/designercore/exceptions/invalidqmlsourceexception.cpp

@@ -42,8 +42,7 @@ InvalidQmlSourceException::InvalidQmlSourceException(int line,
                               const QByteArray &function,
                               const QByteArray &file,
                               const QByteArray &qmlSource)
- :  Exception(line, function, file),
+ :  Exception(line, function, file, QString::fromUtf8(qmlSource))
-    m_qmlSource(QString::fromUtf8(qmlSource))
 {
     createWarning();
 }
@@ -56,9 +55,4 @@ QString InvalidQmlSourceException::type() const
     return QLatin1String("InvalidQmlSourceException");
 }
 
-QString InvalidQmlSourceException::description() const
-{
-    return m_qmlSource;
-}
-
 } // namespace QmlDesigner

src/plugins/qmldesigner/designercore/exceptions/rewritingexception.cpp

@@ -31,8 +31,9 @@ RewritingException::RewritingException(int line,
                                        const QByteArray &function,
                                        const QByteArray &file,
                                        const QByteArray &description,
-                                       const QString &documentTextContent):
+                                       const QString &documentTextContent)
-        Exception(line, function, file), m_description(QString::fromUtf8(description)), m_documentTextContent(documentTextContent)
+    : Exception(line, function, file, QString::fromUtf8(description))
+    , m_documentTextContent(documentTextContent)
 {
     createWarning();
 }
@@ -42,11 +43,6 @@ QString RewritingException::type() const
     return QLatin1String("RewritingException");
 }
 
-QString RewritingException::description() const
-{
-    return m_description;
-}
-
 QString RewritingException::documentTextContent() const
 {
     return m_documentTextContent;

src/plugins/qmldesigner/designercore/include/exception.h

@@ -41,8 +41,8 @@ public:
     virtual ~Exception();
 
     virtual QString type() const = 0;
-    virtual QString description() const;
+    QString description() const;
-    virtual void showException(const QString &title = QString()) const;
+    void showException(const QString &title = QString()) const;
 
     int line() const;
     QString function() const;
@@ -55,11 +55,20 @@ public:
     static bool shouldAssert();
     static bool warnAboutException();
 
+protected:
+    Exception(int line,
+              const QByteArray &function,
+              const QByteArray &file,
+              const QString &description);
+    static QString defaultDescription(int line, const QByteArray &function, const QByteArray &file);
+    QString defaultDescription();
+
 private:
-    int m_line;
+    const int m_line;
-    QString m_function;
+    const QString m_function;
-    QString m_file;
+    const QString m_file;
-    QString m_backTrace;
+    const QString m_description;
+    const QString m_backTrace;
     static bool s_shouldAssert;
 };
 

src/plugins/qmldesigner/designercore/include/invalidargumentexception.h

@@ -40,8 +40,17 @@ public:
 
     QString type() const override;
     QString argument() const;
-    QString description() const override;
 
+protected:
+    InvalidArgumentException(int line,
+                             const QByteArray &function,
+                             const QByteArray &file,
+                             const QByteArray &argument,
+                             const QString &description);
+    static QString invalidArgumentDescription(int line,
+                                              const QByteArray &function,
+                                              const QByteArray &file,
+                                              const QByteArray &argument);
 private:
     const QString m_argument;
 };

src/plugins/qmldesigner/designercore/include/invalididexception.h

@@ -47,11 +47,6 @@ public:
                        const QByteArray &description);
 
     QString type() const override;
-    QString description() const override;
-
-private:
-    QString m_id;
-    QString m_description;
 };
 
 }

src/plugins/qmldesigner/designercore/include/invalidqmlsourceexception.h

@@ -38,10 +38,6 @@ public:
                               const QByteArray &qmlSource = QByteArray());
 
     QString type() const override;
-    QString description() const override;
-
-private:
-   QString m_qmlSource;
 };
 
 } // namespace QmlDesigner

src/plugins/qmldesigner/designercore/include/rewritingexception.h

@@ -39,11 +39,10 @@ public:
                        const QString &documentTextContent);
 
     QString type() const override;
-    QString description() const override;
     QString documentTextContent() const;
+
 private:
-    QString m_description;
+    const QString m_documentTextContent;
-    QString m_documentTextContent;
 };
 
 } // namespace QmlDesigner