I disagree for something like an API where you can’t control whether module developers write robust code. Ideally I think an API should prevent a poorly coded module from crashing Rack whenever possible. Putting the test in the API is a step in that direction.
1 Like
Valid point, but a lot of apps like VCV don’t enable assertions in release builds, although I guess VCV does. The problem is that a failed assertion causes a “crash”. Although I guess you are assuming/hoping that asserts would be off in the realease build, so you wouldn’t get the CPU hit?
Also, I’m all for being efficient, but this is an API that does not need to be called often. I can’t imagine a time where the runtime overhead would be significant.
Ah ok, understood, thanks for the explanation.
So yeah, I guess supressing the warning is my route to happiness
2 Likes
Yes.
Maybe not in this case, but it’s a good habit to get into with real time stuff.
Assuming they test with a debug build, this approach will help them learn to by showing them what they’ve done wrong instead of hiding it from them.
Do it with a pragma just for that bit of code, disabling a warning globally is not wise.
I’ve been doing realtime programming, including embedded for a long time, and have a different opinion. Both I and Mr. Knuth would disagree with your “good habit” statement. But hopefully we can call this a minor disagreement 
And, fwiw, my plugins were known to be pretty darned efficient with CPU cycles.
I must be dense. The original warning says you are using an out of bounds array index.
Eh? I never said they weren’t.
no, just using that to buttress my opinion. as in “I’m well known for writing efficient VCV modules and I don’t agree that all VCV APIs need to be as fast as possible”. Didn’t mean to imply otherwise.
1 Like
My claim is that it WILL execute under the right semi-contrived scenario. IT ABSOLUTELY CAN ACTUALLY INDEX OUT OF BOUNDS AND CRASH.
I might be wrong about this, of course. But that’s my claim.
Okay, I see the point that the out of bound array index will never execute.
Yeah, I understand your point now.
1 Like
Indeed, I have done this elsewhere with some other questionable coding practices 
Forgive me as a C++ novice though, does the pragma need to be actually in the API source, it doesn’t seem to work if I use it around the method call, I assume I am doing this incorrectly?
void process(const ProcessArgs &args) override
{
...
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Warray-bounds"
outputs[POLY_CHANNELS_OUTPUT].setChannels(16);
#pragma GCC diagnostic pop
...
}
I think you are right, it needs to be in the source. In the past I’ve disabled warnings before including the rack headers, then restoring after. That was for the Microsoft compiler, though. (MS compiler has different warnings than gcc - not surprising).
put it into incude/engine/Port.hpp and it works:
void setChannels(int channels) {
// If disconnected, keep the number of channels at 0.
if (this->channels == 0) {
return;
}
// Set higher channel voltages to 0
for (int c = channels; c < this->channels; c++) {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Warray-bounds"
voltages[c] = 0.f;
#pragma GCC diagnostic pop
}
// Don't allow caller to set port as disconnected
if (channels == 0) {
channels = 1;
}
this->channels = channels;
}
1 Like
What happens if you use
outputs[POLY_CHANNELS_OUTPUT].setChannels(PORT_MAX_CHANNELS);
instead of
outputs[POLY_CHANNELS_OUTPUT].setChannels(16);
MIDI is a serial protocol, one message at a time. So, speculating wildly, that, combined with the speeds attainable by serial communication in consumer hardware back when MIDI was first being developed and rolled out, was probably a factor in limiting the specification to 16 channels. Maybe. I’m probably wrong, though. Would be interesting to read historical notes about the discussions around that.
1 Like
I was around then, and was the holder of midi device ID #3, and an active representative to MIDI Manufacturers Association for many years. I wasn’t privy to discussions around the original protocol, as it just came into the world from Roland and Sequential. But, I’ll stand by my original speculation, above.
Btw, device IDs were given our sequentially, The ones starting at 1 were for North America and starting at hex 40 were for Japan. So 1=Sequential 2=Big Briar (Bob Moog’s company) 3=Voyetra, $40 = Roland.
2 Likes