Context
The proposal below aims at defining what level of type deduction we accept in out code-base. It is intended to be a starting point for discussion (not a finalized, ready to be implemented set of rules).
It tries to balance the pros and cons of type deduction:
Pros:
- Reduces code verbosity.
Cons:
- Reduces code clarity and expressiveness (self documentation).
- Tends to create more verbose and obscure compiler errors.
Proposal
Some General Rules
Do not consider contextual help from IDE as a good reason to remove explicitness from the source code. While they can be very handy, not all IDEs provide the same level of contextual information, and code must remain understandable when such help is not available (e.g. when reviewing PRs online).
auto
In general, auto should not be used unless the type is clearly and unambiguously expressed somewhere else in the same expression, e.g. by using a casting expression.
/* DO:
* The type of `var` is clear from the casting of the assigned data. */
auto *var = static_cast<blender::Map<std::string, ID *> *>(user_data);
const bool result = my_callback(my_id);
/* DON'T:
* There is no immediate way to know the type of `result`. */
auto result = my_callback(my_id);
Note that auto should also be used with unnamed types, e.g. to store a lambda (closure type) in a local variable, when there is no other choice to store a local callback (i.e. when a blender::FunctionRef or similar cannot be used).
Another valid usage of auto is with iterators: Usually, the exact type of the iterator does not matter, as long as they are just used through the common iterator patterns.
blender::Array<int, 42> my_array;
/* ... */
/* DO: */
for (auto my_it = my_array.rbegin(); my_it != my_array.rend(); my_it++) {
int my_val = *my_it;
/* ... */
}
/* DON'T: */
for (auto my_it = my_array.rbegin(); my_it != my_array.rend(); my_it++) {
auto my_val = *my_it;
/* ... */
}
/* DO: */
for (int my_val : my_array) {
/* ... */
}
/* DON'T: */
for (auto my_val : my_array) {
/* ... */
}
Lambdas
Lambdas should always explicitly define their return value. This matches expectations with regular functions.
In general, lambdas should not do generic capture of all local variables, but rather explicitly define which variables are captured, and whether it is by reference or copy. Otherwise, one has to read the whole lambda code to find out which variables are used, which ones may be modified, etc.
Non-trivial lambdas should be defined outside of the function call making use of them. A lambda is non-trivial if it does more than a single operation, like wrapping a call to another function.
There are some valid exceptions to these rules though, when the lambda contains most of the logic of the function, which then acts as a mere wrapper around it. E.g. when using certain iteration patterns:
bool AttributeAccessor::contains(const StringRef attribute_id) const
{
bool found = false;
this->foreach_attribute([&](const AttributeIter &iter) {
if (attribute_id == iter.name) {
found = true;
iter.stop();
}
});
return found;
}
Templates
Do not rely on type deduction when using templated functions or types, outside of trivial cases. In particular:
- Templated types (classes etc.) should always have explicit template parameters, unless they have a default value defined.
- Templated function calls should always have explicit template parameters when they affect their return types. This is consistent with the fact that there is no type inference on returned value in C++.
Example of trivial type deduction:
/* Even though this would seem fairly trivial, the created data type has to be explicitly defined
* (no type inference on returned value). */
ID *id = MEM_new<ID>();
// ...
/* There is no need to call explicitly `MEM_delete<ID>(id)` here. */
MEM_delete(id);
Example of non-trivial type deduction, threading::parallel_reduce<>:
int val = 0;
// ...
/* DON'T:
* Nothing in the call below would clearly indicate the type of computed value.
* Further more, a mistake in one of the parameter type or callbacks definition can lead to many lines
* of fairly obscure errors. */
// val = threading::parallel_reduce(
// my_vec,
// 1024,
// 0,
// [](const IndexRange range, int value) { return value + int(range[0]); },
// [](const int &a, const int &b) { return a + b; });
/* DO:
* Here it is obvious what the _intended_ produced value type is.
* It will also often generate much more concise and readable error messages in case of mistakes. */
val = threading::parallel_reduce<int>(
my_vec,
1024,
0,
[](const IndexRange range, int value) -> int { return value + int(range[0]); },
[](const int &a, const int &b) -> int { return a + b; });
