treewide: change *const to *mut based on cassandra.h #225
Conversation
|
I see that there are other types with this issue. I'll address them as well. |
There was some reasoning in the comment to `cass_data_type_new` why the type was changed to *const. I think it is misleading to change the return type. Thus, I'm changing it back to *mut (and in other places, so we are consistent with cassandra.h).
muzarski
force-pushed
the
cassdatatype-mut
branch
from
a964116 to
9b246ed
Compare
muzarski
force-pushed
the
cassdatatype-mut
branch
from
9b246ed to
bd177d4
Compare
muzarski
requested review from
wprzytula and
Lorak-mmk
and removed request for
wprzytula
muzarski
changed the title
There was a problem hiding this comment.
I see no harm in this change.
I don't know in general what pointer types should we used.
There are 2 competing semantics, Rust one (mut = exclusive, const = shared) and C++ one (mut = logically mutable, const = logically immutable). Afaik *const and *mut are not strictly bound to any of them.
IIUC, this PR aligns them with C++ semantics, right?
Correct. Take for example some type T, that is shared but can be mutated. In rust, we would represent it as e.g. |
There are some types that are shared (
Arc), but are mutable from cpp-driver perspective.For some reason, there were some discrepancies between C API (defined in cassandra.h) and rust binding functions. On C side, the pointer would be
T*(non-const), while on Rust side it would be*const T. This PR fixes those discrepancies, so we are consistent with cassandra.hWhile going over these types, I found out that for exampleNvm, I just didn't notice thatCassResultunnecessarily implementsArcFFI. I'll address it in a separate PR (because there are other things to adjust as well).CassResultis cloned incass_future_get_result- this is because we use.clone()here and notArc::clone(). I'll change it then, so it's easier to spot in the future.Pre-review checklist
[ ] I have implemented Rust unit tests for the features/changes introduced.[ ] I have enabled appropriate tests in.github/workflows/build.ymlingtest_filter.[ ] I have enabled appropriate tests in.github/workflows/cassandra.ymlingtest_filter.