Patch review

Yuriy Skalko yuriy.skalko at gmail.com
Wed Sep 29 10:01:42 UTC 2021


> Older gcc will have problem:
> 
> make[6]: Entering directory '/root/lyx/src/frontends/qt'
>   CXX      ColorCache.o
>   CXX      GuiBibtex.o
> GuiBibtex.cpp: In member function 'void 
> lyx::frontend::GuiBibtex::setFileEncodings(const 
> std::__debug::vector<std::basic_string<wchar_t, std::char_traits<wchar_t>, 
> std::allocator<wchar_t> > >&)':
> GuiBibtex.cpp:529:44: error: conversion from 'int' to 'Qt::MatchFlags {aka 
> QFlags<Qt::MatchFlag>}' is ambiguous
>             Qt::MatchExactly | Qt::MatchWrap);
 > ...
> 
> Pavel

Thanks for testing, Pavel. I've had a suspicion to this part with bit 
operators. I've removed it in the updated patch.


> Besides the problems reported by Pavel (which I can reproduce even in C++17 mode with gcc 5.5), I like the patch.
> How confident are we that gcc is right when it says that 'T(foo)' is equivalent to 'foo' when foo is of type 'T'?
> 
> JMarc

I skipped messages related to `char_type` since that can be defined 
differently on different systems. The rest should be OK.


Yuriy
-------------- next part --------------
From 04db31562072528a5d5ae05a01b999a3026e208d Mon Sep 17 00:00:00 2001
From: Yuriy Skalko <yuriy.skalko at gmail.com>
Date: Wed, 29 Sep 2021 12:49:21 +0300
Subject: [PATCH] Remove useless casts reported by GCC with -Wuseless-cast
 option

---
 src/BiblioInfo.cpp                    |  2 +-
 src/Buffer.cpp                        |  2 +-
 src/Converter.cpp                     |  2 +-
 src/LaTeX.cpp                         |  2 +-
 src/ParIterator.cpp                   |  6 +++---
 src/frontends/qt/ColorCache.cpp       |  2 +-
 src/frontends/qt/GuiBox.cpp           |  2 +-
 src/frontends/qt/GuiDocument.cpp      |  6 +++---
 src/frontends/qt/GuiExternal.cpp      |  2 +-
 src/frontends/qt/GuiMathMatrix.cpp    |  4 ++--
 src/frontends/qt/GuiPrefs.cpp         |  2 +-
 src/frontends/qt/GuiSendto.cpp        |  2 +-
 src/frontends/qt/GuiView.cpp          |  4 ++--
 src/frontends/qt/Menus.cpp            |  6 ++----
 src/insets/InsetGraphics.cpp          |  2 +-
 src/insets/InsetIPAMacro.cpp          |  4 ++--
 src/insets/InsetTabular.cpp           | 16 ++++++++--------
 src/mathed/InsetMathMacroTemplate.cpp |  2 +-
 18 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/src/BiblioInfo.cpp b/src/BiblioInfo.cpp
index 85c3e884d9..638579a752 100644
--- a/src/BiblioInfo.cpp
+++ b/src/BiblioInfo.cpp
@@ -1402,7 +1402,7 @@ docstring const BiblioInfo::getInfo(docstring const & key,
 {
 	BiblioInfo::const_iterator it = find(key);
 	if (it == end())
-		return docstring(_("Bibliography entry not found!"));
+		return _("Bibliography entry not found!");
 	BibTeXInfo const & data = it->second;
 	BibTeXInfoList xrefptrs;
 	for (docstring const & xref : getXRefs(data)) {
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index b2aaf18535..99d844b615 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -3825,7 +3825,7 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
 			// FIXME (Abdel), I don't understand why we pass 'it' here
 			// instead of 'macroTemplate' defined above... is this correct?
 			macros[macroTemplate.name()][it] =
-				Impl::ScopeMacro(scope, MacroData(const_cast<Buffer *>(owner_), it));
+				Impl::ScopeMacro(scope, MacroData(owner_, it));
 		}
 
 		// next paragraph
diff --git a/src/Converter.cpp b/src/Converter.cpp
index b4340c8f52..4e9a2ad426 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -853,7 +853,7 @@ Converters::RetVal Converters::runLaTeX(Buffer const & buffer, string const & co
 
 	// do the LaTeX run(s)
 	string const name = buffer.latexName();
-	LaTeX latex(command, runparams, FileName(makeAbsPath(name)),
+	LaTeX latex(command, runparams, makeAbsPath(name),
 	            buffer.filePath(), buffer.layoutPos(),
 	            buffer.isClone(), buffer.freshStartRequired());
 	TeXErrors terr;
diff --git a/src/LaTeX.cpp b/src/LaTeX.cpp
index 6e7fe921b8..a8d14b1f74 100644
--- a/src/LaTeX.cpp
+++ b/src/LaTeX.cpp
@@ -795,7 +795,7 @@ int LaTeX::scanLogFile(TeXErrors & terr)
 	string tmp =
 		onlyFileName(changeExtension(file.absFileName(), ".log"));
 	LYXERR(Debug::LATEX, "Log file: " << tmp);
-	FileName const fn = FileName(makeAbsPath(tmp));
+	FileName const fn = makeAbsPath(tmp);
 	// FIXME we should use an ifdocstream here and a docstring for token
 	// below. The encoding of the log file depends on the _output_ (font)
 	// encoding of the TeX file (T1, TU etc.). See #10728.
diff --git a/src/ParIterator.cpp b/src/ParIterator.cpp
index 7289889360..10de5f312d 100644
--- a/src/ParIterator.cpp
+++ b/src/ParIterator.cpp
@@ -69,7 +69,7 @@ ParIterator & ParIterator::operator--()
 
 Paragraph & ParIterator::operator*() const
 {
-	return const_cast<Paragraph&>(text()->getPar(pit()));
+	return text()->getPar(pit());
 }
 
 
@@ -81,7 +81,7 @@ pit_type ParIterator::pit() const
 
 Paragraph * ParIterator::operator->() const
 {
-	return const_cast<Paragraph*>(&text()->getPar(pit()));
+	return &text()->getPar(pit());
 }
 
 
@@ -93,7 +93,7 @@ pit_type ParIterator::outerPar() const
 
 ParagraphList & ParIterator::plist() const
 {
-	return const_cast<ParagraphList&>(text()->paragraphs());
+	return text()->paragraphs();
 }
 
 
diff --git a/src/frontends/qt/ColorCache.cpp b/src/frontends/qt/ColorCache.cpp
index 822f44a9d1..821494edc1 100644
--- a/src/frontends/qt/ColorCache.cpp
+++ b/src/frontends/qt/ColorCache.cpp
@@ -23,7 +23,7 @@ namespace{
 
 QPalette::ColorRole role(ColorCode col)
 {
-	switch (ColorCode(col)) {
+	switch (col) {
 	case Color_background:
 	case Color_commentbg:
 	case Color_greyedoutbg:
diff --git a/src/frontends/qt/GuiBox.cpp b/src/frontends/qt/GuiBox.cpp
index 14cbaf7f5b..179ba52675 100644
--- a/src/frontends/qt/GuiBox.cpp
+++ b/src/frontends/qt/GuiBox.cpp
@@ -172,7 +172,7 @@ void GuiBox::fillComboColor(QComboBox * combo, bool const is_none)
 	for (; cit != color_codes_.end(); ++cit) {
 		QString const latexname = toqstr(lcolor.getLaTeXName(*cit));
 		QString const guiname = toqstr(translateIfPossible(lcolor.getGUIName(*cit)));
-		color = QColor(guiApp->colorCache().get(*cit, false));
+		color = guiApp->colorCache().get(*cit, false);
 		coloritem.fill(color);
 		combo->addItem(QIcon(coloritem), guiname, latexname);
 	}
diff --git a/src/frontends/qt/GuiDocument.cpp b/src/frontends/qt/GuiDocument.cpp
index 40d7bc0bc2..9389272335 100644
--- a/src/frontends/qt/GuiDocument.cpp
+++ b/src/frontends/qt/GuiDocument.cpp
@@ -1029,7 +1029,7 @@ GuiDocument::GuiDocument(GuiView & lv)
 		if (encvar.unsafe() ||encvar.guiName().empty()
 		    || utf8_base_encodings.contains(toqstr(encvar.name())))
 			continue;
-		if (std::string(encvar.name()).find("utf8") == 0)
+		if (encvar.name().find("utf8") == 0)
 			encodingmap_utf8.insert(qt_(encvar.guiName()), toqstr(encvar.name()));
 		else
 			encodingmap.insert(qt_(encvar.guiName()), toqstr(encvar.name()));
@@ -5165,7 +5165,7 @@ GuiDocument::modInfoStruct GuiDocument::modInfo(LyXModule const & mod)
 	QString const guiname = toqstr(translateIfPossible(from_utf8(mod.getName())));
 	m.missingreqs = !isModuleAvailable(mod.getID());
 	if (m.missingreqs) {
-		m.name = QString(qt_("%1 (missing req.)")).arg(guiname);
+		m.name = qt_("%1 (missing req.)").arg(guiname);
 	} else
 		m.name = guiname;
 	m.category = mod.category().empty() ? qt_("Miscellaneous")
@@ -5178,7 +5178,7 @@ GuiDocument::modInfoStruct GuiDocument::modInfo(LyXModule const & mod)
 		desc.truncate(pos);
 	m.local = mod.isLocal();
 	QString const mtype = m.local ? qt_("personal module") : qt_("distributed module");
-	QString modulename = QString(qt_("<b>Module name:</b> <i>%1</i> (%2)")).arg(toqstr(m.id)).arg(mtype);
+	QString modulename = qt_("<b>Module name:</b> <i>%1</i> (%2)").arg(toqstr(m.id)).arg(mtype);
 	// Tooltip is the desc followed by the module name and the type
 	m.description = QString("%1%2")
 		.arg(desc.isEmpty() ? QString() : QString("<p>%1</p>").arg(desc),
diff --git a/src/frontends/qt/GuiExternal.cpp b/src/frontends/qt/GuiExternal.cpp
index 4cd02d28ad..70d6d7c936 100644
--- a/src/frontends/qt/GuiExternal.cpp
+++ b/src/frontends/qt/GuiExternal.cpp
@@ -445,7 +445,7 @@ static void getSize(external::ResizeData & data,
 		data.scale = widgetToDoubleStr(&widthED);
 		data.width = Length();
 	} else {
-		data.width = Length(widgetsToLength(&widthED, &widthUnitCO));
+		data.width = widgetsToLength(&widthED, &widthUnitCO);
 		data.scale = string();
 	}
 	data.height = Length(widgetsToLength(&heightED, &heightUnitCO));
diff --git a/src/frontends/qt/GuiMathMatrix.cpp b/src/frontends/qt/GuiMathMatrix.cpp
index d339e2c976..db062781f0 100644
--- a/src/frontends/qt/GuiMathMatrix.cpp
+++ b/src/frontends/qt/GuiMathMatrix.cpp
@@ -101,7 +101,7 @@ GuiMathMatrix::GuiMathMatrix(GuiView & lv)
 
 void GuiMathMatrix::columnsChanged(int)
 {
-	int const nx = int(columnsSB->value());
+	int const nx = columnsSB->value();
 	halignED->setText(QString(nx, 'c'));
 }
 
@@ -159,7 +159,7 @@ void GuiMathMatrix::slotOK()
 		// otherwise create just a standard AMS matrix
 		if (sh.contains('l') || sh.contains('r') || sh.contains('|')) {
 			string const str_ams = fromqstr(
-				QString("%1 %2 %3").arg(int(1)).arg(int(1)).arg(deco_name));
+				QString("%1 %2 %3").arg(1).arg(1).arg(deco_name));
 			dispatch(FuncRequest(LFUN_MATH_AMS_MATRIX, str_ams));
 		} else {
 			string const str_ams = fromqstr(
diff --git a/src/frontends/qt/GuiPrefs.cpp b/src/frontends/qt/GuiPrefs.cpp
index e80254b1fb..427a294879 100644
--- a/src/frontends/qt/GuiPrefs.cpp
+++ b/src/frontends/qt/GuiPrefs.cpp
@@ -1151,7 +1151,7 @@ void PrefColors::applyRC(LyXRC & rc) const
 void PrefColors::updateRC(LyXRC const & rc)
 {
 	for (size_type i = 0; i < lcolors_.size(); ++i) {
-		QColor color = QColor(guiApp->colorCache().get(lcolors_[i], false));
+		QColor color = guiApp->colorCache().get(lcolors_[i], false);
 		QPixmap coloritem(32, 32);
 		coloritem.fill(color);
 		lyxObjectsLW->item(int(i))->setIcon(QIcon(coloritem));
diff --git a/src/frontends/qt/GuiSendto.cpp b/src/frontends/qt/GuiSendto.cpp
index ec40ff3dcb..1f561789df 100644
--- a/src/frontends/qt/GuiSendto.cpp
+++ b/src/frontends/qt/GuiSendto.cpp
@@ -112,7 +112,7 @@ bool GuiSendTo::isValid()
 {
 	int const line = formatLW->currentRow();
 
-	if (line < 0 || (line > int(formatLW->count())))
+	if (line < 0 || (line > formatLW->count()))
 		return false;
 
 	return (!formatLW->selectedItems().empty()
diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index 303bcfc3a5..45b9a42b80 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -1183,7 +1183,7 @@ bool GuiView::prepareAllBuffersForLogout()
 	// We cannot use a for loop as the buffer list cycles.
 	Buffer * b = first;
 	do {
-		if (!saveBufferIfNeeded(const_cast<Buffer &>(*b), false))
+		if (!saveBufferIfNeeded(*b, false))
 			return false;
 		b = theBufferList().next(b);
 	} while (b != first);
@@ -3172,7 +3172,7 @@ bool GuiView::exportBufferAs(Buffer & b, docstring const & iformat)
 		return false;
 
 	// fname is now the new Buffer location.
-	if (FileName(fname).exists()) {
+	if (fname.exists()) {
 		docstring const file = makeDisplayPath(fname.absFileName(), 30);
 		docstring text = bformat(_("The document %1$s already "
 					   "exists.\n\nDo you want to "
diff --git a/src/frontends/qt/Menus.cpp b/src/frontends/qt/Menus.cpp
index a32a59675f..a99294e82e 100644
--- a/src/frontends/qt/Menus.cpp
+++ b/src/frontends/qt/Menus.cpp
@@ -1302,8 +1302,7 @@ void MenuDefinition::expandToc2(Toc const & toc_list,
 						label += QString::number(++shortcut_count);
 				}
 			}
-			add(MenuItem(MenuItem::Command, label,
-					    FuncRequest(toc_list[i].action())));
+			add(MenuItem(MenuItem::Command, label, toc_list[i].action()));
 			// separator after the menu heading
 			if (toc_list[i].depth() < depth)
 				add(MenuItem(MenuItem::Separator));
@@ -1331,8 +1330,7 @@ void MenuDefinition::expandToc2(Toc const & toc_list,
 				break;
 			}
 			if (new_pos == pos + 1) {
-				add(MenuItem(MenuItem::Command,
-						    label, FuncRequest(toc_list[pos].action())));
+				add(MenuItem(MenuItem::Command, label, toc_list[pos].action()));
 			} else {
 				MenuDefinition sub;
 				sub.expandToc2(toc_list, pos, new_pos, depth + 1, toc_type);
diff --git a/src/insets/InsetGraphics.cpp b/src/insets/InsetGraphics.cpp
index e8c389d74c..577d1cc007 100644
--- a/src/insets/InsetGraphics.cpp
+++ b/src/insets/InsetGraphics.cpp
@@ -722,7 +722,7 @@ string InsetGraphics::prepareFile(OutputParams const & runparams) const
 
 	if (from == to) {
 		// source and destination formats are the same
-		if (!runparams.nice && !FileName(temp_file).hasExtension(ext)) {
+		if (!runparams.nice && !temp_file.hasExtension(ext)) {
 			// The LaTeX compiler will not be able to determine
 			// the file format from the extension, so we must
 			// change it.
diff --git a/src/insets/InsetIPAMacro.cpp b/src/insets/InsetIPAMacro.cpp
index 0cb1a5ff06..154e9bc702 100644
--- a/src/insets/InsetIPAMacro.cpp
+++ b/src/insets/InsetIPAMacro.cpp
@@ -287,7 +287,7 @@ int InsetIPADeco::plaintext(odocstringstream & os,
 			    OutputParams const & runparams, size_t max_length) const
 {
 	odocstringstream ods;
-	int h = (int)(InsetCollapsible::plaintext(ods, runparams, max_length) / 2);
+	int h = InsetCollapsible::plaintext(ods, runparams, max_length) / 2;
 	docstring result = ods.str();
 	docstring const before = result.substr(0, h);
 	docstring const after = result.substr(h, result.size());
@@ -311,7 +311,7 @@ void InsetIPADeco::docbook(XMLStream & xs, OutputParams const & runparams) const
 	// The special combining character must be put in the middle, between the two other characters.
 	// It will not work if there is anything else than two pure characters, so going back to plaintext.
 	odocstringstream ods;
-	int h = (int)(InsetText::plaintext(ods, runparams) / 2);
+	int h = InsetText::plaintext(ods, runparams) / 2;
 	docstring result = ods.str();
 	docstring const before = result.substr(0, h);
 	docstring const after = result.substr(h, result.size());
diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp
index 503a239666..e98f951cf4 100644
--- a/src/insets/InsetTabular.cpp
+++ b/src/insets/InsetTabular.cpp
@@ -615,7 +615,7 @@ DocIterator separatorPos(InsetTableCell const * cell, docstring const & align_d)
 
 InsetTableCell splitCell(InsetTableCell & head, docstring const & align_d, bool & hassep)
 {
-	InsetTableCell tail = InsetTableCell(head);
+	InsetTableCell tail = head;
 	DocIterator const dit = separatorPos(&head, align_d);
 	hassep = static_cast<bool>(dit);
 	if (hassep) {
@@ -837,13 +837,13 @@ void Tabular::appendRow(row_type row)
 
 void Tabular::insertRow(row_type const row, bool copy)
 {
-	row_info.insert(row_info.begin() + row + 1, RowData(row_info[row]));
+	row_info.insert(row_info.begin() + row + 1, row_info[row]);
 	cell_info.insert(cell_info.begin() + row + 1,
 		cell_vector(0, CellData(buffer_)));
 
 	for (col_type c = 0; c < ncols(); ++c) {
 		cell_info[row + 1].insert(cell_info[row + 1].begin() + c,
-			copy ? CellData(cell_info[row][c]) : CellData(buffer_));
+			copy ? cell_info[row][c] : CellData(buffer_));
 		if (cell_info[row][c].multirow == CELL_BEGIN_OF_MULTIROW)
 			cell_info[row + 1][c].multirow = CELL_PART_OF_MULTIROW;
 	}
@@ -1000,11 +1000,11 @@ void Tabular::appendColumn(col_type col)
 void Tabular::insertColumn(col_type const col, bool copy)
 {
 	bool const ct = buffer().params().track_changes;
-	column_info.insert(column_info.begin() + col + 1, ColumnData(column_info[col]));
+	column_info.insert(column_info.begin() + col + 1, column_info[col]);
 
 	for (row_type r = 0; r < nrows(); ++r) {
 		cell_info[r].insert(cell_info[r].begin() + col + 1,
-			copy ? CellData(cell_info[r][col]) : CellData(buffer_));
+			copy ? cell_info[r][col] : CellData(buffer_));
 		if (cell_info[r][col].multicolumn == CELL_BEGIN_OF_MULTICOLUMN)
 			cell_info[r][col + 1].multicolumn = CELL_PART_OF_MULTICOLUMN;
 	}
@@ -4523,7 +4523,7 @@ void InsetTabular::metrics(MetricsInfo & mi, Dimension & dim) const
 			// determine horizontal offset because of decimal align (if necessary)
 			int decimal_width = 0;
 			if (tabular.getAlignment(cell) == LYX_ALIGN_DECIMAL) {
-				InsetTableCell tail = InsetTableCell(*tabular.cellInset(cell));
+				InsetTableCell tail = *tabular.cellInset(cell);
 				tail.setBuffer(tabular.buffer());
 				// we need to set macrocontext position everywhere
 				// otherwise we crash with nested insets (e.g. footnotes)
@@ -4776,7 +4776,7 @@ void InsetTabular::drawCellLines(PainterInfo & pi, int x, int y,
 	Color colour = Color_tabularline;
 	if (tabular.column_info[col].change.changed()
 	    || tabular.row_info[row].change.changed())
-		colour = InsetTableCell(*tabular.cellInset(cell)).paragraphs().front().lookupChange(0).color();
+		colour = tabular.cellInset(cell)->paragraphs().front().lookupChange(0).color();
 
 	// Top
 	bool drawline = tabular.topLine(cell)
@@ -6016,7 +6016,7 @@ bool InsetTabular::getStatus(Cursor & cur, FuncRequest const & cmd,
 		}
 		// check if there is already a caption
 		bool have_caption = false;
-		InsetTableCell itc = InsetTableCell(*tabular.cellInset(cur.idx()));
+		InsetTableCell itc = *tabular.cellInset(cur.idx());
 		ParagraphList::const_iterator pit = itc.paragraphs().begin();
 		ParagraphList::const_iterator pend = itc.paragraphs().end();
 		for (; pit != pend; ++pit) {
diff --git a/src/mathed/InsetMathMacroTemplate.cpp b/src/mathed/InsetMathMacroTemplate.cpp
index c552ba0dad..110d5ad0dd 100644
--- a/src/mathed/InsetMathMacroTemplate.cpp
+++ b/src/mathed/InsetMathMacroTemplate.cpp
@@ -676,7 +676,7 @@ int InsetMathMacroTemplate::maxArgumentInDefinition() const
 		if (it.nextInset()->lyxCode() != MATH_MACROARG_CODE)
 			continue;
 		InsetMathMacroArgument * arg = static_cast<InsetMathMacroArgument*>(it.nextInset());
-		maxArg = std::max(int(arg->number()), maxArg);
+		maxArg = std::max(arg->number(), maxArg);
 	}
 	return maxArg;
 }
-- 
2.28.0.windows.1



More information about the lyx-devel mailing list