From 412445afb52973eaef11875b286885761501f1ea Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 26 May 2021 15:01:14 +0200 Subject: [PATCH] Merge bitcoin-core/gui#313: qt: Optimize string concatenation by default a02c970eb001b456d74ddc30750fe8b55348ddac qt, refactor: Revert explicit including QStringBuilder (Hennadii Stepanov) 3fd3a0fc87a81d42755246830124833e9ca3f0a9 qt, build: Optimize string concatenation (Hennadii Stepanov) Pull request description: From [Qt docs](https://doc.qt.io/qt-5/qstring.html#more-efficient-string-construction): > ... multiple uses of the \[`QString`\] '+' operator usually means multiple memory allocations. When concatenating n substrings, where n > 2, there can be as many as n - 1 calls to the memory allocator. With this PR > ... the '+' will automatically be performed as the `QStringBuilder` '%' everywhere. The change in the `src/Makefile.qt.include` file does not justify submitting this PR into the main repo, IMHO. ACKs for top commit: laanwj: Code review ACK a02c970eb001b456d74ddc30750fe8b55348ddac Talkless: utACK a02c970eb001b456d74ddc30750fe8b55348ddac, built successfully on Debian Sid with Qt 5.15.2, but did not check if any displayed strings are "wrong" after refactoring. jarolrod: ACK a02c970eb001b456d74ddc30750fe8b55348ddac Tree-SHA512: cbb476ee96f27c3bd6e125efab74d8bf24bbdb4c30576b3feea45e203405f3bf5b497dd7d3e11361fc825fcbf4b893b152921a9efdeaf73b42d1865d85f0ae84 --- src/Makefile.qt.include | 2 +- src/qt/bitcoin.cpp | 9 ++++----- src/qt/guiutil.cpp | 3 +-- src/qt/optionsmodel.cpp | 3 ++- src/qt/overviewpage.cpp | 2 +- src/qt/peertablemodel.cpp | 2 +- src/qt/recentrequeststablemodel.cpp | 9 ++++++++- src/qt/test/wallettests.cpp | 2 +- src/qt/transactiondesc.cpp | 9 ++++++--- src/qt/transactiontablemodel.cpp | 6 ++++-- 10 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index c9efbe6504..a97956c48d 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -364,7 +364,7 @@ RES_ANIMATION = $(wildcard $(srcdir)/qt/res/animation/spinner-*.png) BITCOIN_RC = qt/res/dash-qt-res.rc -BITCOIN_QT_INCLUDES = -DQT_NO_KEYWORDS +BITCOIN_QT_INCLUDES = -DQT_NO_KEYWORDS -DQT_USE_QSTRINGBUILDER qt_libbitcoinqt_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_QT_INCLUDES) \ $(QT_INCLUDES) $(QT_DBUS_INCLUDES) $(QR_CFLAGS) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index fc66231389..52bd0767b3 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -54,7 +54,6 @@ #include #include #include -#include #include #include #include @@ -492,8 +491,8 @@ void BitcoinApplication::handleRunawayException(const QString &message) { QMessageBox::critical( nullptr, tr("Runaway exception"), - tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) % - QLatin1String("

") % GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT)); + tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) + + QLatin1String("

") + GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT)); ::exit(EXIT_FAILURE); } @@ -503,8 +502,8 @@ void BitcoinApplication::handleNonFatalException(const QString& message) QMessageBox::warning( nullptr, tr("Internal error"), tr("An internal error occurred. %1 will attempt to continue safely. This is " - "an unexpected bug which can be reported as described below.").arg(PACKAGE_NAME) % - QLatin1String("

") % GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT)); + "an unexpected bug which can be reported as described below.").arg(PACKAGE_NAME) + + QLatin1String("

") + GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT)); } WId BitcoinApplication::getMainWinId() const diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index a67efe2552..649b009f25 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -69,7 +69,6 @@ #include #include #include -#include #include // for Qt::mightBeRichText #include #include @@ -1931,7 +1930,7 @@ QString MakeHtmlLink(const QString& source, const QString& link) { return QString(source).replace( link, - QLatin1String("") % link % QLatin1String("")); + QLatin1String("") + link + QLatin1String("")); } void PrintSlotException( diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index fb07fd662e..1758b95dbf 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -26,6 +26,7 @@ #endif #include +#include #include #include @@ -381,7 +382,7 @@ static ProxySetting GetProxySetting(QSettings &settings, const QString &name) static void SetProxySetting(QSettings &settings, const QString &name, const ProxySetting &ip_port) { - settings.setValue(name, ip_port.ip + ":" + ip_port.port); + settings.setValue(name, QString{ip_port.ip + QLatin1Char(':') + ip_port.port}); } static const QString GetDefaultProxyAddress() diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 16c2a5dfae..115ce523f7 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -469,7 +469,7 @@ void OverviewPage::updateCoinJoinProgress() ui->coinJoinProgress->setValue(progress); - QString strToolPip = ("" + tr("Overall progress") + ": %1%
" + + QString strToolPip = QString("" + tr("Overall progress") + ": %1%
" + tr("Denominated") + ": %2%
" + tr("Partially mixed") + ": %3%
" + tr("Mixed") + ": %4%
" + diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 1dfc195390..05a2d4239a 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -115,7 +115,7 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const return (qint64)rec->nodeStats.nodeid; case Address: // prepend to peer address down-arrow symbol for inbound connection and up-arrow for outbound connection - return QString(rec->nodeStats.fInbound ? "↓ " : "↑ ") + QString::fromStdString(rec->nodeStats.m_addr_name); + return QString::fromStdString((rec->nodeStats.fInbound ? "↓ " : "↑ ") + rec->nodeStats.m_addr_name); case ConnectionType: return GUIUtil::ConnectionTypeToQString(rec->nodeStats.m_conn_type, /* prepend_direction */ false); case Network: diff --git a/src/qt/recentrequeststablemodel.cpp b/src/qt/recentrequeststablemodel.cpp index 5c9623f9ff..c28fef6490 100644 --- a/src/qt/recentrequeststablemodel.cpp +++ b/src/qt/recentrequeststablemodel.cpp @@ -17,6 +17,9 @@ #include +#include +#include + RecentRequestsTableModel::RecentRequestsTableModel(WalletModel *parent) : QAbstractTableModel(parent), walletModel(parent) { @@ -126,7 +129,11 @@ void RecentRequestsTableModel::updateAmountColumnTitle() /** Gets title for amount column including current display unit if optionsModel reference available. */ QString RecentRequestsTableModel::getAmountTitle() { - return (this->walletModel->getOptionsModel() != nullptr) ? tr("Requested") + " ("+BitcoinUnits::name(this->walletModel->getOptionsModel()->getDisplayUnit()) + ")" : ""; + if (!walletModel->getOptionsModel()) return {}; + return tr("Requested") + + QLatin1String(" (") + + BitcoinUnits::name(this->walletModel->getOptionsModel()->getDisplayUnit()) + + QLatin1Char(')'); } QModelIndex RecentRequestsTableModel::index(int row, int column, const QModelIndex &parent) const diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 15140e9645..3a8b297037 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -201,7 +201,7 @@ void TestGUI(interfaces::Node& node) QCOMPARE(uri.count("amount=0.00000001"), 2); QCOMPARE(receiveRequestDialog->QObject::findChild("amount_tag")->text(), QString("Amount:")); - QCOMPARE(receiveRequestDialog->QObject::findChild("amount_content")->text(), QString("0.00000001 ") + BitcoinUnits::name(unit)); + QCOMPARE(receiveRequestDialog->QObject::findChild("amount_content")->text(), QString::fromStdString("0.00000001 ") + BitcoinUnits::name(unit)); QCOMPARE(uri.count("label=TEST_LABEL_1"), 2); QCOMPARE(receiveRequestDialog->QObject::findChild("label_tag")->text(), QString("Label:")); diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 0e89ffe556..2f90ca679f 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -26,6 +26,8 @@ #include #include +#include + QString TransactionDesc::FormatTxStatus(const interfaces::WalletTx& wtx, const interfaces::WalletTxStatus& status, bool inMempool, int numBlocks) { if (!status.is_final) @@ -44,19 +46,20 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTx& wtx, const i bool fChainLocked = status.is_chainlocked; if (nDepth == 0) { - strTxStatus = tr("0/unconfirmed, %1").arg((inMempool ? tr("in memory pool") : tr("not in memory pool"))) + (status.is_abandoned ? ", "+tr("abandoned") : ""); + const QString abandoned{status.is_abandoned ? QLatin1String(", ") + tr("abandoned") : QString()}; + strTxStatus = tr("0/unconfirmed, %1").arg((inMempool ? tr("in memory pool") : tr("not in memory pool"))) + abandoned; } else if (!fChainLocked && nDepth < 6) { strTxStatus = tr("%1/unconfirmed").arg(nDepth); } else { strTxStatus = tr("%1 confirmations").arg(nDepth); if (fChainLocked) { - strTxStatus += ", " + tr("locked via ChainLocks"); + strTxStatus += QLatin1String(", ") + tr("locked via ChainLocks"); return strTxStatus; } } if (status.is_islocked) { - strTxStatus += ", " + tr("verified via InstantSend"); + strTxStatus += QLatin1String(", ") + tr("verified via InstantSend"); } return strTxStatus; diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 20ac423b1c..0a7b0066c3 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include #include @@ -466,9 +468,9 @@ QVariant TransactionTableModel::txAddressDecoration(const TransactionRecord *wtx QString TransactionTableModel::formatTxToAddress(const TransactionRecord *wtx, bool tooltip) const { QString watchAddress; - if (tooltip) { + if (tooltip && wtx->involvesWatchAddress) { // Mark transactions involving watch-only addresses by adding " (watch-only)" - watchAddress = wtx->involvesWatchAddress ? QString(" (") + tr("watch-only") + QString(")") : ""; + watchAddress = QLatin1String(" (") + tr("watch-only") + QLatin1Char(')'); } switch(wtx->type)