[PATCH] Loop refactoring

Richard Kimberly Heck rikiheck at lyx.org
Wed Oct 7 15:31:29 UTC 2020


On 10/7/20 10:31 AM, Yuriy Skalko wrote:
> And the last patch based on the static analyzers output.

diff --git a/src/Author.cpp b/src/Author.cpp
index 9a2dc1ea43..b4cf9fd97b 100644
--- a/src/Author.cpp
+++ b/src/Author.cpp
@@ -31,8 +31,8 @@ static int computeHash(docstring const & name,
 	string const full_author_string = to_utf8(name + email);
 	// Bernstein's hash function
 	unsigned int hash = 5381;
-	for (unsigned int i = 0; i < full_author_string.length(); ++i)
-		hash = ((hash << 5) + hash) + (unsigned int)(full_author_string[i]);
+	for (char c : full_author_string)

It shouldn't matter much here, but this can presumably be const &.
There are other cases where it does matter, though, and I think
we will see other warnings about this. So I'd fix them all while
we're at it. I won't mark the other char and char_type ones.



diff --git a/src/LaTeXFeatures.cpp b/src/LaTeXFeatures.cpp
index 3d67128f58..f9261fd4a3 100644
--- a/src/LaTeXFeatures.cpp
+++ b/src/LaTeXFeatures.cpp
@@ -1258,9 +1258,9 @@ string const LaTeXFeatures::getPackages() const
 
 	//  These are all the 'simple' includes.  i.e
 	//  packages which we just \usepackage{package}
-	for (int i = 0; i < nb_simplefeatures; ++i) {
-		if (mustProvide(simplefeatures[i]))
-			packages << "\\usepackage{" << simplefeatures[i] << "}\n";
+	for (auto & feature : simplefeatures) {

This is a case where it definitely should be const &.

+	for (auto & feature : bibliofeatures) {

Same.

+		for (auto & change : changes) {

Another.

I think you can just search for "auto &" to find the rest.

diff --git a/src/frontends/qt/Dialog.cpp b/src/frontends/qt/Dialog.cpp
index a646a898f5..b09163b25a 100644
--- a/src/frontends/qt/Dialog.cpp
+++ b/src/frontends/qt/Dialog.cpp
@@ -49,10 +49,6 @@ Dialog::Dialog(GuiView & lv, QString const & name, QString const & title)
 {}
 
diff --git a/src/frontends/qt/GuiCitation.cpp b/src/frontends/qt/GuiCitation.cpp
index 16b958b4be..a4853ff9e8 100644
--- a/src/frontends/qt/GuiCitation.cpp
+++ b/src/frontends/qt/GuiCitation.cpp
@@ -706,8 +706,7 @@ void GuiCitation::setPreTexts(vector<docstring> const & m)
 			selected_model_.match(selected_model_.index(0, 1),
 					     Qt::DisplayRole, toqstr(key), -1,
 					     Qt::MatchFlags(Qt::MatchExactly | Qt::MatchWrap));
-		for (int i = 0; i < qmil.size(); ++i){
-			QModelIndex idx = qmil[i];
+		for (auto idx : qmil) {

Probably const & again.


@@ -745,8 +744,7 @@ void GuiCitation::setPostTexts(vector<docstring> const & m)
 			selected_model_.match(selected_model_.index(0, 1),
 					     Qt::DisplayRole, toqstr(key), -1,
 					     Qt::MatchFlags(Qt::MatchExactly | Qt::MatchWrap));
-		for (int i = 0; i < qmil.size(); ++i){
-			QModelIndex idx = qmil[i];
+		for (auto idx : qmil) {

Here too.

diff --git a/src/frontends/qt/GuiLyXFiles.cpp b/src/frontends/qt/GuiLyXFiles.cpp
index 8269c4d218..f723158141 100644
--- a/src/frontends/qt/GuiLyXFiles.cpp
+++ b/src/frontends/qt/GuiLyXFiles.cpp
@@ -463,10 +463,10 @@ void GuiLyXFiles::updateContents()
 			QTreeWidgetItem * subcatItem = nullptr;
 			if (cats.contains(catsave)) {
 				QList<QTreeWidgetItem *> pcats = filesLW->findItems(cat, Qt::MatchExactly);
-				for (int iit = 0; iit < pcats.size(); ++iit) {
-					for (int cit = 0; cit < pcats.at(iit)->childCount(); ++cit) {
-						if (pcats.at(iit)->child(cit)->text(0) == subcat) {
-							subcatItem = pcats.at(iit)->child(cit);
+				for (auto pcat : pcats) {

Again const &.


@@ -3039,9 +3039,9 @@ QTreeWidgetItem * PrefShortcuts::insertShortcutItem(FuncRequest const & lfun,
 	if (tag == KeyMap::UserUnbind) {
 		QList<QTreeWidgetItem*> const items = shortcutsTW->findItems(lfun_name,
 			Qt::MatchFlags(Qt::MatchExactly | Qt::MatchRecursive), 0);
-		for (int i = 0; i < items.size(); ++i) {
-			if (items[i]->text(1) == shortcut) {
-				newItem = items[i];
+		for (auto item : items) {

Another.


@@ -3128,8 +3128,7 @@ void PrefShortcuts::unhideEmpty(QString const & lfun, bool select)
 	// list of items that match lfun
 	QList<QTreeWidgetItem*> items = shortcutsTW->findItems(lfun,
 	     Qt::MatchFlags(Qt::MatchExactly | Qt::MatchRecursive), 0);
-	for (int i = 0; i < items.size(); ++i) {
-		QTreeWidgetItem * item = items[i];
+	for (auto item : items) {

Another.

@@ -3145,24 +3144,24 @@ void PrefShortcuts::removeShortcut()
 	// it seems that only one item can be selected, but I am
 	// removing all selected items anyway.
 	QList<QTreeWidgetItem*> items = shortcutsTW->selectedItems();
-	for (int i = 0; i < items.size(); ++i) {
-		string shortcut = fromqstr(items[i]->data(1, Qt::UserRole).toString());
-		string lfun = fromqstr(items[i]->text(0));
+	for (auto & item : items) {

Not this one! Few more like that follow.


diff --git a/src/frontends/qt/GuiToolbar.cpp b/src/frontends/qt/GuiToolbar.cpp
index 5c8c603daf..cb6c057c47 100644
--- a/src/frontends/qt/GuiToolbar.cpp
+++ b/src/frontends/qt/GuiToolbar.cpp
@@ -276,8 +276,8 @@ void StaticMenuButton::updateTriggered()
 
 	bool enabled = false;
 	QList<QAction *> acts = menu()->actions();
-	for (int i = 0; i < acts.size(); ++i)
-		if (acts[i]->isEnabled()) {
+	for (auto & act : acts)

const &.


@@ -561,8 +561,8 @@ void GuiToolbar::update(int context)
 
 	// This is a speed bottleneck because this is called on every keypress
 	// and update calls getStatus, which copies the cursor at least two times
-	for (int i = 0; i < actions_.size(); ++i)
-		actions_[i]->update();
+	for (auto & action : actions_)

Probably also.


diff --git a/src/frontends/qt/InsetParamsDialog.cpp b/src/frontends/qt/InsetParamsDialog.cpp
index 1abed922f9..7899ceee8f 100644
--- a/src/frontends/qt/InsetParamsDialog.cpp
+++ b/src/frontends/qt/InsetParamsDialog.cpp
@@ -204,9 +204,9 @@ docstring InsetParamsDialog::checkWidgets(bool immediate)
 	immediateApplyCB->setEnabled(ins && !read_only);
 	// This seems to be the only way to access custom buttons
 	QList<QAbstractButton*> buttons = buttonBox->buttons();
-	for (int i = 0; i < buttons.size(); ++i) {
-		if (buttonBox->buttonRole(buttons.at(i)) == QDialogButtonBox::ActionRole)
-			buttons.at(i)->setEnabled(widget_ok && !read_only
+	for (auto button : buttons) {

Can't hurt to be &, though since it's a pointer...


@@ -995,8 +995,8 @@ bool InsetMathHull::ams() const
 	case hullEqnArray:
 		break;
 	}
-	for (size_t row = 0; row < numbered_.size(); ++row)
-		if (numbered_[row] == NOTAG)
+	for (auto row : numbered_)

const &.


index 8c47b7a441..2f9bb15832 100644
--- a/src/support/qstring_helpers.cpp
+++ b/src/support/qstring_helpers.cpp
@@ -72,8 +72,7 @@ std::string fromqstr(QString const & str)
 QString charFilterRegExp(QString const & filter)
 {
 	QString re = ".*";
-	for (int i = 0; i < filter.length(); ++i) {
-		QChar c = filter[i];
+	for (QChar c : filter) {

const &, I think.


@@ -85,8 +84,7 @@ QString charFilterRegExp(QString const & filter)
 QString charFilterRegExpC(QString const & filter)
 {
 	QString re = "(";
-	for (int i = 0; i < filter.length(); ++i) {
-		QChar c = filter[i];
+	for (QChar c : filter) {

And here.


diff --git a/src/tex2lyx/table.cpp b/src/tex2lyx/table.cpp
index bd7225db0f..a790582066 100644
--- a/src/tex2lyx/table.cpp
+++ b/src/tex2lyx/table.cpp
@@ -589,8 +589,8 @@ void fix_colalign(vector<ColInfo> & colinfo)
 	}
 	// Move the lines and alignment settings to the special field if
 	// necessary
-	for (size_t col = 0; col < colinfo.size(); ++col)
-		fix_colalign(colinfo[col]);
+	for (auto & col : colinfo)

This one is fine.

 
@@ -921,16 +921,16 @@ void parse_table(Parser & p, ostream & os, bool is_long_tabular,
 void handle_hline_above(RowInfo & ri, vector<CellInfo> & ci)
 {
 	ri.topline = true;
-	for (size_t col = 0; col < ci.size(); ++col)
-		ci[col].topline = true;
+	for (auto & col : ci)

Here too.


 void handle_hline_below(RowInfo & ri, vector<CellInfo> & ci)
 {
 	ri.bottomline = true;
-	for (size_t col = 0; col < ci.size(); ++col)
-		ci[col].bottomline = true;
+	for (auto & col : ci)

And here.
 
 
@@ -1516,8 +1516,8 @@ void handle_tabular(Parser & p, ostream & os, string const & name,
 	//cerr << "// output what we have\n";
 	// output what we have
 	size_type cols = colinfo.size();
-	for (size_t col = 0; col < colinfo.size(); ++col) {
-		if (colinfo[col].decimal_point != '\0' && colinfo[col].align != 'd')
+	for (auto & col : colinfo) {

And here.


@@ -1545,18 +1545,18 @@ void handle_tabular(Parser & p, ostream & os, string const & name,
 	os << write_attribute("tabularwidth", tabularwidth) << ">\n";
 
 	//cerr << "// after header\n";
-	for (size_t col = 0; col < colinfo.size(); ++col) {
-		if (colinfo[col].decimal_point != '\0' && colinfo[col].align != 'd')
+	for (auto & col : colinfo) {

I think this one can be const.



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20201007/23e2805d/attachment.html>


More information about the lyx-devel mailing list