[BUG] incorrect use of const senmentics in CPP wrapper of C-API
Closed this issue · 0 comments
The Problem
In the CPP wrapper of the C-API, almost all the classes have a unique_ptr
to the opaque pointers returned by the C-API with the custom deleter. This is the standard way of resource management via C-API. However, because of the pointer semantics, the const
functions of these classes can also get non-const
pointer to the actual resource. This leads to mismatch on what is correct between semantically and syntactically.
For example, the add_buffer
function of DatasetMutable
receives a Buffer const& data
which is extremely confusing to the user. Is the buffer provided mutable or not? Moreover, to make the linter happy, we added // NOSONAR: no-const
.
Another example is that the update
function of Model
is const
, confusing for the user.
Root cause
The root cause of the problem is that in a const
version, you still get a non-const
pointer to the actual resource, which is syntactically correct. To make the linter happy, we made many semantically incorrect const
functions, or used // NOSONAR: no-const
to silence the linter.
Solution
To solve this issue, we need to define two get
functions for all the classes.
Customize unique pointer
We customize the get
function of the unique pointer. Concretely we change the using
declaration:
To a customized class:
// unique pointer definition
template <typename T, auto func>
class UniquePtr: public std::unique_ptr<T, DeleterFunctor<func>> {
public:
using std::unique_ptr::unique_ptr;
using std::unique_ptr::operator=;
T* get() { return static_cast<std::unique_ptr<T, DeleterFunctor<func>>&>(*this).get(); }
T const* get() const { return static_cast<std::unique_ptr<T, DeleterFunctor<func>> const&>(*this).get(); }
};
Getter function
We should define two get
functions for all the classes, one for const
and one for non-const
. For example for Model
class, we should do the following.
PowerGridModel* get() { return model_.get(); }
PowerGridModel const* get() const { return model_.get(); }
This forces that the const
object can only return a const
pointer.
Re-route internal functions
We re-route all other functions of the same class by using get
. This is already the case for most of them, but we need to check for sure.
Fix compilation error
Now if we try to compile the code, many places (in the cross reference of the wrapper class, and in the tests) will fail because the const-correctness. We fix the error by removing the incorrect const
declaration in the function arguments, tailing const
, or variable declaration.
Remove the linter hack
Now the linter will not complain about the const
syntactically, remove all the linter hack for SonarCloud and clang-tidy.