Based on the patch from https://bugs.kde.org/show_bug.cgi?id=371511
See also https://bugreports.qt.io/browse/QTBUG-72260
BUG: 371511
mlaurent |
KDE PIM (Applications 18.12 (master)) |
Based on the patch from https://bugs.kde.org/show_bug.cgi?id=371511
See also https://bugreports.qt.io/browse/QTBUG-72260
BUG: 371511
No Linters Available |
No Unit Test Coverage |
Buildable 6034 | |
Build 6052: arc lint + arc unit |
src/articleviewer-ng/webengine/articleviewerwebengine.cpp | ||
---|---|---|
81 | Note: You dont actually need a member at all here, no? |
src/articleviewer-ng/webengine/articleviewerwebengine.cpp | ||
---|---|---|
81 | Right, it is possible without it as well. |
src/articleviewer-ng/webengine/articleviewerwebengine.cpp | ||
---|---|---|
68 | ~AkregatorRequestInterceptor() override = default(); no ? |
src/articleviewer-ng/webengine/articleviewerwebengine.cpp | ||
---|---|---|
68 | Is there any point in using 'override' on destructors? There is no function signature to trigger the usual warning case (derived class defines a function with a different signature, so not really replacing the base class virtual), so there is no way a destructor can be written to not override. The compiler doesn't consider destructors in its override warning. The only way in which 'override' would be useful on a destructor would be to give an error if the base class destructor was not declared virtual, but with a correctly written base class this should not arise. |
src/articleviewer-ng/webengine/articleviewerwebengine.cpp | ||
---|---|---|
68 | I removed the destructor completely. |
src/articleviewer-ng/webengine/articleviewerwebengine.cpp | ||
---|---|---|
68 | Sorry for getting more off-topic, but /me wants to clarify this:
Right, and why not protect against this? I think adding an override to dtors makes a lot of sense. Especially if you think of override as of: "I expect there to exist a base class version of this function which is declared virtual". Also see discussion on qt-development: https://lists.qt-project.org/pipermail/development/2018-August/033437.html |
FYI: Patch looks good to me now, but I cannot judge whether it fixed the crash. So cannot give a +1.
All I can say that I use it since I've created the patch and Akregator never crashed when closing a tab. :)