Naming Conventions and Organization

[zicklag]

Hey there! I'll start by saying I really like this library and it's super useful, but I feel like the module organization and type naming conventions feel a little weird and I was wondering what you thought about maybe tidying it up a little bit. Some, for instances:

• When I include the prelude, it feels a little weird that I get some module prefixes such as c_slice, repr_c, and str. It's not that bad, but subjectively it's just kind of hard to guess. Like, it wasn't clear to me initially that not everything would be in repr_c when I was looking for the slice types, then when I needed a string I found it was in the str module, but that felt like it should have been named c_str or something at that point, not just str.
• In the example we have:

  #[derive_ReprC]
  #[repr(C)]
  #[derive(Debug, Clone, Copy)]
  pub
  struct Point {
      x: f64,
      y: f64,
  }

  having both derive(Debug, Clone, Copy) and a separate derive_ReprC annotation feels really strange. I'm sure there's probably technical reasons for it, but if we have to do a derive_ReprC macro, maybe we could name it something else and make it fully snake_case like the other macros such as ffi_export. It would feel a lot more natural that way I think.

Anyway, just some thoughts. 🙂

[danielhenrymantilla]

Yes, tidying up the names and stuff is planned to be done rather soon (it's been one of the main requirements for 0.x.0 releases).

Some ideas:

• If the macro can be made into a derive, then indeed it's very likely to become #[derive(ReprC)] (right now it hasn't since ReprC delegates to a CType derive, and a derive macro cannot expand to another derive macro invocation).

  Should it not be possible, then the expected syntax would become #[safer_ffi::repr(C)] (so as to remove the "repr(C) redundancy" of the current annotations).

• I have recently had an idea of using a helper trait and a type alias to feature nicer paths: we could have repr_c::Box<[T]> Just Work™:

  //! `repr_c` module.
  use internals::*;
  mod internals {
      pub struct CBox<T> …;
      mod c_slice {
          // repr(C) version of Box<[T]>
          pub struct Box<T> …
      }
      pub trait Boxable { type Boxed; }
      impl<T> Boxable for T { type Boxed = Box<T>; }
      impl<T> Boxable for [T] { type Boxed = c_slice::CBox<T>; }
      // and even:
      impl Boxable for c_str { type Boxed = char_p::Box; }
  }
  pub type Box<Pointee : ?Sized + Boxable> = <Pointee as Boxable>::Boxed;

  With this, and with Ref<'_, Pointee> / RefMut<'_, Pointee> wrappers (we can't type alias &'_ T 😄), all the types could be unified as:

  use ::safer_ffi::prelude::{*, repr_c as c}; // this may be end up being exposed in the prelude:
  
  #[ffi_export]
  fn example (
      x: c::Box<[u8]>,
      y: c::RefMut<'_, i32>,
      c_str: c::Ref<'_, c_str>, // layout: `char const *`
      utf8_bytes: c::Ref<'_, str>, // layout: same as `slice_ref_uint8_t`
  ) -> c::Option<c::Box<c_str>>

  The c:: for Option may not be mandatory, but if it's deemed acceptable it could offer a robust solution to Argument is not FFI-safe #31.

• Since the repr_c:: prefix (or even the c:: shorthand) may be a bit too noisy when used in a nested fashion (although it will be handy to easy C-ize a function signature: just namespace most types using c::), I may / could also offer a naming convention to offer C…-prefixed types:

  use ::safer_ffi::prelude::{*, repr_c as c}; // this may be end up being exposed in the prelude:
  
  #[ffi_export]
  fn example (
      x: CBox<[u8]>,
      y: CRefMut<'_, i32>,
      c_str: CRef<'_, c_str>, // layout: `char const *`
      utf8_bytes: CRef<'_, str>, // layout: same as `slice_ref_uint8_t`
  ) -> COption<CBox<c_str>>

To any readers out there, feel free to (politely) chime in regarding these ideas 🙂

[zicklag]

Awesome, that all sounds good. 🙂 👍

As far as the c:: prefix or just using a C… prefix for the struct names, I could go either way with it, they seem about equal to me. On one hand the c:: module feels kind of "tidy" somehow ( maybe in the fact that the :: triggers the auto-complete in the IDE with a list of the options you have inside of it ), but the C class prefix is easier to type.

As far as using c::Option, that really doesn't seem to be a big deal to me. All of these FFI function signatures are going to be a bit doctored anyway and it being apparent as to the explicit means taken to make them FFI safe doesn't seem to be a bad thing to me. The thing I would be worried about would be whether or not both c::Option and Option would work. If they both worked it could be confusing to users as to when it would be necessary to do one or the other, but with good documentation and maybe warnings from the macro for potential issues, that shouldn't be a problem.

then the expected syntax would become #[safer_ffi::repr(C)] (so as to remove the "repr(C) redundancy" of the current annotations).

That's a lot nicer. 👍