Put content of CassDataType into UnsafeCell

Previously `CassDataType` was just an enum, held inside `Arc`. User was
given a pointer to `CassDataType` using `Arc::as_ptr` or `Arc::into_ptr`.
There are however some functions that mutate the data - and they were
given the very same pointers.
Current code was most likely sound - but I'm not completely sure, Rust
reference is very confusing in this aspect.
It was however very confusing - when a programmer reads or writes
a function that that *mut CassDataType it is not obivious that this data
lies inside Arc and so has shared ownership.

To make this more explicit this commit puts `CassDataType` inside
UnsafeCell. Now each access needs to use `.get_unchecked()` and
`.get_mut_unchecked()` methods and an unsafe block / function, so it
will be easier to spot aliasing ^ mutability problems in the future.

In the future we can use `Arc::get_mut_unchecked()` for this purpose,
but it's not yet stabilised.

Author: Lorak-mmk

scylla-rust-wrapper/src/cass_types.rs

@@ -1,25 +1,25 @@
 use crate::cass_collection_types::CassCollectionType;
 use crate::cass_error::CassError;
-use crate::cass_types::{CassDataType, MapDataType};
+use crate::cass_types::{CassDataType, CassDataTypeInner, MapDataType};
 use crate::types::*;
 use crate::value::CassCqlValue;
 use crate::{argconv::*, value};
 use std::convert::TryFrom;
 use std::sync::Arc;
 
 // These constants help us to save an allocation in case user calls `cass_collection_new` (untyped collection).
-static UNTYPED_LIST_TYPE: CassDataType = CassDataType::List {
+static UNTYPED_LIST_TYPE: CassDataType = CassDataType::new(CassDataTypeInner::List {
     typ: None,
     frozen: false,
-};
-static UNTYPED_SET_TYPE: CassDataType = CassDataType::Set {
+});
+static UNTYPED_SET_TYPE: CassDataType = CassDataType::new(CassDataTypeInner::Set {
     typ: None,
     frozen: false,
-};
-static UNTYPED_MAP_TYPE: CassDataType = CassDataType::Map {
+});
+static UNTYPED_MAP_TYPE: CassDataType = CassDataType::new(CassDataTypeInner::Map {
     typ: MapDataType::Untyped,
     frozen: false,
-};
+});
 
 #[derive(Clone)]
 pub struct CassCollection {
@@ -35,18 +35,22 @@ impl CassCollection {
         let index = self.items.len();
 
         // Do validation only if it's a typed collection.
-        if let Some(data_type) = &self.data_type {
-            match data_type.as_ref() {
-                CassDataType::List { typ: subtype, .. }
-                | CassDataType::Set { typ: subtype, .. } => {
+        if let Some(data_type) = &self
+            .data_type
+            .as_ref()
+            .map(|dt| unsafe { dt.get_unchecked() })
+        {
+            match data_type {
+                CassDataTypeInner::List { typ: subtype, .. }
+                | CassDataTypeInner::Set { typ: subtype, .. } => {
                     if let Some(subtype) = subtype {
                         if !value::is_type_compatible(value, subtype) {
                             return CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE;
                         }
                     }
                 }
 
-                CassDataType::Map { typ, .. } => {
+                CassDataTypeInner::Map { typ, .. } => {
                     // Cpp-driver does the typecheck only if both map types are present...
                     // However, we decided not to mimic this behaviour (which is probably a bug).
                     // We will do the typecheck if just the key type is defined as well (half-typed maps).
@@ -146,12 +150,16 @@ unsafe extern "C" fn cass_collection_new_from_data_type(
     item_count: size_t,
 ) -> *mut CassCollection {
     let data_type = clone_arced(data_type);
-    let (capacity, collection_type) = match data_type.as_ref() {
-        CassDataType::List { .. } => (item_count, CassCollectionType::CASS_COLLECTION_TYPE_LIST),
-        CassDataType::Set { .. } => (item_count, CassCollectionType::CASS_COLLECTION_TYPE_SET),
+    let (capacity, collection_type) = match data_type.get_unchecked() {
+        CassDataTypeInner::List { .. } => {
+            (item_count, CassCollectionType::CASS_COLLECTION_TYPE_LIST)
+        }
+        CassDataTypeInner::Set { .. } => (item_count, CassCollectionType::CASS_COLLECTION_TYPE_SET),
         // Maps consist of a key and a value, so twice
         // the number of CassCqlValue will be stored.
-        CassDataType::Map { .. } => (item_count * 2, CassCollectionType::CASS_COLLECTION_TYPE_MAP),
+        CassDataTypeInner::Map { .. } => {
+            (item_count * 2, CassCollectionType::CASS_COLLECTION_TYPE_MAP)
+        }
         _ => return std::ptr::null_mut(),
     };
     let capacity = capacity as usize;
@@ -216,7 +224,7 @@ mod tests {
 
     use crate::{
         cass_error::CassError,
-        cass_types::{CassDataType, CassValueType, MapDataType},
+        cass_types::{CassDataType, CassDataTypeInner, CassValueType, MapDataType},
         collection::{
             cass_collection_append_double, cass_collection_append_float, cass_collection_free,
         },
@@ -256,7 +264,7 @@ mod tests {
 
             // untyped map (via cass_collection_new_from_data_type - collection's type is Some(untyped_map)).
             {
-                let dt = Arc::new(CassDataType::Map {
+                let dt = CassDataType::new_arced(CassDataTypeInner::Map {
                     typ: MapDataType::Untyped,
                     frozen: false,
                 });
@@ -285,8 +293,8 @@ mod tests {
 
             // half-typed map (key-only)
             {
-                let dt = Arc::new(CassDataType::Map {
-                    typ: MapDataType::Key(Arc::new(CassDataType::Value(
+                let dt = CassDataType::new_arced(CassDataTypeInner::Map {
+                    typ: MapDataType::Key(CassDataType::new_arced(CassDataTypeInner::Value(
                         CassValueType::CASS_VALUE_TYPE_BOOLEAN,
                     ))),
                     frozen: false,
@@ -324,10 +332,12 @@ mod tests {
 
             // typed map
             {
-                let dt = Arc::new(CassDataType::Map {
+                let dt = CassDataType::new_arced(CassDataTypeInner::Map {
                     typ: MapDataType::KeyAndValue(
-                        Arc::new(CassDataType::Value(CassValueType::CASS_VALUE_TYPE_BOOLEAN)),
-                        Arc::new(CassDataType::Value(
+                        CassDataType::new_arced(CassDataTypeInner::Value(
+                            CassValueType::CASS_VALUE_TYPE_BOOLEAN,
+                        )),
+                        CassDataType::new_arced(CassDataTypeInner::Value(
                             CassValueType::CASS_VALUE_TYPE_SMALL_INT,
                         )),
                     ),
@@ -383,7 +393,7 @@ mod tests {
 
             // untyped set (via cass_collection_new_from_data_type, collection's type is Some(untyped_set))
             {
-                let dt = Arc::new(CassDataType::Set {
+                let dt = CassDataType::new_arced(CassDataTypeInner::Set {
                     typ: None,
                     frozen: false,
                 });
@@ -404,8 +414,8 @@ mod tests {
 
             // typed set
             {
-                let dt = Arc::new(CassDataType::Set {
-                    typ: Some(Arc::new(CassDataType::Value(
+                let dt = CassDataType::new_arced(CassDataTypeInner::Set {
+                    typ: Some(CassDataType::new_arced(CassDataTypeInner::Value(
                         CassValueType::CASS_VALUE_TYPE_BOOLEAN,
                     ))),
                     frozen: false,
@@ -443,7 +453,7 @@ mod tests {
 
             // untyped list (via cass_collection_new_from_data_type, collection's type is Some(untyped_list))
             {
-                let dt = Arc::new(CassDataType::Set {
+                let dt = CassDataType::new_arced(CassDataTypeInner::Set {
                     typ: None,
                     frozen: false,
                 });
@@ -464,8 +474,8 @@ mod tests {
 
             // typed list
             {
-                let dt = Arc::new(CassDataType::Set {
-                    typ: Some(Arc::new(CassDataType::Value(
+                let dt = CassDataType::new_arced(CassDataTypeInner::Set {
+                    typ: Some(CassDataType::new_arced(CassDataTypeInner::Value(
                         CassValueType::CASS_VALUE_TYPE_BOOLEAN,
                     ))),
                     frozen: false,

scylla-rust-wrapper/src/collection.rs

@@ -1,7 +1,8 @@
 use crate::argconv::*;
 use crate::cass_error::CassError;
 use crate::cass_types::{
-    cass_data_type_type, get_column_type, CassColumnSpec, CassDataType, CassValueType, MapDataType,
+    cass_data_type_type, get_column_type, CassColumnSpec, CassDataType, CassDataTypeInner,
+    CassValueType, MapDataType,
 };
 use crate::inet::CassInet;
 use crate::metadata::{
@@ -195,10 +196,10 @@ fn create_cass_row_columns(row: Row, metadata: &Arc<CassResultMetadata>) -> Vec<
 }
 
 fn get_column_value(column: CqlValue, column_type: &Arc<CassDataType>) -> Value {
-    match (column, column_type.as_ref()) {
+    match (column, unsafe { column_type.get_unchecked() }) {
         (
             CqlValue::List(list),
-            CassDataType::List {
+            CassDataTypeInner::List {
                 typ: Some(list_type),
                 ..
             },
@@ -212,7 +213,7 @@ fn get_column_value(column: CqlValue, column_type: &Arc<CassDataType>) -> Value
         )),
         (
             CqlValue::Map(map),
-            CassDataType::Map {
+            CassDataTypeInner::Map {
                 typ: MapDataType::KeyAndValue(key_type, value_type),
                 ..
             },
@@ -234,7 +235,7 @@ fn get_column_value(column: CqlValue, column_type: &Arc<CassDataType>) -> Value
         )),
         (
             CqlValue::Set(set),
-            CassDataType::Set {
+            CassDataTypeInner::Set {
                 typ: Some(set_type),
                 ..
             },
@@ -252,7 +253,7 @@ fn get_column_value(column: CqlValue, column_type: &Arc<CassDataType>) -> Value
                 type_name,
                 fields,
             },
-            CassDataType::UDT(udt_type),
+            CassDataTypeInner::UDT(udt_type),
         ) => CollectionValue(Collection::UserDefinedType {
             keyspace,
             type_name,
@@ -274,7 +275,7 @@ fn get_column_value(column: CqlValue, column_type: &Arc<CassDataType>) -> Value
                 })
                 .collect(),
         }),
-        (CqlValue::Tuple(tuple), CassDataType::Tuple(tuple_types)) => {
+        (CqlValue::Tuple(tuple), CassDataTypeInner::Tuple(tuple_types)) => {
             CollectionValue(Collection::Tuple(
                 tuple
                     .into_iter()
@@ -1472,7 +1473,7 @@ pub unsafe extern "C" fn cass_value_is_collection(value: *const CassValue) -> ca
     let val = ptr_to_ref(value);
 
     matches!(
-        val.value_type.get_value_type(),
+        val.value_type.get_unchecked().get_value_type(),
         CassValueType::CASS_VALUE_TYPE_LIST
             | CassValueType::CASS_VALUE_TYPE_SET
             | CassValueType::CASS_VALUE_TYPE_MAP
@@ -1483,7 +1484,8 @@ pub unsafe extern "C" fn cass_value_is_collection(value: *const CassValue) -> ca
 pub unsafe extern "C" fn cass_value_is_duration(value: *const CassValue) -> cass_bool_t {
     let val = ptr_to_ref(value);
 
-    (val.value_type.get_value_type() == CassValueType::CASS_VALUE_TYPE_DURATION) as cass_bool_t
+    (val.value_type.get_unchecked().get_value_type() == CassValueType::CASS_VALUE_TYPE_DURATION)
+        as cass_bool_t
 }
 
 #[no_mangle]
@@ -1508,15 +1510,15 @@ pub unsafe extern "C" fn cass_value_primary_sub_type(
 ) -> CassValueType {
     let val = ptr_to_ref(collection);
 
-    match val.value_type.as_ref() {
-        CassDataType::List {
+    match val.value_type.get_unchecked() {
+        CassDataTypeInner::List {
             typ: Some(list), ..
-        } => list.get_value_type(),
-        CassDataType::Set { typ: Some(set), .. } => set.get_value_type(),
-        CassDataType::Map {
+        } => list.get_unchecked().get_value_type(),
+        CassDataTypeInner::Set { typ: Some(set), .. } => set.get_unchecked().get_value_type(),
+        CassDataTypeInner::Map {
             typ: MapDataType::Key(key) | MapDataType::KeyAndValue(key, _),
             ..
-        } => key.get_value_type(),
+        } => key.get_unchecked().get_value_type(),
         _ => CassValueType::CASS_VALUE_TYPE_UNKNOWN,
     }
 }
@@ -1527,11 +1529,11 @@ pub unsafe extern "C" fn cass_value_secondary_sub_type(
 ) -> CassValueType {
     let val = ptr_to_ref(collection);
 
-    match val.value_type.as_ref() {
-        CassDataType::Map {
+    match val.value_type.get_unchecked() {
+        CassDataTypeInner::Map {
             typ: MapDataType::KeyAndValue(_, value),
             ..
-        } => value.get_value_type(),
+        } => value.get_unchecked().get_value_type(),
         _ => CassValueType::CASS_VALUE_TYPE_UNKNOWN,
     }
 }
@@ -1614,7 +1616,7 @@ mod tests {
 
     use crate::{
         cass_error::CassError,
-        cass_types::{CassDataType, CassValueType},
+        cass_types::{CassDataType, CassDataTypeInner, CassValueType},
         query_result::{
             cass_result_column_data_type, cass_result_column_name, cass_result_first_row,
             ptr_to_cstr_n, ptr_to_ref, size_t,
@@ -1723,22 +1725,26 @@ mod tests {
             {
                 let first_col_data_type = ptr_to_ref(cass_result_column_data_type(result_ptr, 0));
                 assert_eq!(
-                    &CassDataType::Value(CassValueType::CASS_VALUE_TYPE_BIGINT),
+                    &CassDataType::new(CassDataTypeInner::Value(
+                        CassValueType::CASS_VALUE_TYPE_BIGINT
+                    )),
                     first_col_data_type
                 );
                 let second_col_data_type = ptr_to_ref(cass_result_column_data_type(result_ptr, 1));
                 assert_eq!(
-                    &CassDataType::Value(CassValueType::CASS_VALUE_TYPE_VARINT),
+                    &CassDataType::new(CassDataTypeInner::Value(
+                        CassValueType::CASS_VALUE_TYPE_VARINT
+                    )),
                     second_col_data_type
                 );
                 let third_col_data_type = ptr_to_ref(cass_result_column_data_type(result_ptr, 2));
                 assert_eq!(
-                    &CassDataType::List {
-                        typ: Some(Arc::new(CassDataType::Value(
+                    &CassDataType::new(CassDataTypeInner::List {
+                        typ: Some(CassDataType::new_arced(CassDataTypeInner::Value(
                             CassValueType::CASS_VALUE_TYPE_DOUBLE
                         ))),
                         frozen: false
-                    },
+                    }),
                     third_col_data_type
                 );
                 let out_of_bound_col_data_type = cass_result_column_data_type(result_ptr, 555);

scylla-rust-wrapper/src/query_result.rs

@@ -1,7 +1,7 @@
 use crate::argconv::*;
 use crate::batch::CassBatch;
 use crate::cass_error::*;
-use crate::cass_types::{CassDataType, UDTDataType};
+use crate::cass_types::{CassDataType, CassDataTypeInner, UDTDataType};
 use crate::cluster::build_session_builder;
 use crate::cluster::CassCluster;
 use crate::exec_profile::{CassExecProfile, ExecProfileName, PerStatementExecProfile};
@@ -509,7 +509,7 @@ pub unsafe extern "C" fn cass_session_get_schema_meta(
         for udt_name in keyspace.user_defined_types.keys() {
             user_defined_type_data_type.insert(
                 udt_name.clone(),
-                Arc::new(CassDataType::UDT(UDTDataType::create_with_params(
+                CassDataType::new_arced(CassDataTypeInner::UDT(UDTDataType::create_with_params(
                     &keyspace.user_defined_types,
                     keyspace_name,
                     udt_name,

scylla-rust-wrapper/src/session.rs

@@ -1,12 +1,13 @@
 use crate::argconv::*;
 use crate::cass_error::CassError;
 use crate::cass_types::CassDataType;
+use crate::cass_types::CassDataTypeInner;
 use crate::types::*;
 use crate::value;
 use crate::value::CassCqlValue;
 use std::sync::Arc;
 
-static UNTYPED_TUPLE_TYPE: CassDataType = CassDataType::Tuple(Vec::new());
+static UNTYPED_TUPLE_TYPE: CassDataType = CassDataType::new(CassDataTypeInner::Tuple(Vec::new()));
 
 #[derive(Clone)]
 pub struct CassTuple {
@@ -17,8 +18,8 @@ pub struct CassTuple {
 impl CassTuple {
     fn get_types(&self) -> Option<&Vec<Arc<CassDataType>>> {
         match &self.data_type {
-            Some(t) => match &**t {
-                CassDataType::Tuple(v) => Some(v),
+            Some(t) => match unsafe { t.as_ref().get_unchecked() } {
+                CassDataTypeInner::Tuple(v) => Some(v),
                 _ => unreachable!(),
             },
             None => None,
@@ -70,8 +71,8 @@ unsafe extern "C" fn cass_tuple_new_from_data_type(
     data_type: *const CassDataType,
 ) -> *mut CassTuple {
     let data_type = clone_arced(data_type);
-    let item_count = match &*data_type {
-        CassDataType::Tuple(v) => v.len(),
+    let item_count = match data_type.get_unchecked() {
+        CassDataTypeInner::Tuple(v) => v.len(),
         _ => return std::ptr::null_mut(),
     };
     Box::into_raw(Box::new(CassTuple {