missing `const` throughout headers; causes difficult usage, integration, and slowdown

[diablodale]

Headers throughout min-api are missing const. The const that is totally absent in legacy SDKs, mostly absent in max-api, is also mostly absent in min-api. To use min, I need it to be better than the legacy SDKs. This is major SDK work to resolve a systemic problem; needs to be done by Cycling74, not customers.

c74_min_operator_matrix.h has so many missing const that I can't list them all here. Here are two of dozens in that file:

https://github.com/Cycling74/min-api/blob/32449860b7929bd822cf543c551a9f5fb9fad6e5/include/c74_min_operator_matrix.h#L27-L29

https://github.com/Cycling74/min-api/blob/32449860b7929bd822cf543c551a9f5fb9fad6e5/include/c74_min_operator_matrix.h#L217-L218

Not isolated to that one file. Second example c74_min_buffer.h

https://github.com/Cycling74/min-api/blob/32449860b7929bd822cf543c551a9f5fb9fad6e5/include/c74_min_buffer.h#L49-L54

I request that min-api use const in all situations possible. As a rule, the default for all function parameters should be const. Only if a reference parameter is to be mutated should it be non-const. This leads then to declaring max_jit_mop_new(const max::t_symbol* const s, const long argc, const max::t_atom* const argv).

I do see some examples of member functions being declared const when they do not change state of their class. This const usage should be verified across all min-api member functions to ensure this is consistently on all member functions that don't mutate their class.

[j3kz]

I agree and we may consider max_jit_mop_new(max::t_symbol const * const s, long const argc, max::t_atom const * const argv) syntax which is more readable in the code.

[tap]

There are places, such as the example highlighted by @j3kz, where const usage can certainly be improved.

I do not understand the other two examples from @diablodale. These parameters are being passed by copy. The relevant GSL section can be found here: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f16-for-in-parameters-pass-cheaply-copied-types-by-value-and-others-by-reference-to-const

[diablodale]

@j3kz haha, yes max::t_symbol const * const s or const max::t_symbol * const s are both fine with me. Something in my past, has me put const as left as possible. In practice, I have no preference and can follow the style of the team.

@tap great reference. Yet, I think you misread F.16. Cheap things (int, float, POD-stuff) pass by copy Complex things (large structs, STL containers, etc.) pass by const reference. This is focused on performance...the cpu/mem overhead of sending data to a function via parameters. But F.16 does not provide guidance for the issue at hand. The issue of const-correctness.

I'm saying to const the pass by copy in every situation you don't mutate it in the function https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con-constants-and-immutability. It better documents code, helps coders from making mistakes, provides opportunities for compiler optimizations, etc. This approach is so important in 21st century coding...that new languages like Rust go so far that variables are by default const and you have to go through hoops to mutate them.

If function code is only reading a parameter...make it const. This protects the coder of that function from inadvertent mistakes. And allows more flexible usage of the function itself. Remember the legacy SDK and its header ext_proto.h with the following ugh 😝 prototype

void *outlet_anything(void *o, t_symbol *s, short ac, t_atom *av);

I can't write the solid code I want to. That legacy prototype forces me to send mutable non-const values for everything. But as we all know, t_symbols are always logically const (as editing inside that "blackbox" is a no-no), and my av should be const (because outlet_anything shouldn't be altering it), and pointers to them all may/may not be const. But no...that prototype forces all params to be non-const pointers to non-const values.

That prototype, atom_setsym() and others are so troublesome to use in well-structured code, that I in my custom max headers alter them to I can write better code. My prototypes are

void *outlet_anything(void * const o, const t_symbol * const s, const short ac, const t_atom * const av);
t_max_err atom_setsym(t_atom * const a, const t_symbol * const s);

With those prototypes, I can send const or non-const parameters. I can now in my code (before it ever gets to that max function) use const variables to hold const things. Yay! And the prototype tells a naive max programmer that this function is not going to change anything.

[Edit] and a comment about that cpp guideline link above. There is a section saying const on parameters can be pedantic and lead to false-positive warnings by a compiler. So the guideline says that compilers should flag (warn) when they discover this. But the const-correctness is still a valid guideline...its just that compilers won't warn about it.

[diablodale]

Want to add clarification on that const-correctness subtlety...

The rightmost const in this prototype foobar(int * const a); is pedantic. For the consumer of the function, it doesn't matter so much. For the coder of the function itself, it is valuable as it restricts the code in that function from accidental mistakes. For example:

void foobar(int * a) {
++a;
}

The above incremented the pointer. But the coder meant to increment the int to which the pointer pointed. Now contrast this to the following:

void foobar(int * const a) {
++a;
}

In that above version, the compiler will protect the coder from themselves. It won't compile. And so they can change their code to be the correctish ++(*a). The exception in my link writes to the frequency and reasoning. I lean towards protecting myself when I write the inside of that function. In my experience, all humans make mistakes.

Separate from the right-most const, the following is definitely desired if the dereferenced value of the pointer should be const and the function should not be able to change it

void jimjam(const int * a) {
  // can't change *a
}

[tap]

Thanks @diablodale for the clarification, the argument, and the link to the appropriate section of the ISOCPP guidelines.

While there is little we can do with the classic Max SDK, I agree with you that we can and should improve this with the Min API.