Valgrind: Collapse project and global settings classes

Change-Id: I41b157bb07a451851641c7a5785334bd28036f45
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/valgrind/callgrindtool.cpp

@@ -742,7 +742,7 @@ void CallgrindToolPrivate::setupRunner(CallgrindToolRunner *toolRunner)
     QTC_ASSERT(m_visualization, return);
 
     // apply project settings
-    ValgrindProjectSettings settings;
+    ValgrindSettings settings{false};
     settings.fromMap(runControl->settingsData(ANALYZER_VALGRIND_SETTINGS));
     m_visualization->setMinimumInclusiveCostRatio(settings.visualizationMinimumInclusiveCostRatio() / 100.0);
     m_proxyModel.setMinimumInclusiveCostRatio(settings.minimumInclusiveCostRatio() / 100.0);

src/plugins/valgrind/memcheckerrorview.cpp

@@ -86,7 +86,7 @@ FilePath MemcheckErrorView::defaultSuppressionFile() const
 
 // slot, can (for now) be invoked either when the settings were modified *or* when the active
 // settings object has changed.
-void MemcheckErrorView::settingsChanged(ValgrindBaseSettings *settings)
+void MemcheckErrorView::settingsChanged(ValgrindSettings *settings)
 {
     QTC_ASSERT(settings, return);
     m_settings = settings;

src/plugins/valgrind/memcheckerrorview.h

@@ -12,7 +12,7 @@
 namespace Valgrind {
 namespace Internal {
 
-class ValgrindBaseSettings;
+class ValgrindSettings;
 
 class MemcheckErrorView : public Debugger::DetailedErrorView
 {
@@ -22,8 +22,8 @@ public:
 
     void setDefaultSuppressionFile(const Utils::FilePath &suppFile);
     Utils::FilePath defaultSuppressionFile() const;
-    ValgrindBaseSettings *settings() const { return m_settings; }
+    ValgrindSettings *settings() const { return m_settings; }
-    void settingsChanged(ValgrindBaseSettings *settings);
+    void settingsChanged(ValgrindSettings *settings);
 
 private:
     void suppressError();
@@ -31,7 +31,7 @@ private:
 
     QAction *m_suppressAction;
     Utils::FilePath m_defaultSuppFile;
-    ValgrindBaseSettings *m_settings = nullptr;
+    ValgrindSettings *m_settings = nullptr;
 };
 
 } // namespace Internal

src/plugins/valgrind/memchecktool.cpp

@@ -193,13 +193,13 @@ void MemcheckToolRunner::addToolArguments(CommandLine &cmd) const
 
     QString leakCheckValue;
     switch (m_settings.leakCheckOnFinish()) {
-    case ValgrindBaseSettings::LeakCheckOnFinishNo:
+    case ValgrindSettings::LeakCheckOnFinishNo:
         leakCheckValue = "no";
         break;
-    case ValgrindBaseSettings::LeakCheckOnFinishYes:
+    case ValgrindSettings::LeakCheckOnFinishYes:
         leakCheckValue = "full";
         break;
-    case ValgrindBaseSettings::LeakCheckOnFinishSummaryOnly:
+    case ValgrindSettings::LeakCheckOnFinishSummaryOnly:
     default:
         leakCheckValue = "summary";
         break;
@@ -420,7 +420,7 @@ private:
     void heobAction();
 
 private:
-    ValgrindBaseSettings *m_settings;
+    ValgrindSettings *m_settings;
     QMenu *m_filterMenu = nullptr;
 
     Valgrind::XmlProtocol::ErrorListModel m_errorModel;
@@ -941,11 +941,11 @@ void MemcheckToolPrivate::maybeActiveRunConfigurationChanged()
 {
     updateRunActions();
 
-    ValgrindBaseSettings *settings = nullptr;
+    ValgrindSettings *settings = nullptr;
     if (Project *project = ProjectManager::startupProject())
         if (Target *target = project->activeTarget())
             if (RunConfiguration *rc = target->activeRunConfiguration())
-                settings = rc->currentSettings<ValgrindBaseSettings>(ANALYZER_VALGRIND_SETTINGS);
+                settings = rc->currentSettings<ValgrindSettings>(ANALYZER_VALGRIND_SETTINGS);
 
     if (!settings) // fallback to global settings
         settings = &globalSettings();
@@ -962,7 +962,7 @@ void MemcheckToolPrivate::maybeActiveRunConfigurationChanged()
     // now make the new settings current, update and connect input widgets
     m_settings = settings;
     QTC_ASSERT(m_settings, return);
-    connect(m_settings, &ValgrindBaseSettings::destroyed,
+    connect(m_settings, &ValgrindSettings::destroyed,
             this, &MemcheckToolPrivate::settingsDestroyed);
 
     updateFromSettings();

src/plugins/valgrind/valgrindengine.cpp

@@ -104,16 +104,16 @@ QStringList ValgrindToolRunner::genericToolArguments() const
     QString smcCheckValue;
 
     switch (m_settings.selfModifyingCodeDetection()) {
-    case ValgrindBaseSettings::DetectSmcNo:
+    case ValgrindSettings::DetectSmcNo:
         smcCheckValue = "none";
         break;
-    case ValgrindBaseSettings::DetectSmcEverywhere:
+    case ValgrindSettings::DetectSmcEverywhere:
         smcCheckValue = "all";
         break;
-    case ValgrindBaseSettings::DetectSmcEverywhereButFile:
+    case ValgrindSettings::DetectSmcEverywhereButFile:
         smcCheckValue = "all-non-file";
         break;
-    case ValgrindBaseSettings::DetectSmcStackOnly:
+    case ValgrindSettings::DetectSmcStackOnly:
     default:
         smcCheckValue = "stack";
         break;

src/plugins/valgrind/valgrindengine.h

@@ -24,7 +24,7 @@ protected:
     virtual QString progressTitle() const = 0;
     virtual void addToolArguments(Utils::CommandLine &cmd) const = 0;
 
-    ValgrindProjectSettings m_settings;
+    ValgrindSettings m_settings{false};
     QFutureInterface<void> m_progress;
     ValgrindRunner m_runner;
 

src/plugins/valgrind/valgrindplugin.cpp

@@ -31,7 +31,7 @@ class ValgrindRunConfigurationAspect : public GlobalOrProjectAspect
 public:
     ValgrindRunConfigurationAspect(Target *)
     {
-        setProjectSettings(new ValgrindProjectSettings);
+        setProjectSettings(new ValgrindSettings(false));
         setGlobalSettings(&globalSettings());
         setId(ANALYZER_VALGRIND_SETTINGS);
         setDisplayName(Tr::tr("Valgrind Settings"));

src/plugins/valgrind/valgrindsettings.cpp

@@ -182,9 +182,12 @@ void SuppressionAspect::bufferToGui()
 //
 //////////////////////////////////////////////////////////////////
 
-ValgrindBaseSettings::ValgrindBaseSettings(bool global)
+ValgrindSettings::ValgrindSettings(bool global)
     : suppressions(this, global)
 {
+    setSettingsGroup("Analyzer");
+    setAutoApply(false);
+
     // Note that this is used twice, once for project settings in the .user files
     // and once for global settings in QtCreator.ini. This uses intentionally
     // the same key to facilitate copying using fromMap/toMap.
@@ -250,6 +253,12 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
     numCallers.setDefaultValue(25);
     numCallers.setLabelText(Tr::tr("Backtrace frame count:"));
 
+    lastSuppressionDirectory.setSettingsKey(base + "LastSuppressionDirectory");
+    lastSuppressionDirectory.setVisible(global);
+
+    lastSuppressionHistory.setSettingsKey(base + "LastSuppressionHistory");
+    lastSuppressionHistory.setVisible(global);
+
     // Callgrind
 
     kcachegrindExecutable.setSettingsKey(base + "KCachegrindExecutable");
@@ -320,6 +329,24 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
         defaultErrorKinds << i;
     visibleErrorKinds.setDefaultValue(defaultErrorKinds);
 
+    detectCycles.setSettingsKey(base + "Callgrind.CycleDetection");
+    detectCycles.setDefaultValue(true);
+    detectCycles.setLabelText("O"); // FIXME: Create a real icon
+    detectCycles.setToolTip(Tr::tr("Enable cycle detection to properly handle recursive "
+        "or circular function calls."));
+    detectCycles.setVisible(global);
+
+    costFormat.setSettingsKey(base + "Callgrind.CostFormat");
+    costFormat.setDefaultValue(CostDelegate::FormatRelative);
+    costFormat.setDisplayStyle(SelectionAspect::DisplayStyle::ComboBox);
+    costFormat.setVisible(global);
+
+    shortenTemplates.setSettingsKey(base + "Callgrind.ShortenTemplates");
+    shortenTemplates.setDefaultValue(true);
+    shortenTemplates.setLabelText("<>"); // FIXME: Create a real icon
+    shortenTemplates.setToolTip(Tr::tr("Remove template parameter lists when displaying function names."));
+    shortenTemplates.setVisible(global);
+
     setLayouter([this] {
         using namespace Layouting;
 
@@ -367,73 +394,28 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
         };
         // clang-format on
     });
+
+    if (global) {
+        readSettings();
+    } else {
+        // FIXME: Is this needed?
+        connect(this, &AspectContainer::fromMapFinished, [this] {
+            // FIXME: Update project page e.g. on "Restore Global", aspects
+            // there are 'autoapply', and Aspect::cancel() is normally part of
+            // the 'manual apply' machinery.
+            setAutoApply(false);
+            cancel();
+            setAutoApply(true);
+        });
+    }
 }
 
-
+ValgrindSettings &globalSettings()
-//////////////////////////////////////////////////////////////////
-//
-// ValgrindGlobalSettings
-//
-//////////////////////////////////////////////////////////////////
-
-ValgrindGlobalSettings &globalSettings()
 {
-    static ValgrindGlobalSettings theSettings;
+    static ValgrindSettings theSettings{true};
     return theSettings;
 }
 
-ValgrindGlobalSettings::ValgrindGlobalSettings()
-    : ValgrindBaseSettings(true)
-{
-    const QString base = "Analyzer.Valgrind";
-
-    lastSuppressionDirectory.setSettingsKey(base + "LastSuppressionDirectory");
-
-    lastSuppressionHistory.setSettingsKey(base + "LastSuppressionHistory");
-
-    detectCycles.setSettingsKey(base + "Callgrind.CycleDetection");
-    detectCycles.setDefaultValue(true);
-    detectCycles.setLabelText("O"); // FIXME: Create a real icon
-    detectCycles.setToolTip(Tr::tr("Enable cycle detection to properly handle recursive "
-        "or circular function calls."));
-
-    costFormat.setSettingsKey(base + "Callgrind.CostFormat");
-    costFormat.setDefaultValue(CostDelegate::FormatRelative);
-    costFormat.setDisplayStyle(SelectionAspect::DisplayStyle::ComboBox);
-
-    shortenTemplates.setSettingsKey(base + "Callgrind.ShortenTemplates");
-    shortenTemplates.setDefaultValue(true);
-    shortenTemplates.setLabelText("<>"); // FIXME: Create a real icon
-    shortenTemplates.setToolTip(Tr::tr("Remove template parameter lists when displaying function names."));
-
-    setSettingsGroup("Analyzer");
-    readSettings();
-    setAutoApply(false);
-}
-
-//
-// Memcheck
-//
-
-//////////////////////////////////////////////////////////////////
-//
-// ValgrindProjectSettings
-//
-//////////////////////////////////////////////////////////////////
-
-ValgrindProjectSettings::ValgrindProjectSettings()
-    : ValgrindBaseSettings(false)
-{
-    connect(this, &AspectContainer::fromMapFinished, [this] {
-        // FIXME: Update project page e.g. on "Restore Global", aspects
-        // there are 'autoapply', and Aspect::cancel() is normally part of
-        // the 'manual apply' machinery.
-        setAutoApply(false);
-        cancel();
-        setAutoApply(true);
-    });
-}
-
 //
 // ValgrindOptionsPage
 //
@@ -454,5 +436,4 @@ public:
 
 const ValgrindOptionsPage settingsPage;
 
-
 } // Valgrind::Internal

src/plugins/valgrind/valgrindsettings.h

@@ -31,19 +31,20 @@ private:
     void bufferToGui() override;
     bool guiToBuffer() override;
 
-    friend class ValgrindBaseSettings;
+    friend class ValgrindSettings;
     SuppressionAspectPrivate *d = nullptr;
 };
 
 /**
  * Valgrind settings shared for global and per-project.
  */
-class ValgrindBaseSettings : public Utils::AspectContainer
+class ValgrindSettings : public Utils::AspectContainer
 {
     Q_OBJECT
 
 public:
-    explicit ValgrindBaseSettings(bool global);
+    // These exists once globally, and once per project
+    explicit ValgrindSettings(bool global);
 
     enum SelfModifyingCodeDetection {
         DetectSmcNo,
@@ -58,20 +59,14 @@ public:
         LeakCheckOnFinishYes
     };
 
-/**
+    // Generic valgrind settings
- * Base valgrind settings
- */
-public:
     Utils::FilePathAspect valgrindExecutable{this};
     Utils::StringAspect valgrindArguments{this};
     Utils::SelectionAspect selfModifyingCodeDetection{this};
 
     SuppressionAspect suppressions;
 
-/**
+    // Memcheck
- * Base memcheck settings
- */
-public:
     Utils::StringAspect memcheckArguments{this};
     Utils::IntegerAspect numCallers{this};
     Utils::SelectionAspect leakCheckOnFinish{this};
@@ -80,12 +75,12 @@ public:
     Utils::BoolAspect filterExternalIssues{this};
     Utils::IntegersAspect visibleErrorKinds{this};
 
+    Utils::FilePathAspect lastSuppressionDirectory{this}; // Global only
+    Utils::StringAspect lastSuppressionHistory{this}; // Global only
+
     void setVisibleErrorKinds(const QList<int> &);
 
-/**
+    // Callgrind
- * Base callgrind settings
- */
-public:
     Utils::StringAspect callgrindArguments{this};
     Utils::FilePathAspect kcachegrindExecutable{this};
 
@@ -96,46 +91,12 @@ public:
     Utils::BoolAspect enableEventToolTips{this};
     Utils::DoubleAspect minimumInclusiveCostRatio{this};
     Utils::DoubleAspect visualizationMinimumInclusiveCostRatio{this};
+
+    Utils::SelectionAspect costFormat{this}; // Global only
+    Utils::BoolAspect detectCycles{this}; // Global only
+    Utils::BoolAspect shortenTemplates{this}; // Global only
 };
 
-/**
+ValgrindSettings &globalSettings();
- * Global valgrind settings
- */
-class ValgrindGlobalSettings : public ValgrindBaseSettings
-{
-    Q_OBJECT
-
-public:
-    ValgrindGlobalSettings();
-
-    /**
-     * Global memcheck settings
-     */
-
-    Utils::FilePathAspect lastSuppressionDirectory{this};
-    Utils::StringAspect lastSuppressionHistory{this};
-
-
-    /**
-     * Global callgrind settings
-     */
-    Utils::SelectionAspect costFormat{this};
-    Utils::BoolAspect detectCycles{this};
-    Utils::BoolAspect shortenTemplates{this};
-};
-
-ValgrindGlobalSettings &globalSettings();
-
-
-/**
- * Per-project valgrind settings.
- */
-class ValgrindProjectSettings : public ValgrindBaseSettings
-{
-    Q_OBJECT
-
-public:
-    ValgrindProjectSettings();
-};
 
 } // Valgrind::Internal