Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Efficiency, Readability, and Performance in Various Cppcheck Modules #6650

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Executor::Executor(const std::list<FileWithDetails> &files, const std::list<File
: mFiles(files), mFileSettings(fileSettings), mSettings(settings), mSuppressions(suppressions), mErrorLogger(errorLogger)
{
// the two inputs may only be used exclusively
assert(!(!files.empty() && !fileSettings.empty()));
assert(files.empty() || fileSettings.empty());
}

// TODO: this logic is duplicated in CppCheck::reportErr()
Expand Down
6 changes: 3 additions & 3 deletions cli/singleexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ unsigned int SingleExecutor::check()
std::size_t processedsize = 0;
unsigned int c = 0;

for (std::list<FileWithDetails>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have not changed such loops yet since they are in a mutable scope and you cannot fully enforce the intent of constness in a range loop until C++17 which introduces std::as_const (I have been trying to add a helper for that).

That seems pedantic but since changing the code does not actually change anything it doesn't matter which style you use.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coincidentally I finally ran into some code which highlights the issue - see #6653.

result += mCppcheck.check(*i);
processedsize += i->size();
for (const auto& file : mFiles) {
result += mCppcheck.check(file);
processedsize += file.size();
++c;
if (!mSettings.quiet)
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
Expand Down
9 changes: 2 additions & 7 deletions gui/aboutdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

AboutDialog::AboutDialog(const QString &version, const QString &extraVersion, QWidget *parent)
: QDialog(parent)
, mUI(new Ui::About)
, mUI(std::make_unique<Ui::About>())
{
mUI->setupUi(this);

Expand All @@ -39,9 +39,4 @@ AboutDialog::AboutDialog(const QString &version, const QString &extraVersion, QW
QString url = "<a href=\"https://cppcheck.sourceforge.io/\">https://cppcheck.sourceforge.io/</a>";
mUI->mHomepage->setText(mUI->mHomepage->text().arg(url));
connect(mUI->mButtons, &QDialogButtonBox::accepted, this, &AboutDialog::accept);
}

AboutDialog::~AboutDialog()
{
delete mUI;
}
}
5 changes: 3 additions & 2 deletions gui/aboutdialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <QDialog>
#include <QObject>
#include <QString>
#include <memory>

class QWidget;
namespace Ui {
Expand All @@ -42,10 +43,10 @@ class AboutDialog : public QDialog {
const QString &extraVersion,
QWidget *parent = nullptr);

~AboutDialog() override;
~AboutDialog() override = default;

private:
Ui::About* mUI;
std::unique_ptr<Ui::About> mUI;
};
/// @}
#endif // ABOUT_DIALOG_H
8 changes: 1 addition & 7 deletions gui/applicationdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ ApplicationDialog::ApplicationDialog(const QString &title,
Application &app,
QWidget *parent) :
QDialog(parent),
mUI(new Ui::ApplicationDialog),
mUI(std::make_unique<Ui::ApplicationDialog>()),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::make_unique is not available until C++14 and the code targets C++11.

mApplication(app)
{
mUI->setupUi(this);
Expand All @@ -50,12 +50,6 @@ ApplicationDialog::ApplicationDialog(const QString &title,
adjustSize();
}


ApplicationDialog::~ApplicationDialog()
{
delete mUI;
}

void ApplicationDialog::browse()
{
QString filter;
Expand Down
5 changes: 3 additions & 2 deletions gui/applicationdialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <QDialog>
#include <QObject>
#include <QString>
#include <memory>

class QWidget;
class Application;
Expand Down Expand Up @@ -51,7 +52,7 @@ class ApplicationDialog : public QDialog {
ApplicationDialog(const QString &title,
Application &app,
QWidget *parent = nullptr);
~ApplicationDialog() override;
~ApplicationDialog() override = default;

protected slots:

Expand All @@ -69,7 +70,7 @@ protected slots:
* @brief UI from the Qt designer
*
*/
Ui::ApplicationDialog* mUI;
std::unique_ptr<Ui::ApplicationDialog> mUI;

private:

Expand Down
Loading