-
Notifications
You must be signed in to change notification settings - Fork 6
Description
Abstract
Replace extensive use of unchecked (and uncheckable) raw pointers with std::span, which means requiring a C++20 capable compiler.
Motivation
A simple example is replacing:
void getMassFractions(double* const y) const;with:
void getMassFractions(std::span<double> y) const;This provides a number of improvements for calling these methods without being tied to a specific container, providing flexibility for end users without requiring the use of raw pointers. For example, all of the following work:
using std::span;
size_t K = gas->nSpecies();
// std::vector
vector<double> yy(K);
gas->getMassFractions(yy);
// Eigen arrays (starting in the just-released Eigen 5.0.0)
Eigen::ArrayXd ee(K);
gas->getMassFractions(ee);
// For older Eigen versions:
gas->getMassFractions(span<double>(ee.data(), ee.data() + ee.size()));
// Raw pointers
double* ss = new double[K];
gas->getMassFractions(span<double>(ss, K));
delete[] ss;At least for some compilers and standard libraries (GCC / libstdc++) using std::span provides bounds checking when the NDEBUG macro is not defined. Others (Clang / libc++) seem not to check under any circumstances. Substituting a different implementation of span (for example https://github.com/tcbrindle/span) could be useful in such cases. This also gives us the option of introducing our own bounds checks at key entry points.
The last option shown here of constructing the span from a raw pointer and length would couple cleanly with the generated CLib, where the length of the input array is consistently provided with all functions using array inputs and outputs.
Implementation Considerations
There are also benefits within the Cantera implementation, since span works with other C++ features such as assignment operators and range-based for loops. For example:
// legacy
void Phase::getMassFractions(double* const y) const {
copy(m_y.begin(), m_y.end(), y);
}
// replacement
void Phase::getMassFractions(span<double> y) const {
y = m_y;
}More benefits to working with objects of this type rather than bare pointers are on the horizon in C++23, with things like std::views::enumerate and std::views::zip.
span objects also provide methods for accessing sub-ranges. For example, the implementation of saveState might change in something like the following way:
// legacy
void Phase::saveState(size_t lenstate, double* state) const {
if (lenstate < stateSize()) {
throw ArraySizeError("Phase::saveState", lenstate, stateSize());
}
savePartialState(lenstate, state);
auto native = nativeState();
if (native.count("X")) {
getMoleFractions(state + native["X"]);
} else if (native.count("Y")) {
getMassFractions(state + native["Y"]);
}
}
// replacement
void Phase::saveState(span<double> state) const {
if (state.size() < stateSize()) {
throw ArraySizeError("Phase::saveState", state.size(), stateSize());
}
savePartialState(state.first(partialStateSize()));
auto native = nativeState();
if (native.count("X")) {
getMoleFractions(state.subspan(native["X"], nSpecies()));
} else if (native.count("Y")) {
getMassFractions(state.subspan(native["Y"], nSpecies()));
}
}While the implementation in this case isn't any more succinct, it at least gets rid of the explicit pointer arithmetic, and also eliminates the need for a separate Phase::saveState(vector<double> state) overload.
Regarding the availability of C++20 support, GCC 11.2, available in Ubuntu 22.04, provides std::span as well as most of the C++20 features that I think we would want to use in the near-term. Support in other compilers (Clang, MSVC) has likewise been around for several years at this point, so I think bumping this requirement after the Cantera 3.2 release is reasonable.
The biggest challenge with implementing this, I think, is the sheer number of methods that are affected by this change, and the number of transitional methods that would be created if we were to do this with our usual approach to deprecating and replacing methods in subsequent minor versions. It might only be feasible to consider this as an abrupt breaking change associated with a bump to the major version.