From 0a44ce381e80517bd78ddce161e4614c11b9735b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 31 Oct 2020 23:16:48 +0100 Subject: [PATCH 1/5] [partition] Reduce warnings (Clang, FreeBSD) - remove unused this captures from lambda - rename variables that are short, cryptic, and shadowed - remove documentation for parameters that don't exist --- src/modules/partition/gui/ChoicePage.cpp | 27 +++++++++++++----------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index a91c9a8e1..2b68cafe7 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -906,7 +906,9 @@ ChoicePage::updateDeviceStatePreview() m_beforePartitionBarsView->setSelectionMode( QAbstractItemView::SingleSelection ); m_beforePartitionLabelsView->setSelectionMode( QAbstractItemView::SingleSelection ); break; - default: + case InstallChoice::NoChoice: + case InstallChoice::Erase: + case InstallChoice::Manual: m_beforePartitionBarsView->setSelectionMode( QAbstractItemView::NoSelection ); m_beforePartitionLabelsView->setSelectionMode( QAbstractItemView::NoSelection ); } @@ -990,7 +992,7 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice ) m_previewAfterFrame->show(); m_previewAfterLabel->show(); - SelectionFilter filter = [this]( const QModelIndex& index ) { + SelectionFilter filter = []( const QModelIndex& index ) { return PartUtils::canBeResized( static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ) ); }; @@ -1079,7 +1081,7 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice ) } else { - SelectionFilter filter = [this]( const QModelIndex& index ) { + SelectionFilter filter = []( const QModelIndex& index ) { return PartUtils::canBeReplaced( static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ) ); }; @@ -1125,7 +1127,9 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice ) case InstallChoice::Alongside: previewSelectionMode = QAbstractItemView::SingleSelection; break; - default: + case InstallChoice::NoChoice: + case InstallChoice::Erase: + case InstallChoice::Manual: previewSelectionMode = QAbstractItemView::NoSelection; } @@ -1179,15 +1183,15 @@ ChoicePage::setupEfiSystemPartitionSelector() QComboBox* ChoicePage::createBootloaderComboBox( QWidget* parent ) { - QComboBox* bcb = new QComboBox( parent ); - bcb->setModel( m_core->bootLoaderModel() ); + QComboBox* comboForBootloader = new QComboBox( parent ); + comboForBootloader->setModel( m_core->bootLoaderModel() ); // When the chosen bootloader device changes, we update the choice in the PCM - connect( bcb, QOverload< int >::of( &QComboBox::currentIndexChanged ), this, [this]( int newIndex ) { - QComboBox* bcb = qobject_cast< QComboBox* >( sender() ); - if ( bcb ) + connect( comboForBootloader, QOverload< int >::of( &QComboBox::currentIndexChanged ), this, [this]( int newIndex ) { + QComboBox* bootloaderCombo = qobject_cast< QComboBox* >( sender() ); + if ( bootloaderCombo ) { - QVariant var = bcb->itemData( newIndex, BootLoaderModel::BootLoaderPathRole ); + QVariant var = bootloaderCombo->itemData( newIndex, BootLoaderModel::BootLoaderPathRole ); if ( !var.isValid() ) { return; @@ -1196,7 +1200,7 @@ ChoicePage::createBootloaderComboBox( QWidget* parent ) } } ); - return bcb; + return comboForBootloader; } @@ -1220,7 +1224,6 @@ operator<<( QDebug& s, PartitionIterator& it ) * @brief ChoicePage::setupActions happens every time a new Device* is selected in the * device picker. Sets up the text and visibility of the partitioning actions based * on the currently selected Device*, bootloader and os-prober output. - * @param currentDevice */ void ChoicePage::setupActions() From 4d444cbfbb1b09efea32ee1b5432bfc3d1d9893b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 31 Oct 2020 23:37:06 +0100 Subject: [PATCH 2/5] [partition] Reduce warnings (Clang, FreeBSD) The code doesn't match the comment: there are no by-ref captures in the code, and the shadowing of parameters and local variables is confusing. Remove one variable that is passed in as an argument (and just pass the value as argument) and copy-capture the other rather than doing weird argument passing. --- src/modules/partition/gui/ChoicePage.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 2b68cafe7..35aec51e4 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -736,14 +736,12 @@ ChoicePage::doReplaceSelectedPartition( const QModelIndex& current ) return; } + // This will be deleted by the second lambda, below. QString* homePartitionPath = new QString(); - bool doReuseHomePartition = m_reuseHomeCheckBox->isChecked(); - // NOTE: using by-ref captures because we need to write homePartitionPath and - // doReuseHomePartition *after* the device revert, for later use. ScanningDialog::run( QtConcurrent::run( - [this, current]( QString* homePartitionPath, bool doReuseHomePartition ) { + [this, current, homePartitionPath]( bool doReuseHomePartition ) { QMutexLocker locker( &m_coreMutex ); if ( m_core->isDirty() ) @@ -823,9 +821,8 @@ ChoicePage::doReplaceSelectedPartition( const QModelIndex& current ) } } }, - homePartitionPath, - doReuseHomePartition ), - [=] { + m_reuseHomeCheckBox->isChecked() ), + [this, homePartitionPath] { m_reuseHomeCheckBox->setVisible( !homePartitionPath->isEmpty() ); if ( !homePartitionPath->isEmpty() ) m_reuseHomeCheckBox->setText( tr( "Reuse %1 as home partition for %2." ) From c41ff94f8aca8020c274ebf52a36381d47374d87 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 31 Oct 2020 23:48:20 +0100 Subject: [PATCH 3/5] [locale] Reduce warnings (C++17) Now that Calamares is compiled as C++17, we get this: src/modules/locale/timezonewidget/TimeZoneImage.cpp:28:55: warning: out-of-line definition of constexpr static data member is redundant in C++17 and is deprecated [-Wdeprecated] /* static constexpr */ const QSize TimeZoneImageList::imageSize; --- src/modules/locale/timezonewidget/TimeZoneImage.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/modules/locale/timezonewidget/TimeZoneImage.cpp b/src/modules/locale/timezonewidget/TimeZoneImage.cpp index 54aa1afd5..ad772ef63 100644 --- a/src/modules/locale/timezonewidget/TimeZoneImage.cpp +++ b/src/modules/locale/timezonewidget/TimeZoneImage.cpp @@ -24,9 +24,6 @@ static_assert( TimeZoneImageList::zoneCount == ( sizeof( zoneNames ) / sizeof( z #define ZONE_NAME QStringLiteral( "zone" ) -/* static constexpr */ const int TimeZoneImageList::zoneCount; -/* static constexpr */ const QSize TimeZoneImageList::imageSize; - static_assert( TimeZoneImageList::zoneCount == 37, "Incorrect number of zones" ); TimeZoneImageList::TimeZoneImageList() {} From d26fde664745f4b35751f26106d325ac02f08974 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 1 Nov 2020 00:05:05 +0100 Subject: [PATCH 4/5] [partition] Reduce warnings - Add a helper header that munges the warnings-settings so that KPMcore headers can be included. --- src/libcalamares/partition/KPMHelper.h | 36 +++++++++++++++++++ .../partition/tests/PartitionJobTests.cpp | 4 +-- 2 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 src/libcalamares/partition/KPMHelper.h diff --git a/src/libcalamares/partition/KPMHelper.h b/src/libcalamares/partition/KPMHelper.h new file mode 100644 index 000000000..900c973a2 --- /dev/null +++ b/src/libcalamares/partition/KPMHelper.h @@ -0,0 +1,36 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2020 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Calamares is Free Software: see the License-Identifier above. + * + */ + +/* + * KPMCore header file inclusion. + * + * Includes the system KPMCore headers without warnings (by switching off + * the expected warnings). + */ +#ifndef PARTITION_KPMHELPER_H +#define PARTITION_KPMHELPER_H + +// The kpmcore headers are not C++17 warning-proof, especially +// with picky compilers like Clang 10. Since we use Clang for the +// find-all-the-warnings case, switch those warnings off for +// the we-can't-change-them system headers. +#ifdef __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdocumentation" +#pragma clang diagnostic ignored "-Wsuggest-destructor-override" +#endif + +#include +#include + +#ifdef __clang__ +#pragma clang diagnostic pop +#endif + +#endif diff --git a/src/modules/partition/tests/PartitionJobTests.cpp b/src/modules/partition/tests/PartitionJobTests.cpp index 2b0b1a1dc..204819fef 100644 --- a/src/modules/partition/tests/PartitionJobTests.cpp +++ b/src/modules/partition/tests/PartitionJobTests.cpp @@ -15,14 +15,12 @@ #include "jobs/CreatePartitionTableJob.h" #include "jobs/ResizePartitionJob.h" +#include "partition/KPMHelper.h" #include "partition/KPMManager.h" #include "partition/PartitionQuery.h" #include "utils/Logger.h" #include "utils/Units.h" -#include -#include - #include #include #include From 4a08fdbb92fbdd1dd5ec13bfb99ceca6fd2fb407 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 1 Nov 2020 00:20:17 +0100 Subject: [PATCH 5/5] [partition] Reduce warnings from KPMCore - make KPMHelper.h an "everything include" and suppress warnings from it, then use it in the tests. --- src/libcalamares/partition/KPMHelper.h | 7 +++++++ src/modules/partition/tests/CreateLayoutsTests.cpp | 4 ---- src/modules/partition/tests/CreateLayoutsTests.h | 3 ++- src/modules/partition/tests/PartitionJobTests.h | 7 +------ 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/libcalamares/partition/KPMHelper.h b/src/libcalamares/partition/KPMHelper.h index 900c973a2..f6007b119 100644 --- a/src/libcalamares/partition/KPMHelper.h +++ b/src/libcalamares/partition/KPMHelper.h @@ -24,9 +24,16 @@ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdocumentation" #pragma clang diagnostic ignored "-Wsuggest-destructor-override" +#pragma clang diagnostic ignored "-Winconsistent-missing-destructor-override" #endif #include +#include +#include +#include +#include +#include +#include #include #ifdef __clang__ diff --git a/src/modules/partition/tests/CreateLayoutsTests.cpp b/src/modules/partition/tests/CreateLayoutsTests.cpp index 6a56eee99..fb991fc82 100644 --- a/src/modules/partition/tests/CreateLayoutsTests.cpp +++ b/src/modules/partition/tests/CreateLayoutsTests.cpp @@ -15,10 +15,6 @@ #include "partition/KPMManager.h" #include "utils/Logger.h" -#include -#include -#include - #include #include diff --git a/src/modules/partition/tests/CreateLayoutsTests.h b/src/modules/partition/tests/CreateLayoutsTests.h index 2ecc7b634..5953b06a7 100644 --- a/src/modules/partition/tests/CreateLayoutsTests.h +++ b/src/modules/partition/tests/CreateLayoutsTests.h @@ -10,8 +10,9 @@ #ifndef CLEARMOUNTSJOBTESTS_H #define CLEARMOUNTSJOBTESTS_H +#include "partition/KPMHelper.h" + #include -#include class CreateLayoutsTests : public QObject { diff --git a/src/modules/partition/tests/PartitionJobTests.h b/src/modules/partition/tests/PartitionJobTests.h index c2c01088f..9e4455ddc 100644 --- a/src/modules/partition/tests/PartitionJobTests.h +++ b/src/modules/partition/tests/PartitionJobTests.h @@ -12,12 +12,7 @@ #include "JobQueue.h" -// CalaPM -#include -#include -#include -#include -#include +#include "partition/KPMHelper.h" // Qt #include