ub: Remove illegal FAM from ParamSet (#299)

Flexible Array Members are not legal in C++ and their use here has
already caused at least one "miscompilation" (see #230). This change
replaces it with regular arrays in the children, with a pointer passed
up to the parent. This costs about 1k extra bytes, but that seems likely
to be code that was being compiled out when it shouldn't be.

Baseline (5edc967): 1540416 build/Release/delugeOLED.bin
With this change  : 1541344 build/Release/delugeOLED.bin

src/deluge/model/clip/instrument_clip.cpp

@@ -4272,7 +4272,7 @@ void InstrumentClip::recordNoteOn(ModelStackWithNoteRow* modelStack, int32_t vel
 	    modelStack->addOtherTwoThingsAutomaticallyGivenNoteRow()->addParamCollection(mpeParams, mpeParamsSummary);
 
 	for (int32_t m = 0; m < kNumExpressionDimensions; m++) {
-		AutoParam* param = (m == 0 ? &mpeParams->params[m] : &mpeParams->fakeParams[m - 1]);
+		AutoParam* param = &mpeParams->params[m];
 		ModelStackWithAutoParam* modelStackWithAutoParam = modelStackWithParamCollection->addAutoParam(m, param);
 
 		Action* action = actionLogger.getNewAction(ACTION_RECORD, true);

src/deluge/modulation/params/param_set.cpp

@@ -34,8 +34,8 @@
 #include "storage/storage_manager.h"
 #include "util/functions.h"
 
-ParamSet::ParamSet(int32_t newObjectSize, ParamCollectionSummary* summary) : ParamCollection(newObjectSize, summary) {
-	topUintToRepParams = 1;
+ParamSet::ParamSet(int32_t newObjectSize, ParamCollectionSummary* summary)
+    : ParamCollection(newObjectSize, summary), numParams_(0), params(nullptr), topUintToRepParams(1) {
 }
 
 void ParamSet::beenCloned(bool copyAutomation, int32_t reverseDirectionWithLength) {
@@ -360,6 +360,8 @@ void ParamSet::notifyPingpongOccurred(ModelStackWithParamCollection* modelStack)
 // UnpatchedParamSet --------------------------------------------------------------------------------------------
 
 UnpatchedParamSet::UnpatchedParamSet(ParamCollectionSummary* summary) : ParamSet(sizeof(UnpatchedParamSet), summary) {
+	params = params_.data();
+	numParams_ = static_cast<int32_t>(params_.size());
 }
 
 bool UnpatchedParamSet::shouldParamIndicateMiddleValue(ModelStackWithParamId const* modelStack) {
@@ -385,6 +387,8 @@ bool UnpatchedParamSet::doesParamIdAllowAutomation(ModelStackWithParamId const*
 
 PatchedParamSet::PatchedParamSet(ParamCollectionSummary* summary) : ParamSet(sizeof(PatchedParamSet), summary) {
 	topUintToRepParams = (kNumParams - 1) >> 5;
+	params = params_.data();
+	numParams_ = static_cast<int32_t>(params_.size());
 }
 
 void PatchedParamSet::notifyParamModifiedInSomeWay(ModelStackWithAutoParam const* modelStack, int32_t oldValue,
@@ -482,6 +486,8 @@ bool PatchedParamSet::shouldParamIndicateMiddleValue(ModelStackWithParamId const
 
 ExpressionParamSet::ExpressionParamSet(ParamCollectionSummary* summary, bool forDrum)
     : ParamSet(sizeof(ExpressionParamSet), summary) {
+	params = params_.data();
+	numParams_ = static_cast<int32_t>(params_.size());
 	bendRanges[BEND_RANGE_MAIN] = FlashStorage::defaultBendRange[BEND_RANGE_MAIN];
 
 	bendRanges[BEND_RANGE_FINGER_LEVEL] =

src/deluge/modulation/params/param_set.h

@@ -20,6 +20,7 @@
 #include "definitions_cxx.hpp"
 #include "modulation/automation/auto_param.h"
 #include "modulation/params/param_collection.h"
+#include <array>
 
 class Sound;
 class ParamManagerForTimeline;
@@ -34,7 +35,12 @@ class ModelStackWithParamCollection;
 // This differs from other inheriting classes of ParamCollection.
 
 class ParamSet : public ParamCollection {
+protected:
+	/// Number of parameters in the params array
+	int32_t numParams_;
+
 public:
+	AutoParam* params;
 	ParamSet(int32_t newObjectSize, ParamCollectionSummary* summary);
 
 	inline int32_t getValue(int32_t p) { return params[p].getCurrentValue(); }
@@ -76,17 +82,14 @@ class ParamSet : public ParamCollection {
 	// For undoing / redoing
 	void remotelySwapParamState(AutoParamState* state, ModelStackWithParamId* modelStack) final;
 
-	virtual int32_t getNumParams() = 0;
+	int32_t getNumParams() { return numParams_; }
 
 	ModelStackWithAutoParam* getAutoParamFromId(ModelStackWithParamId* modelStack, bool allowCreation = true) final;
 	void notifyParamModifiedInSomeWay(ModelStackWithAutoParam const* modelStack, int32_t oldValue,
 	                                  bool automationChanged, bool automatedBefore, bool automatedNow);
 
 	uint8_t topUintToRepParams;
 
-	AutoParam params
-	    [1]; // Total hack - we declare this last, then the subclasses "extend" it by having extra unused space after it
-
 private:
 	void backUpParamToAction(int32_t p, Action* action, ModelStackWithParamCollection* modelStack);
 	void checkWhetherParamHasInterpolationNow(ModelStackWithParamCollection const* modelStack, int32_t p);
@@ -95,30 +98,29 @@ class ParamSet : public ParamCollection {
 class UnpatchedParamSet final : public ParamSet {
 public:
 	UnpatchedParamSet(ParamCollectionSummary* summary);
-	int32_t getNumParams() { return kMaxNumUnpatchedParams; }
 	bool shouldParamIndicateMiddleValue(ModelStackWithParamId const* modelStack);
 	bool doesParamIdAllowAutomation(ModelStackWithParamId const* modelStack);
 
-	AutoParam fakeParams[kMaxNumUnpatchedParams - 1];
+private:
+	std::array<AutoParam, kMaxNumUnpatchedParams> params_;
 };
 
 class PatchedParamSet final : public ParamSet {
 public:
 	PatchedParamSet(ParamCollectionSummary* summary);
-	int32_t getNumParams() { return kNumParams; }
 	void notifyParamModifiedInSomeWay(ModelStackWithAutoParam const* modelStack, int32_t oldValue,
 	                                  bool automationChanged, bool automatedBefore, bool automatedNow);
 	int32_t paramValueToKnobPos(int32_t paramValue, ModelStackWithAutoParam* modelStack);
 	int32_t knobPosToParamValue(int32_t knobPos, ModelStackWithAutoParam* modelStack);
 	bool shouldParamIndicateMiddleValue(ModelStackWithParamId const* modelStack);
 
-	AutoParam fakeParams[kNumParams - 1];
+private:
+	std::array<AutoParam, kNumParams> params_;
 };
 
 class ExpressionParamSet final : public ParamSet {
 public:
 	ExpressionParamSet(ParamCollectionSummary* summary, bool forDrum = false);
-	int32_t getNumParams() { return kNumExpressionDimensions; }
 	void notifyParamModifiedInSomeWay(ModelStackWithAutoParam const* modelStack, int32_t oldValue,
 	                                  bool automationChanged, bool automatedBefore, bool automatedNow);
 	bool mayParamInterpolate(int32_t paramId) { return false; }
@@ -132,10 +134,11 @@ class ExpressionParamSet final : public ParamSet {
 	void cancelAllOverriding();
 	void deleteAllAutomation(Action* action, ModelStackWithParamCollection* modelStack);
 
-	AutoParam fakeParams[kNumExpressionDimensions - 1];
-
 	// bendRanges being stored here in ExpressionParamSet still seems like the best option. I was thinking storing them in the ParamManager would make more sense, except for one thing
 	// - persistence when preset/Instrument changes. ExpressionParamSets do this unique thing where they normally aren't "stolen" or "backed up" - unless the last Clip is being deleted,
 	// in which case they do move to the backedUpParamManager. This is exactly the persistence we want for bendRanges too.
 	uint8_t bendRanges[2];
+
+private:
+	std::array<AutoParam, kNumExpressionDimensions> params_;
 };