always implement traits for C type

lib/src/atomic.rs

@@ -3,13 +3,13 @@
 use crate::traits::*;
 
 impl PassByValue for usize {
-    type CType = usize;
+    type RustType = usize;
 
-    unsafe fn from_ctype(arg: usize) -> usize {
+    unsafe fn from_ctype(self) -> usize {
-        arg
-    }
-
-    fn as_ctype(self) -> usize {
         self
     }
+
+    fn as_ctype(arg: usize) -> usize {
+        arg
+    }
 }

lib/src/replica.rs

@@ -1,7 +1,7 @@
 use crate::traits::*;
 use crate::util::err_to_tcstring;
 use crate::{result::TCResult, status::TCStatus, string::TCString, task::TCTask, uuid::TCUuid};
-use taskchampion::{Replica, StorageConfig, Uuid};
+use taskchampion::{Replica, StorageConfig};
 
 /// A replica represents an instance of a user's task data, providing an easy interface
 /// for querying and modifying that data.
@@ -143,7 +143,7 @@ pub extern "C" fn tc_replica_get_task(rep: *mut TCReplica, tcuuid: TCUuid) -> *m
         rep,
         |rep| {
             // SAFETY: see TCUuid docstring
-            let uuid = unsafe { Uuid::from_arg(tcuuid) };
+            let uuid = unsafe { TCUuid::from_arg(tcuuid) };
             if let Some(task) = rep.get_task(uuid)? {
                 Ok(TCTask::from(task).return_val())
             } else {
@@ -187,7 +187,7 @@ pub extern "C" fn tc_replica_import_task_with_uuid(
         rep,
         |rep| {
             // SAFETY: see TCUuid docstring
-            let uuid = unsafe { Uuid::from_arg(tcuuid) };
+            let uuid = unsafe { TCUuid::from_arg(tcuuid) };
             let task = rep.import_task_with_uuid(uuid)?;
             Ok(TCTask::from(task).return_val())
         },

lib/src/string.rs

@@ -255,7 +255,7 @@ pub extern "C" fn tc_string_content_with_len(
     //  - len_out is not NULL (promised by caller)
     //  - len_out points to valid memory (promised by caller)
     //  - len_out is properly aligned (C convention)
-    unsafe { bytes.len().to_arg_out(len_out) };
+    unsafe { usize::to_arg_out(bytes.len(), len_out) };
     bytes.as_ptr() as *const libc::c_char
 }
 

lib/src/strings.rs

@@ -3,37 +3,37 @@ use crate::traits::*;
 use crate::util::{drop_pointer_array, vec_into_raw_parts};
 use std::ptr::NonNull;
 
-/// TCStrings represents a list of string.
+/// TCStrings represents a list of strings.
 ///
 /// The content of this struct must be treated as read-only.
 #[repr(C)]
 pub struct TCStrings {
-    /// number of tags in items
+    /// number of strings in items
     len: libc::size_t,
 
     /// total size of items (internal use only)
     _capacity: libc::size_t,
 
-    /// strings representing each tag. these remain owned by the TCStrings instance and will be freed
+    /// strings representing each string. these remain owned by the TCStrings instance and will be freed
     /// by tc_strings_free.
     items: *const NonNull<TCString<'static>>,
 }
 
-impl PassByValue for Vec<NonNull<TCString<'static>>> {
+impl PassByValue for TCStrings {
-    type CType = TCStrings;
+    type RustType = Vec<NonNull<TCString<'static>>>;
 
-    unsafe fn from_ctype(arg: TCStrings) -> Self {
+    unsafe fn from_ctype(self) -> Self::RustType {
         // SAFETY:
         //  - C treats TCStrings as read-only, so items, len, and _capacity all came
         //    from a Vec originally.
-        unsafe { Vec::from_raw_parts(arg.items as *mut _, arg.len, arg._capacity) }
+        unsafe { Vec::from_raw_parts(self.items as *mut _, self.len, self._capacity) }
     }
 
-    fn as_ctype(self) -> TCStrings {
+    fn as_ctype(arg: Self::RustType) -> Self {
         // emulate Vec::into_raw_parts():
         // - disable dropping the Vec with ManuallyDrop
         // - extract ptr, len, and capacity using those methods
-        let (items, len, _capacity) = vec_into_raw_parts(self);
+        let (items, len, _capacity) = vec_into_raw_parts(arg);
         TCStrings {
             len,
             _capacity,
@@ -55,10 +55,10 @@ impl Default for TCStrings {
 /// Free a TCStrings instance.  The instance, and all TCStrings it contains, must not be used after
 /// this call.
 #[no_mangle]
-pub extern "C" fn tc_strings_free<'a>(tctags: *mut TCStrings) {
+pub extern "C" fn tc_strings_free<'a>(tcstrings: *mut TCStrings) {
-    debug_assert!(!tctags.is_null());
+    debug_assert!(!tcstrings.is_null());
     // SAFETY:
-    //  - *tctags is a valid TCStrings (caller promises to treat it as read-only)
+    //  - *tcstrings is a valid TCStrings (caller promises to treat it as read-only)
-    let tags = unsafe { Vec::take_from_arg(tctags, TCStrings::default()) };
+    let strings = unsafe { TCStrings::take_from_arg(tcstrings, TCStrings::default()) };
-    drop_pointer_array(tags);
+    drop_pointer_array(strings);
 }

lib/src/task.rs

@@ -247,7 +247,7 @@ pub extern "C" fn tc_task_to_immut<'a>(task: *mut TCTask) {
 /// Get a task's UUID.
 #[no_mangle]
 pub extern "C" fn tc_task_get_uuid(task: *mut TCTask) -> TCUuid {
-    wrap(task, |task| task.get_uuid().return_val())
+    wrap(task, |task| TCUuid::return_val(task.get_uuid()))
 }
 
 /// Get a task's status.
@@ -331,7 +331,7 @@ pub extern "C" fn tc_task_get_tags<'a>(task: *mut TCTask) -> TCStrings {
                 .expect("TCString::return_val() returned NULL")
             })
             .collect();
-        tcstrings.return_val()
+        TCStrings::return_val(tcstrings)
     })
 }
 

lib/src/traits.rs

@@ -2,29 +2,30 @@
 /// values are implicitly copyable, via C's struct assignment.
 ///
 /// The Rust and C types may differ, with from_ctype and as_ctype converting between them.
+/// Implement this trait for the C type.
 pub(crate) trait PassByValue: Sized {
-    type CType;
+    type RustType;
 
     /// Convert a C value to a Rust value.
     ///
     /// # Safety
     ///
-    /// `arg` must be a valid CType.
+    /// `self` must be a valid CType.
-    unsafe fn from_ctype(arg: Self::CType) -> Self;
+    unsafe fn from_ctype(self) -> Self::RustType;
 
     /// Convert a Rust value to a C value.
-    fn as_ctype(self) -> Self::CType;
+    fn as_ctype(arg: Self::RustType) -> Self;
 
     /// Take a value from C as an argument.
     ///
     /// # Safety
     ///
-    /// `arg` must be a valid CType.  This is typically ensured either by requiring that C
+    /// `self` must be a valid CType.  This is typically ensured either by requiring that C
     /// code not modify it, or by defining the valid values in C comments.
-    unsafe fn from_arg(arg: Self::CType) -> Self {
+    unsafe fn from_arg(arg: Self) -> Self::RustType {
         // SAFETY:
         //  - arg is a valid CType (promised by caller)
-        unsafe { Self::from_ctype(arg) }
+        unsafe { arg.from_ctype() }
     }
 
     /// Take a value from C as a pointer argument, replacing it with the given value.  This is used
@@ -33,7 +34,7 @@ pub(crate) trait PassByValue: Sized {
     /// # Safety
     ///
     /// `*arg` must be a valid CType, as with [`from_arg`].
-    unsafe fn take_from_arg(arg: *mut Self::CType, mut replacement: Self::CType) -> Self {
+    unsafe fn take_from_arg(arg: *mut Self, mut replacement: Self) -> Self::RustType {
         // SAFETY:
         //  - arg is valid (promised by caller)
         //  - replacement is valid (guaranteed by Rust)
@@ -44,8 +45,8 @@ pub(crate) trait PassByValue: Sized {
     }
 
     /// Return a value to C
-    fn return_val(self) -> Self::CType {
+    fn return_val(arg: Self::RustType) -> Self {
-        self.as_ctype()
+        Self::as_ctype(arg)
     }
 
     /// Return a value to C, via an "output parameter"
@@ -54,12 +55,12 @@ pub(crate) trait PassByValue: Sized {
     ///
     /// `arg_out` must not be NULL and must be properly aligned and pointing to valid memory
     /// of the size of CType.
-    unsafe fn to_arg_out(self, arg_out: *mut Self::CType) {
+    unsafe fn to_arg_out(val: Self::RustType, arg_out: *mut Self) {
         debug_assert!(!arg_out.is_null());
         // SAFETY:
         //  - arg_out is not NULL (promised by caller, asserted)
         //  - arg_out is properly aligned and points to valid memory (promised by caller)
-        unsafe { *arg_out = self.as_ctype() };
+        unsafe { *arg_out = Self::as_ctype(val) };
     }
 }
 

lib/src/uuid.rs

@@ -10,30 +10,30 @@ use taskchampion::Uuid;
 #[repr(C)]
 pub struct TCUuid([u8; 16]);
 
-impl PassByValue for Uuid {
+impl PassByValue for TCUuid {
-    type CType = TCUuid;
+    type RustType = Uuid;
 
-    unsafe fn from_ctype(arg: TCUuid) -> Self {
+    unsafe fn from_ctype(self) -> Self::RustType {
         // SAFETY:
         //  - any 16-byte value is a valid Uuid
-        Uuid::from_bytes(arg.0)
+        Uuid::from_bytes(self.0)
     }
 
-    fn as_ctype(self) -> TCUuid {
+    fn as_ctype(arg: Uuid) -> Self {
-        TCUuid(*self.as_bytes())
+        TCUuid(*arg.as_bytes())
     }
 }
 
 /// Create a new, randomly-generated UUID.
 #[no_mangle]
 pub extern "C" fn tc_uuid_new_v4() -> TCUuid {
-    Uuid::new_v4().return_val()
+    TCUuid::return_val(Uuid::new_v4())
 }
 
 /// Create a new UUID with the nil value.
 #[no_mangle]
 pub extern "C" fn tc_uuid_nil() -> TCUuid {
-    Uuid::nil().return_val()
+    TCUuid::return_val(Uuid::nil())
 }
 
 // NOTE: this must be a simple constant so that cbindgen can evaluate it
@@ -56,7 +56,7 @@ pub extern "C" fn tc_uuid_to_buf<'a>(tcuuid: TCUuid, buf: *mut libc::c_char) {
     };
     // SAFETY:
     //  - tcuuid is a valid TCUuid (all byte patterns are valid)
-    let uuid: Uuid = unsafe { Uuid::from_arg(tcuuid) };
+    let uuid: Uuid = unsafe { TCUuid::from_arg(tcuuid) };
     uuid.to_hyphenated().encode_lower(buf);
 }
 
@@ -66,7 +66,7 @@ pub extern "C" fn tc_uuid_to_buf<'a>(tcuuid: TCUuid, buf: *mut libc::c_char) {
 pub extern "C" fn tc_uuid_to_str(tcuuid: TCUuid) -> *mut TCString<'static> {
     // SAFETY:
     //  - tcuuid is a valid TCUuid (all byte patterns are valid)
-    let uuid: Uuid = unsafe { Uuid::from_arg(tcuuid) };
+    let uuid: Uuid = unsafe { TCUuid::from_arg(tcuuid) };
     let s = uuid.to_string();
     // SAFETY: see TCString docstring
     unsafe { TCString::from(s).return_val() }
@@ -85,7 +85,7 @@ pub extern "C" fn tc_uuid_from_str<'a>(s: *mut TCString, uuid_out: *mut TCUuid)
             // SAFETY:
             //  - uuid_out is not NULL (promised by caller)
             //  - alignment is not required
-            unsafe { u.to_arg_out(uuid_out) };
+            unsafe { TCUuid::to_arg_out(u, uuid_out) };
             return true;
         }
     }

lib/taskchampion.h

@@ -127,16 +127,13 @@ typedef struct TCUuid {
 } TCUuid;
 
 /**
- * TCStrings represents a list of tags associated with a task.
+ * TCStrings represents a list of strings.
  *
  * The content of this struct must be treated as read-only.
- *
- * The lifetime of a TCStrings instance is independent of the task, and it
- * will remain valid even if the task is freed.
  */
 typedef struct TCStrings {
   /**
-   * number of tags in items
+   * number of strings in items
    */
   size_t len;
   /**
@@ -144,7 +141,7 @@ typedef struct TCStrings {
    */
   size_t _capacity;
   /**
-   * strings representing each tag. these remain owned by the TCStrings instance and will be freed
+   * strings representing each string. these remain owned by the TCStrings instance and will be freed
    * by tc_strings_free.
    */
   struct TCString *const *items;
@@ -276,7 +273,7 @@ void tc_string_free(struct TCString *tcstring);
  * Free a TCStrings instance.  The instance, and all TCStrings it contains, must not be used after
  * this call.
  */
-void tc_strings_free(struct TCStrings *tctags);
+void tc_strings_free(struct TCStrings *tcstrings);
 
 /**
  * Convert an immutable task into a mutable task.
@@ -349,13 +346,16 @@ bool tc_task_is_waiting(struct TCTask *task);
 bool tc_task_is_active(struct TCTask *task);
 
 /**
- * Check if a task has the given tag.  If the tag is invalid, this function will simply return
+ * Check if a task has the given tag.  If the tag is invalid, this function will return false, as
- * false, with no error from `tc_task_error`.
+ * that (invalid) tag is not present. No error will be reported via `tc_task_error`.
  */
 bool tc_task_has_tag(struct TCTask *task, struct TCString *tag);
 
 /**
- * Get the tags for the task.  The task must not be NULL.
+ * Get the tags for the task.
+ *
+ * The caller must free the returned TCStrings instance.  The TCStrings instance does not
+ * reference the task and the two may be freed in any order.
  */
 struct TCStrings tc_task_get_tags(struct TCTask *task);