Address review comments

include/Flags.h

@@ -52,11 +52,6 @@ class Flags
 	constexpr auto testAny(Flags flags) const -> bool { return (*this & flags) != Flags{}; }
 	constexpr auto testFlag(EnumType flag) const -> bool { return static_cast<bool>(*this & flag); }
 
-	constexpr void setFlag(EnumType flag, bool value = true)
-	{
-		m_value = value ? (m_value | flag) : (m_value & ~flag);
-	}
-
 	constexpr auto operator~() const -> Flags { return Flags{~m_value}; }
 	friend constexpr auto operator&(Flags l, Flags r) -> Flags { return Flags{l.m_value & r.m_value}; }
 	friend constexpr auto operator|(Flags l, Flags r) -> Flags { return Flags{l.m_value | r.m_value}; }

include/PluginPinConnector.h

@@ -30,6 +30,7 @@
 #include <cassert>
 #include <cstdint>
 #include <cstring>
+#include <type_traits>
 #include <vector>
 
 #include "AudioData.h"
@@ -56,6 +57,50 @@ class PluginPinConnectorView;
 } // namespace gui
 
 
+/**
+ * A non-owning span of CoreAudioData.
+ *
+ * Access like this:
+ *   bus[channel pair index][frame index]
+ *
+ * where
+ *   0 <= channel pair index < channelPairs
+ *   0 <= frame index < frames
+ *
+ * TODO C++23: Use std::mdspan
+ */
+template<typename T>
+struct AudioBus
+{
+	static_assert(std::is_same_v<std::remove_const_t<T>, SampleFrame>);
+
+	AudioBus() = default;
+	AudioBus(const AudioBus&) = default;
+
+	AudioBus(T* const* bus, ch_cnt_t channelPairs, f_cnt_t frames)
+		: bus{bus}
+		, channelPairs{channelPairs}
+		, frames{frames}
+	{
+	}
+
+	template<typename U = T, std::enable_if_t<std::is_const_v<U>, bool> = true>
+	AudioBus(const AudioBus<std::remove_const_t<U>>& other)
+		: bus{other.bus}
+		, channelPairs{other.channelPairs}
+		, frames{other.frames}
+	{
+	}
+
+	T* const* bus = nullptr; //!< [channel pair index][frame index]
+	ch_cnt_t channelPairs = 0;
+	f_cnt_t frames = 0;
+};
+
+using CoreAudioBus = AudioBus<const SampleFrame>;
+using CoreAudioBusMut = AudioBus<SampleFrame>;
+
+
 //! Configuration for audio channel routing in/out of plugin
 class LMMS_EXPORT PluginPinConnector
 	: public Model
@@ -241,7 +286,7 @@ public slots:
 	 * This needs to be known because the default connections (and view?) for instruments with sidechain
 	 * inputs is different from effects, even though they may both have the same channel counts.
 	 */
-	bool m_isInstrument = false;
+	const bool m_isInstrument = false;
 };
 
 
@@ -489,7 +534,7 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
 		switch (routedChannels)
 		{
 			case 0b00:
-				// Both track channels are bypassed, so nothing needs to be written to output
+				// Both track channels are bypassed, so nothing is allowed to be written to output
 				break;
 			case 0b01:
 				routeNx2(outPtr, outChannel, std::integral_constant<std::uint8_t, 0b01>{});

include/RemotePlugin.h

@@ -164,7 +164,7 @@ public slots:
 	auto inputBuffer() const -> Span<float> { return m_inputBuffer; }
 	auto outputBuffer() const -> Span<float> { return m_outputBuffer; }
 
-	PluginPinConnector* m_pinConnector = nullptr;
+	PluginPinConnector* const m_pinConnector = nullptr;
 
 private:
 	QProcess m_process;

include/SampleFrame.h

@@ -32,7 +32,6 @@
 #include <algorithm>
 #include <array>
 #include <cmath>
-#include <type_traits>
 
 
 namespace lmms
@@ -229,57 +228,13 @@ inline void copyFromSampleFrames(InterleavedSampleType<float>* target, const Sam
 }
 
 
-//! A non-owning 2-channel buffer
+//! A non-owning `SampleFrame` buffer (interleaved, 2-channel)
 using CoreAudioData = Span<const SampleFrame>;
 
 //! Mutable CoreAudioData
 using CoreAudioDataMut = Span<SampleFrame>;
 
 
-/**
- * A non-owning span of CoreAudioData.
- *
- * Access like this:
- *   bus[channel pair index][frame index]
- *
- * where
- *   0 <= channel pair index < channelPairs
- *   0 <= frame index < frames
- *
- * TODO C++23: Use std::mdspan
- */
-template<typename T>
-struct AudioBus
-{
-	static_assert(std::is_same_v<std::remove_const_t<T>, SampleFrame>);
-
-	AudioBus() = default;
-	AudioBus(const AudioBus&) = default;
-
-	AudioBus(T* const* bus, ch_cnt_t channelPairs, f_cnt_t frames)
-		: bus{bus}
-		, channelPairs{channelPairs}
-		, frames{frames}
-	{
-	}
-
-	template<typename U = T, std::enable_if_t<std::is_const_v<U>, bool> = true>
-	AudioBus(const AudioBus<std::remove_const_t<U>>& other)
-		: bus{other.bus}
-		, channelPairs{other.channelPairs}
-		, frames{other.frames}
-	{
-	}
-
-	T* const* bus = nullptr; //!< [channel pair index][frame index]
-	ch_cnt_t channelPairs = 0;
-	f_cnt_t frames = 0;
-};
-
-using CoreAudioBus = AudioBus<const SampleFrame>;
-using CoreAudioBusMut = AudioBus<SampleFrame>;
-
-
 } // namespace lmms
 
 #endif // LMMS_SAMPLEFRAME_H

include/lmms_basics.h

@@ -79,7 +79,7 @@ constexpr const char* UI_CTRL_KEY =
 /**
  * Simple minimally functional stand-in for C++20's std::span
  *
- * TODO C++20: Use std::span instead
+ * TODO C++20: Use std::span instead once we have GCC 10 or newer
  */
 template<typename T, std::size_t extents = static_cast<std::size_t>(-1)>
 class Span

src/core/PluginPinConnector.cpp

@@ -191,6 +191,8 @@ void PluginPinConnector::updateAllRoutedChannels()
 
 auto PluginPinConnector::instantiateView(QWidget* parent) const -> gui::PluginPinConnectorView*
 {
+	// This method does not modify the pin connector, but it needs the view to store
+	// a mutable pointer to the pin connector, hence the const_cast.
 	return new gui::PluginPinConnectorView{const_cast<PluginPinConnector*>(this), parent};
 }