Re: [PATCH] Add override specifier
> It wouldn't be awful, but we do not want to allow assignments to these > things. That's why that's there, and undefined. > > Riki > Is not your patch like: > > InsetTableCell::InsetTableCell(InsetTableCell const & in) = default; > > Or I am in the wrong standard? > -- > José Abílio If assignments should be forbidden, this will be better: InsetTableCell::InsetTableCell(InsetTableCell const & in) = delete; Yuriy -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 7 octobre 2020 02:32:06 GMT+02:00, Richard Kimberly Heck a écrit : InsetTableCell::InsetTableCell(InsetTableCell const & in) = default; >> >> >> Or I am in the wrong standard? >> >I think this must be the magic JMarc was looking for. This is in C++11, >so OK for us, right? I like it! JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On 10/6/20 8:18 PM, José Abílio Matos wrote: > > On Monday, October 5, 2020 6:27:15 PM WEST Richard Kimberly Heck wrote: > > > I'm actually not sure why the assignment operator is made unavailable, > > > but attached is a suitable patch, I think. > > > > > > Riki > > > Is not your patch like: > > > InsetTableCell::InsetTableCell(InsetTableCell const & in) = default; > > > Or I am in the wrong standard? > I think this must be the magic JMarc was looking for. This is in C++11, so OK for us, right? Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On Monday, October 5, 2020 6:27:15 PM WEST Richard Kimberly Heck wrote: > I'm actually not sure why the assignment operator is made unavailable, > but attached is a suitable patch, I think. > > Riki Is not your patch like: InsetTableCell::InsetTableCell(InsetTableCell const & in) = default; Or I am in the wrong standard? -- José Abílio-- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On 10/6/20 5:29 PM, Pavel Sanda wrote: > On Tue, Oct 06, 2020 at 11:04:55PM +0200, Jean-Marc Lasgouttes wrote: >> Le 05/10/2020 ?? 19:27, Richard Kimberly Heck a écrit : PS: yes, a proper fix will be eventually required, but I do not really know what we should do. >>> I'm actually not sure why the assignment operator is made unavailable, >>> but attached is a suitable patch, I think. >> It looks reasonable, but I really do not know why we have to do this. I >> hoped there was some nice idiomatic C++11 magic that would get us out of >> this mess. Adding this code feels like going 10 years backwards. > I don't have clang here, but what would happen if we deleted > void operator=(InsetTableCell const &); > in the first place? It wouldn't be awful, but we do not want to allow assignments to these things. That's why that's there, and undefined. Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On Tue, Oct 06, 2020 at 11:04:55PM +0200, Jean-Marc Lasgouttes wrote: > Le 05/10/2020 ?? 19:27, Richard Kimberly Heck a écrit : > >>PS: yes, a proper fix will be eventually required, but I do not really > >>know what we should do. > > > >I'm actually not sure why the assignment operator is made unavailable, > >but attached is a suitable patch, I think. > > It looks reasonable, but I really do not know why we have to do this. I > hoped there was some nice idiomatic C++11 magic that would get us out of > this mess. Adding this code feels like going 10 years backwards. I don't have clang here, but what would happen if we deleted void operator=(InsetTableCell const &); in the first place? Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 05/10/2020 à 19:27, Richard Kimberly Heck a écrit : PS: yes, a proper fix will be eventually required, but I do not really know what we should do. I'm actually not sure why the assignment operator is made unavailable, but attached is a suitable patch, I think. It looks reasonable, but I really do not know why we have to do this. I hoped there was some nice idiomatic C++11 magic that would get us out of this mess. Adding this code feels like going 10 years backwards. JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 05/10/2020 à 21:38, José Abílio Matos a écrit : I created a new directory. And then ran configure on that directory using the CC and CXX environment variables to force clang. This is what I get. I am using qt-5.15.1 FWIW: /home/jamatos/lyx/lyx/src/frontends/qt/GuiView.cpp:221:11: warning: 'width' is deprecated: Use QFontMetrics::horizontalAdvance [-Wdeprecated-declarations] if (fm.width(*sit) > wline) This one is in our code, and I remember that Scott mentionned it. I'm afraid I forgot about it. The previous one was in qt-generated code. Do you have enchant2 installed? If you do and LyX does not use it, then it is a bug. I see that we check for enchant-2 with pkg-config, but it might be that enchant2 is better for you. For some reason I did not had enchant2-devel installed. After the installation it seems to be used and the warning goes away. Great. JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On Monday, October 5, 2020 6:09:40 PM WEST Jean-Marc Lasgouttes wrote:
> Le 05/10/2020 à 18:43, José Abílio Matos a écrit :
> > On Monday, October 5, 2020 5:19:46 PM WEST Jean-Marc Lasgouttes wrote:
> >> Fixed
> >>
> >> JMarc
> >
> > Now, if I may ask what about the following kind of warning:
> >
> > In file included from ./ui_TabularUi.h:28:
> > /home/jamatos/lyx/lyx/src/frontends/qt/GuiSetBorder.h:30:59: warning:
> > 'QFlags' is deprecated: Use default constructor instead
> > [-Wdeprecated-declarations] GuiSetBorder(QWidget * parent = nullptr,
> > Qt::WindowFlags fl =
> > nullptr);
>
> Are you sure that your ui_TabularUi.h has been built by your current qt
> version? It is weird to see Qt complain about code that it generated.
Yes.
I created a new directory. And then ran configure on that directory using the
CC and CXX environment variables to force clang.
This is what I get. I am using qt-5.15.1 FWIW:
/home/jamatos/lyx/lyx/src/frontends/qt/GuiView.cpp:221:11: warning: 'width' is
deprecated: Use QFontMetrics::horizontalAdvance [-Wdeprecated-declarations]
if (fm.width(*sit) > wline)
^
/usr/include/qt5/QtGui/qfontmetrics.h:105:5: note: 'width' has been explicitly
marked deprecated here
QT_DEPRECATED_X("Use QFontMetrics::horizontalAdvance")
^
/usr/include/qt5/QtCore/qglobal.h:294:33: note: expanded from macro
'QT_DEPRECATED_X'
# define QT_DEPRECATED_X(text) Q_DECL_DEPRECATED_X(text)
^
/usr/include/qt5/QtCore/qcompilerdetection.h:675:55: note: expanded from macro
'Q_DECL_DEPRECATED_X'
#define Q_DECL_DEPRECATED_X(text) __attribute__ ((__deprecated__(text)))
^
Probably is more a question of the qt version.
> > There is another type but this seems to be the single instance:
> > In file included from /home/jamatos/lyx/lyx/src/EnchantChecker.cpp:14:
> > /usr/include/enchant/enchant++.h:55:25: warning:
> > 'enchant::Exception::what'
> > hides overloaded virtual function [-Woverloaded-virtual]
> > virtual const char * what () throw() {
>
> Do you have enchant2 installed? If you do and LyX does not use it, then
> it is a bug. I see that we check for enchant-2 with pkg-config, but it
> might be that enchant2 is better for you.
For some reason I did not had enchant2-devel installed. After the installation
it seems to be used and the warning goes away.
> JMarc
Best regards,
--
José Abílio
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On 10/5/20 12:19 PM, Jean-Marc Lasgouttes wrote:
> Le 05/10/2020 à 17:50, José Abílio Matos a écrit :
>> On Monday, October 5, 2020 2:13:35 PM WEST Jean-Marc Lasgouttes wrote:
>>> Yes, clang 10 does
>>>
>>> JMarc
>>
>> BTW compiling with clang 11 and without changing the compile flags I
>> get lots
>> (in the tens) of warnings like this:
>>
>> /home/jamatos/lyx/lyx/src/insets/InsetTabular.h:92:7: warning:
>> definition of
>> implicit copy constructor for 'InsetTableCell' is deprecated because
>> it has a
>> user-declared copy assignment operator [-Wdeprecated-copy]
>> void operator=(InsetTableCell const &);
>
> Fixed ;)
>
> JMarc
>
> PS: yes, a proper fix will be eventually required, but I do not really
> know what we should do.
I'm actually not sure why the assignment operator is made unavailable,
but attached is a suitable patch, I think.
Riki
>From 75572d0af787bbdacc430256c257de9e9ded0100 Mon Sep 17 00:00:00 2001
From: Richard Kimberly Heck
Date: Mon, 5 Oct 2020 13:11:48 -0400
Subject: [PATCH] Explicit InsetTableCell copy constructor
---
src/insets/InsetTabular.cpp | 7 +++
src/insets/InsetTabular.h | 4 +++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp
index 8458ce4728..00a9c8c9a5 100644
--- a/src/insets/InsetTabular.cpp
+++ b/src/insets/InsetTabular.cpp
@@ -4207,6 +4207,13 @@ InsetTableCell::InsetTableCell(Buffer * buf)
isMultiColumn(false), isMultiRow(false), contentAlign(LYX_ALIGN_CENTER)
{}
+InsetTableCell::InsetTableCell(InsetTableCell const & in) : InsetText(in)
+{
+ isFixedWidth = in.isFixedWidth;
+ isMultiColumn = in.isMultiColumn;
+ isMultiRow = in.isMultiRow;
+ contentAlign = in.contentAlign;
+}
bool InsetTableCell::forcePlainLayout(idx_type) const
{
diff --git a/src/insets/InsetTabular.h b/src/insets/InsetTabular.h
index fd942049ba..2be6e8cb01 100644
--- a/src/insets/InsetTabular.h
+++ b/src/insets/InsetTabular.h
@@ -55,6 +55,8 @@ public:
///
explicit InsetTableCell(Buffer * buf);
///
+ InsetTableCell(InsetTableCell const &);
+ ///
InsetCode lyxCode() const { return CELL_CODE; }
///
Inset * clone() const { return new InsetTableCell(*this); }
@@ -113,7 +115,7 @@ private:
// iterating, since this information is needed quite often, and so may
// be quite slow.
// So, well, if someone can do better, please do!
- // --rgh
+ // --rkh
///
bool isFixedWidth;
///
--
2.25.4
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 05/10/2020 à 18:43, José Abílio Matos a écrit :
On Monday, October 5, 2020 5:19:46 PM WEST Jean-Marc Lasgouttes wrote:
Fixed
JMarc
Now, if I may ask what about the following kind of warning:
In file included from ./ui_TabularUi.h:28:
/home/jamatos/lyx/lyx/src/frontends/qt/GuiSetBorder.h:30:59: warning: 'QFlags'
is deprecated: Use default constructor instead [-Wdeprecated-declarations]
GuiSetBorder(QWidget * parent = nullptr, Qt::WindowFlags fl =
nullptr);
Are you sure that your ui_TabularUi.h has been built by your current qt
version? It is weird to see Qt complain about code that it generated.
There is another type but this seems to be the single instance:
In file included from /home/jamatos/lyx/lyx/src/EnchantChecker.cpp:14:
/usr/include/enchant/enchant++.h:55:25: warning: 'enchant::Exception::what'
hides overloaded virtual function [-Woverloaded-virtual]
virtual const char * what () throw() {
Do you have enchant2 installed? If you do and LyX does not use it, then
it is a bug. I see that we check for enchant-2 with pkg-config, but it
might be that enchant2 is better for you.
JMarc
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On Monday, October 5, 2020 5:19:46 PM WEST Jean-Marc Lasgouttes wrote:
> Fixed
>
> JMarc
Now, if I may ask what about the following kind of warning:
In file included from ./ui_TabularUi.h:28:
/home/jamatos/lyx/lyx/src/frontends/qt/GuiSetBorder.h:30:59: warning: 'QFlags'
is deprecated: Use default constructor instead [-Wdeprecated-declarations]
GuiSetBorder(QWidget * parent = nullptr, Qt::WindowFlags fl =
nullptr);
^
/usr/include/qt5/QtCore/qflags.h:123:5: note: 'QFlags' has been explicitly
marked deprecated here
QT_DEPRECATED_X("Use default constructor instead") Q_DECL_CONSTEXPR inline
QFlags(Zero) noexcept : i(0) {}
^
/usr/include/qt5/QtCore/qglobal.h:294:33: note: expanded from macro
'QT_DEPRECATED_X'
# define QT_DEPRECATED_X(text) Q_DECL_DEPRECATED_X(text)
^
/usr/include/qt5/QtCore/qcompilerdetection.h:675:55: note: expanded from macro
'Q_DECL_DEPRECATED_X'
#define Q_DECL_DEPRECATED_X(text) __attribute__ ((__deprecated__(text)))
^
There is another type but this seems to be the single instance:
In file included from /home/jamatos/lyx/lyx/src/EnchantChecker.cpp:14:
/usr/include/enchant/enchant++.h:55:25: warning: 'enchant::Exception::what'
hides overloaded virtual function [-Woverloaded-virtual]
virtual const char * what () throw() {
^
/usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/bits/
exception.h:75:5: note: hidden overloaded virtual function
'std::exception::what' declared here: different qualifiers ('const' vs
unqualified)
what() const _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_NOTHROW;
^
--
José Abílio
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 05/10/2020 à 17:50, José Abílio Matos a écrit : On Monday, October 5, 2020 2:13:35 PM WEST Jean-Marc Lasgouttes wrote: Yes, clang 10 does JMarc BTW compiling with clang 11 and without changing the compile flags I get lots (in the tens) of warnings like this: /home/jamatos/lyx/lyx/src/insets/InsetTabular.h:92:7: warning: definition of implicit copy constructor for 'InsetTableCell' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy] void operator=(InsetTableCell const &); Fixed ;) JMarc PS: yes, a proper fix will be eventually required, but I do not really know what we should do. -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On 10/5/20 11:50 AM, José Abílio Matos wrote:
> On Monday, October 5, 2020 2:13:35 PM WEST Jean-Marc Lasgouttes wrote:
>> Yes, clang 10 does
>>
>> JMarc
> BTW compiling with clang 11 and without changing the compile flags I get lots
> (in the tens) of warnings like this:
>
> /home/jamatos/lyx/lyx/src/insets/InsetTabular.h:92:7: warning: definition of
> implicit copy constructor for 'InsetTableCell' is deprecated because it has a
> user-declared copy assignment operator [-Wdeprecated-copy]
> void operator=(InsetTableCell const &);
> ^
> /home/jamatos/lyx/lyx/src/insets/InsetTabular.h:60:37: note: in implicit copy
> constructor for 'lyx::InsetTableCell' first required here
> Inset * clone() const { return new InsetTableCell(*this); }
I have seen things like this from time to time. I guess we need to
declare our own copy constructor, too
Riki
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
On Monday, October 5, 2020 2:13:35 PM WEST Jean-Marc Lasgouttes wrote:
> Yes, clang 10 does
>
> JMarc
BTW compiling with clang 11 and without changing the compile flags I get lots
(in the tens) of warnings like this:
/home/jamatos/lyx/lyx/src/insets/InsetTabular.h:92:7: warning: definition of
implicit copy constructor for 'InsetTableCell' is deprecated because it has a
user-declared copy assignment operator [-Wdeprecated-copy]
void operator=(InsetTableCell const &);
^
/home/jamatos/lyx/lyx/src/insets/InsetTabular.h:60:37: note: in implicit copy
constructor for 'lyx::InsetTableCell' first required here
Inset * clone() const { return new InsetTableCell(*this); }
--
José Abílio
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 05/10/2020 à 12:26, Yuriy Skalko a écrit : Yes, I missed these. Now it should be complete. Does Clang agree with me? :) Yes, clang 10 does :) JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Am Mon, 5 Oct 2020 13:26:18 +0300 schrieb Yuriy Skalko : > > This one is in too. > > > > We are almost there :) > > > > JMarc > > Yes, I missed these. Now it should be complete. Does Clang agree with me? :) > > Yuriy No warnings now concerning 'override' with clang8.0.0 Kornel pgpxZQXUfR3HR.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
> This one is in too. > > We are almost there :) > > JMarc Yes, I missed these. Now it should be complete. Does Clang agree with me? :) Yuriy From 8f625dd2931ad3a90187c1f70579c614003be87c Mon Sep 17 00:00:00 2001 From: Yuriy Skalko Date: Mon, 5 Oct 2020 13:22:55 +0300 Subject: [PATCH] Amend efc0877f Add the last `override`s. --- src/frontends/qt/GuiView.h | 2 +- src/frontends/qt/GuiViewSource.h | 2 +- src/frontends/qt/LayoutBox.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/frontends/qt/GuiView.h b/src/frontends/qt/GuiView.h index 6bf5ba3208..9c248c 100644 --- a/src/frontends/qt/GuiView.h +++ b/src/frontends/qt/GuiView.h @@ -223,7 +223,7 @@ Q_SIGNALS: public Q_SLOTS: /// - void setBusy(bool); + void setBusy(bool) override; /// idle timeout. /// clear any temporary message and replace with current status. void clearMessage(); diff --git a/src/frontends/qt/GuiViewSource.h b/src/frontends/qt/GuiViewSource.h index 47221c93f6..785c249006 100644 --- a/src/frontends/qt/GuiViewSource.h +++ b/src/frontends/qt/GuiViewSource.h @@ -55,7 +55,7 @@ public: protected: /// - void resizeEvent (QResizeEvent * event); + void resizeEvent (QResizeEvent * event) override; public Q_SLOTS: /// diff --git a/src/frontends/qt/LayoutBox.h b/src/frontends/qt/LayoutBox.h index 7b7828ab41..3980b6f0e6 100644 --- a/src/frontends/qt/LayoutBox.h +++ b/src/frontends/qt/LayoutBox.h @@ -46,7 +46,7 @@ public: bool sorted, bool sortedByCat, bool unknown); /// - void showPopup(); + void showPopup() override; /// bool eventFilter(QObject * o, QEvent * e) override; -- 2.28.0.windows.1 -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 04/10/2020 à 18:18, Yuriy Skalko a écrit : And now clang++ 10 complains a lot... And this patch should fix the rest. This one is in too. We are almost there :) JMarc In file included from ../../../../master/src/frontends/qt/Dialog.cpp:15: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ In file included from ../../../../master/src/frontends/qt/DialogView.cpp:14: In file included from ../../../../master/src/frontends/qt/DialogView.h:16: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ 1 warning generated. In file included from ../../../../master/src/frontends/qt/DockView.cpp:14: In file included from ../../../../master/src/frontends/qt/DockView.h:16: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ 1 warning generated. 1 warning generated. In file included from ../../../../master/src/frontends/qt/FindAndReplace.cpp:13: In file included from ../../../../master/src/frontends/qt/FindAndReplace.h:15: In file included from ../../../../master/src/frontends/qt/DockView.h:16: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ In file included from ../../../../master/src/frontends/qt/GuiAbout.cpp:13: In file included from ../../../../master/src/frontends/qt/GuiAbout.h:15: In file included from ../../../../master/src/frontends/qt/DialogView.h:16: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ In file included from ../../../../master/src/frontends/qt/GuiApplication.cpp:23: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ 1 warning generated. 1 warning generated. In file included from ../../../../master/src/frontends/qt/GuiCitation.cpp:17: In file included from ../../../../master/src/frontends/qt/GuiCitation.h:18: In file included from ../../../../master/src/frontends/qt/DialogView.h:16: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ In file included from ../../../../master/src/frontends/qt/GuiCommandBuffer.cpp:19: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ In file included from ../../../../master/src/frontends/qt/GuiCompare.cpp:21: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning: 'setBusy' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void setBusy(bool); ^ ../../../../master/src/frontends/Delegates.h:72:15: note: overridden virtual function is here virtual void setBusy(bool) = 0; ^ In file included from ../../../../master/src/frontends/qt/GuiCompareHistory.cpp:20: ../../../../master/src/frontends/qt/GuiView.h:226:7: warning:
Re: [PATCH] Add override specifier
On Sun, Oct 04, 2020 at 07:18:07PM +0300, Yuriy Skalko wrote: > > And now clang++ 10 complains a lot... > > And this patch should fix the rest. Looks good to me. P -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
> And now clang++ 10 complains a lot...
And this patch should fix the rest.
Yuriy
From 3786af0ec675cab02d48581ac07f97090534e0f3 Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Sun, 4 Oct 2020 17:56:53 +0300
Subject: [PATCH] Amend efc0877f
Add more `override` specifiers. Replace `throw()` to `noexcept`.
---
src/Converter.h| 4 +-
src/Encoding.h | 2 +-
src/frontends/qt/CategorizedCombo.h| 4 +-
src/frontends/qt/CustomizedWidgets.h | 2 +-
src/frontends/qt/EmptyTable.h | 4 +-
src/frontends/qt/FancyLineEdit.h | 6 +--
src/frontends/qt/FindAndReplace.h | 6 +--
src/frontends/qt/GuiBranches.h | 2 +-
src/frontends/qt/GuiClipboard.h| 22 -
src/frontends/qt/GuiCommandEdit.h | 2 +-
src/frontends/qt/GuiCompleter.h| 2 +-
src/frontends/qt/GuiPrefs.h| 68 +-
src/frontends/qt/GuiProgress.h | 26 +-
src/frontends/qt/GuiSelection.h| 8 +--
src/frontends/qt/GuiSelectionManager.h | 2 +-
src/frontends/qt/GuiSetBorder.h| 4 +-
src/frontends/qt/GuiSpellchecker.h | 2 +-
src/frontends/qt/GuiToolbar.h | 10 ++--
src/frontends/qt/GuiView.h | 24 -
src/frontends/qt/GuiViewSource.h | 2 +-
src/frontends/qt/GuiWorkArea.h | 42
src/frontends/qt/IconPalette.h | 10 ++--
src/frontends/qt/InsertTableWidget.h | 8 +--
src/frontends/qt/LayoutBox.h | 2 +-
src/frontends/qt/PanelStack.h | 2 +-
src/frontends/qt/ToolTipFormatter.h| 2 +-
src/frontends/qt/Validator.h | 10 ++--
src/insets/InsetTabular.h | 4 +-
src/mathed/InsetMathRoot.h | 2 +-
src/mathed/MathCompletionList.h| 8 +--
src/support/ExceptionMessage.h | 4 +-
src/support/RefChanger.h | 4 +-
src/support/docstream.cpp | 14 +++---
src/support/docstream.h| 4 +-
src/support/docstring.cpp | 18 +++
35 files changed, 168 insertions(+), 168 deletions(-)
diff --git a/src/Converter.h b/src/Converter.h
index fe95be6ed8..e9efa00f4f 100644
--- a/src/Converter.h
+++ b/src/Converter.h
@@ -33,8 +33,8 @@ class Formats;
class ConversionException : public std::exception {
public:
ConversionException() {}
- virtual ~ConversionException() throw() {}
- virtual const char * what() const throw()
+ virtual ~ConversionException() noexcept {}
+ const char * what() const noexcept override
{ return "Exception caught in conversion routine!"; }
};
diff --git a/src/Encoding.h b/src/Encoding.h
index 6bd90c7b80..76b022dd9d 100644
--- a/src/Encoding.h
+++ b/src/Encoding.h
@@ -29,7 +29,7 @@ class EncodingException : public std::exception {
public:
EncodingException(char_type c);
virtual ~EncodingException() noexcept {}
- virtual const char * what() const noexcept;
+ virtual const char * what() const noexcept override;
char_type failed_char;
int par_id;
diff --git a/src/frontends/qt/CategorizedCombo.h
b/src/frontends/qt/CategorizedCombo.h
index 3ce7da7883..be351ff993 100644
--- a/src/frontends/qt/CategorizedCombo.h
+++ b/src/frontends/qt/CategorizedCombo.h
@@ -57,10 +57,10 @@ public:
QString const & unavail =
QString());
///
- void showPopup();
+ void showPopup() override;
///
- bool eventFilter(QObject * o, QEvent * e);
+ bool eventFilter(QObject * o, QEvent * e) override;
///
QString const & filter() const;
diff --git a/src/frontends/qt/CustomizedWidgets.h
b/src/frontends/qt/CustomizedWidgets.h
index 69373c21c1..ca68ca99cf 100644
--- a/src/frontends/qt/CustomizedWidgets.h
+++ b/src/frontends/qt/CustomizedWidgets.h
@@ -35,7 +35,7 @@ public:
void setKeySequence(lyx::KeySequence const & s);
void removeFromSequence();
protected Q_SLOTS:
- bool event(QEvent* e);
+ bool event(QEvent* e) override;
void keyPressEvent(QKeyEvent * e) override;
private:
void appendToSequence(QKeyEvent * e);
diff --git a/src/frontends/qt/EmptyTable.h b/src/frontends/qt/EmptyTable.h
index 833676d9b9..99265266d2 100644
--- a/src/frontends/qt/EmptyTable.h
+++ b/src/frontends/qt/EmptyTable.h
@@ -29,7 +29,7 @@ class EmptyTable : public QTableWidget {
public:
EmptyTable(QWidget * parent = 0, int rows = 5, int columns = 5);
- virtual QSize sizeHint() const;
+ QSize sizeHint() const override;
public Q_SLOTS:
/// set the number of columns in the table and emit colsChanged() signal
void setNumberColumns(int nr_cols);
@@ -43,7 +43,7 @@ Q_SIGNALS:
protected:
/// fill in a cell
virtual void paintCell(class QPainter *, int, int);
- virtual void mouseMoveEvent(QMouseEvent *);
+ void mouseMoveEvent(QMouseEvent
Re: [PATCH] Add override specifier
> And now clang++ 10 complains a lot...
>
> JMarc
This should fix it. I assume there will be more places...
Yuriy
From dd855c31bcb983ab533d1002ab7515b74ab4b290 Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Sat, 3 Oct 2020 15:42:14 +0300
Subject: [PATCH] Amend efc0877f
---
src/frontends/qt/CustomizedWidgets.h | 2 +-
src/frontends/qt/DialogView.h| 4 ++--
src/frontends/qt/DockView.h | 2 +-
src/frontends/qt/FancyLineEdit.h | 2 +-
src/frontends/qt/GuiCitation.h | 4 ++--
src/frontends/qt/GuiCommandEdit.h| 2 +-
src/frontends/qt/GuiCompare.h| 2 +-
src/frontends/qt/GuiDialog.h | 2 +-
src/frontends/qt/GuiDocument.h | 2 +-
src/frontends/qt/GuiErrorList.h | 2 +-
src/frontends/qt/GuiProgressView.h | 4 ++--
src/frontends/qt/GuiRef.h| 2 +-
src/frontends/qt/GuiSearch.h | 2 +-
src/frontends/qt/GuiToc.h| 2 +-
src/frontends/qt/GuiToolbar.h| 2 +-
src/frontends/qt/GuiView.h | 4 ++--
src/frontends/qt/GuiWorkArea.h | 6 +++---
src/frontends/qt/IconPalette.h | 4 ++--
src/frontends/qt/PanelStack.h| 2 +-
src/insets/InsetTabular.h| 4 ++--
src/mathed/InsetMathBig.h| 2 +-
src/mathed/InsetMathCancelto.h | 2 +-
src/mathed/InsetMathDecoration.h | 2 +-
src/mathed/InsetMathDelim.h | 2 +-
src/mathed/InsetMathExInt.h | 2 +-
src/mathed/InsetMathKern.h | 2 +-
src/mathed/InsetMathMacroArgument.h | 2 +-
src/mathed/InsetMathNumber.h | 2 +-
src/mathed/InsetMathOverset.h| 2 +-
src/mathed/InsetMathRoot.h | 4 ++--
src/mathed/InsetMathScript.h | 2 +-
src/mathed/InsetMathSize.h | 2 +-
src/mathed/InsetMathSqrt.h | 2 +-
src/mathed/InsetMathStackrel.h | 2 +-
src/mathed/InsetMathSymbol.h | 4 ++--
35 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/src/frontends/qt/CustomizedWidgets.h
b/src/frontends/qt/CustomizedWidgets.h
index 3409679556..69373c21c1 100644
--- a/src/frontends/qt/CustomizedWidgets.h
+++ b/src/frontends/qt/CustomizedWidgets.h
@@ -36,7 +36,7 @@ public:
void removeFromSequence();
protected Q_SLOTS:
bool event(QEvent* e);
- void keyPressEvent(QKeyEvent * e);
+ void keyPressEvent(QKeyEvent * e) override;
private:
void appendToSequence(QKeyEvent * e);
KeySequence keysequence_;
diff --git a/src/frontends/qt/DialogView.h b/src/frontends/qt/DialogView.h
index b99cdebfb8..bb90ab84fd 100644
--- a/src/frontends/qt/DialogView.h
+++ b/src/frontends/qt/DialogView.h
@@ -44,9 +44,9 @@ protected:
bool needBufferOpen() const override { return isBufferDependent(); }
//@}
/// Any dialog that overrides this method should make sure to call it.
- void closeEvent(QCloseEvent * ev);
+ void closeEvent(QCloseEvent * ev) override;
/// Any dialog that overrides this method should make sure to call it.
- void hideEvent(QHideEvent * ev);
+ void hideEvent(QHideEvent * ev) override;
protected Q_SLOTS:
void onBufferViewChanged() override {};
diff --git a/src/frontends/qt/DockView.h b/src/frontends/qt/DockView.h
index 897fb392ff..60a7afe9f1 100644
--- a/src/frontends/qt/DockView.h
+++ b/src/frontends/qt/DockView.h
@@ -48,7 +48,7 @@ public:
/// We don't want to restore geometry session for dock widgets.
void restoreSession() override {}
- void keyPressEvent(QKeyEvent * ev);
+ void keyPressEvent(QKeyEvent * ev) override;
/// Dialog inherited methods
//@{
diff --git a/src/frontends/qt/FancyLineEdit.h b/src/frontends/qt/FancyLineEdit.h
index 5a2a542861..470173effa 100644
--- a/src/frontends/qt/FancyLineEdit.h
+++ b/src/frontends/qt/FancyLineEdit.h
@@ -97,7 +97,7 @@ private Q_SLOTS:
protected:
virtual void resizeEvent(QResizeEvent *e);
- virtual void keyPressEvent(QKeyEvent *e);
+ virtual void keyPressEvent(QKeyEvent *e) override;
private:
void updateMargins();
diff --git a/src/frontends/qt/GuiCitation.h b/src/frontends/qt/GuiCitation.h
index a4784b20ec..db7ac31249 100644
--- a/src/frontends/qt/GuiCitation.h
+++ b/src/frontends/qt/GuiCitation.h
@@ -87,9 +87,9 @@ private:
//@}
///
- void showEvent(QShowEvent * e);
+ void showEvent(QShowEvent * e) override;
///
- void closeEvent(QCloseEvent * e);
+ void closeEvent(QCloseEvent * e) override;
/// prepares a call to GuiCitation::searchKeys when we
/// are ready to search the BibTeX entries
void findText(QString const & text, bool reset = false);
diff --git a/src/frontends/qt/GuiCommandEdit.h
b/src/frontends/qt/GuiCommandEdit.h
index 76833115cd..82a546a3dd 100644
--- a/src/frontends/qt/GuiCommandEdit.h
+++ b/src/frontends/qt/GuiCommandEdit.h
@@ -40,7 +40,7 @@ protected:
///
virtual bool event(QEvent * e);
///
- virtual void
Re: [PATCH] Add override specifier
Le 03/10/2020 à 13:19, Jean-Marc Lasgouttes a écrit : Le 02/10/2020 à 19:53, Yuriy Skalko a écrit : Here is an updated patch. Yes, there are more places to add the `override`. I'll try to change the rest on the second step. Is it OK to commit the patch in its current state? Thanks a lot. It is in now. And now clang++ 10 complains a lot... JMarc In file included from ../../../../master/src/frontends/qt/DialogView.cpp:14: ../../../../master/src/frontends/qt/DialogView.h:47:7: warning: 'closeEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void closeEvent(QCloseEvent * ev); ^ /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qdialog.h:105:10: note: overridden virtual function is here void closeEvent(QCloseEvent *) override; ^ In file included from ../../../../master/src/frontends/qt/DialogView.cpp:14: ../../../../master/src/frontends/qt/DialogView.h:49:7: warning: 'hideEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void hideEvent(QHideEvent * ev); ^ /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qwidget.h:647:18: note: overridden virtual function is here virtual void hideEvent(QHideEvent *event); ^ 2 warnings generated. In file included from ../../../../master/src/frontends/qt/DockView.cpp:14: ../../../../master/src/frontends/qt/DockView.h:51:7: warning: 'keyPressEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void keyPressEvent(QKeyEvent * ev); ^ /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qwidget.h:619:18: note: overridden virtual function is here virtual void keyPressEvent(QKeyEvent *event); ^ 1 warning generated. In file included from ../../../../master/src/frontends/qt/FindAndReplace.cpp:13: In file included from ../../../../master/src/frontends/qt/FindAndReplace.h:15: ../../../../master/src/frontends/qt/DockView.h:51:7: warning: 'keyPressEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void keyPressEvent(QKeyEvent * ev); ^ /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qwidget.h:619:18: note: overridden virtual function is here virtual void keyPressEvent(QKeyEvent *event); ^ In file included from ../../../../master/src/frontends/qt/GuiAbout.cpp:13: In file included from ../../../../master/src/frontends/qt/GuiAbout.h:15: ../../../../master/src/frontends/qt/DialogView.h:47:7: warning: 'closeEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void closeEvent(QCloseEvent * ev); ^ /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qdialog.h:105:10: note: overridden virtual function is here void closeEvent(QCloseEvent *) override; ^ In file included from ../../../../master/src/frontends/qt/GuiAbout.cpp:13: In file included from ../../../../master/src/frontends/qt/GuiAbout.h:15: ../../../../master/src/frontends/qt/DialogView.h:49:7: warning: 'hideEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void hideEvent(QHideEvent * ev); ^ /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qwidget.h:647:18: note: overridden virtual function is here virtual void hideEvent(QHideEvent *event); ^ 2 warnings generated. 1 warning generated. In file included from ../../../../master/src/frontends/qt/GuiBibtex.cpp:16: In file included from ../../../../master/src/frontends/qt/GuiBibtex.h:16: ../../../../master/src/frontends/qt/GuiDialog.h:65:7: warning: 'closeEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void closeEvent(QCloseEvent * e); ^ /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qdialog.h:105:10: note: overridden virtual function is here void closeEvent(QCloseEvent *) override; ^ 1 warning generated. In file included from ../../../../master/src/frontends/qt/GuiChanges.cpp:14: In file included from ../../../../master/src/frontends/qt/GuiChanges.h:16: ../../../../master/src/frontends/qt/GuiDialog.h:65:7: warning: 'closeEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void closeEvent(QCloseEvent * e); ^ /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qdialog.h:105:10: note: overridden virtual function is here void closeEvent(QCloseEvent *) override; ^ In file included from ../../../../master/src/frontends/qt/GuiCharacter.cpp:16: In file included from ../../../../master/src/frontends/qt/GuiCharacter.h:17: ../../../../master/src/frontends/qt/GuiDialog.h:65:7: warning: 'closeEvent' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] void closeEvent(QCloseEvent * e); ^
Re: [PATCH] Add override specifier
Le 02/10/2020 à 19:53, Yuriy Skalko a écrit : Here is an updated patch. Yes, there are more places to add the `override`. I'll try to change the rest on the second step. Is it OK to commit the patch in its current state? Thanks a lot. It is in now. JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
> Actually, boost 1.74 only uses override in a handful of cases. Not worth > updating now. > > Therefore, it does not seem doable to use -Wsuggest-override with gcc. > > JMarc This should be useful to disable such warnings from boost: https://stackoverflow.com/questions/3308523/how-to-eliminate-external-lib-third-party-warnings-in-gcc Yuriy -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
> I just did that and see that many files have now been handled, probably > because cppcheck did not warn about Qt headers: GuiView.h, GuiWorkArea.h... > > > Here is a log file for your enjoyment. > > I'll try to handle the boost part by updating our local copy to the latest > version 1.74 (which fixes this issue). > > > A solution to avoid issues with older systems where system boost is not the > latest would be to limit the introduction of -Wsuggest-override to gcc 10 and > upper (I do gcc 5+ for now). Clang has its own limited version which is on by > default. > > > JMarc Here is an updated patch. Yes, there are more places to add the `override`. I'll try to change the rest on the second step. Is it OK to commit the patch in its current state? Yuriy <> -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 02/10/2020 à 15:39, Jean-Marc Lasgouttes a écrit : Le 02/10/2020 à 15:08, Jean-Marc Lasgouttes a écrit : A few remarks * there are places with a double space before 'override' * it does not make sense to keep 'virtual' when overriding. Could you fix these issues and make the commit message a bit more consistent about why we do that? On my side, I will add the warning -Wsuggest-override to gcc options. I just did that and see that many files have now been handled, probably because cppcheck did not warn about Qt headers: GuiView.h, GuiWorkArea.h... Here is a log file for your enjoyment. I'll try to handle the boost part by updating our local copy to the latest version 1.74 (which fixes this issue). Actually, boost 1.74 only uses override in a handful of cases. Not worth updating now. Therefore, it does not seem doable to use -Wsuggest-override with gcc. JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 02/10/2020 à 15:08, Jean-Marc Lasgouttes a écrit : A few remarks * there are places with a double space before 'override' * it does not make sense to keep 'virtual' when overriding. Could you fix these issues and make the commit message a bit more consistent about why we do that? On my side, I will add the warning -Wsuggest-override to gcc options. I just did that and see that many files have now been handled, probably because cppcheck did not warn about Qt headers: GuiView.h, GuiWorkArea.h... Here is a log file for your enjoyment. I'll try to handle the boost part by updating our local copy to the latest version 1.74 (which fixes this issue). A solution to avoid issues with older systems where system boost is not the latest would be to limit the introduction of -Wsuggest-override to gcc 10 and upper (I do gcc 5+ for now). Clang has its own limited version which is on by default. JMarc >From deca25db1936b704a1e565df1272df3da223ee01 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Fri, 2 Oct 2020 15:37:32 +0200 Subject: [PATCH] Enable -Wsuggest-override when warnings are on with gcc>=5 This will keep us honest about what virtual methods we override --- config/lyxinclude.m4 | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/config/lyxinclude.m4 b/config/lyxinclude.m4 index 369ecaefbb..7769728bf6 100644 --- a/config/lyxinclude.m4 +++ b/config/lyxinclude.m4 @@ -410,11 +410,16 @@ if test x$GXX = xyes; then if test x$enable_warnings = xyes ; then AM_CPPFLAGS="$AM_CPPFLAGS -Wall -Wextra" case $gxx_version in - 9.*|10.*|clang-10*) + dnl the weird thing below is because we cannot use directly square braces + @<:@5-8@:>@.*) + AM_CXXFLAGS="$AM_CXXFLAGS -Wsuggest-override";; + 9.*|10.*) + AM_CXXFLAGS="$AM_CXXFLAGS -Wsuggest-override -Wno-deprecated-copy";; + clang-10*) AM_CXXFLAGS="$AM_CXXFLAGS -Wno-deprecated-copy";; *);; esac -fi + fi case $gxx_version in 2.*|3.*|4.@<:@0-6@:>@) AC_MSG_ERROR([gcc >= 4.7 is required]);; esac -- 2.25.1 -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [PATCH] Add override specifier
Le 01/10/2020 à 20:40, Yuriy Skalko a écrit : Hello, I've added `override` specifiers into the LyX codebase. This will improve code maintainability in future. To find overridden methods I've used the Cppcheck static analyzer. Probably some are missed and should be marked later. Jean-Marc already marked some of such methods in commit c8f4b684, I've continued this job. Good idea Yuriy! A few remarks * there are places with a double space before 'override' * it does not make sense to keep 'virtual' when overriding. Could you fix these issues and make the commit message a bit more consistent about why we do that? On my side, I will add the warning -Wsuggest-override to gcc options. JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
