Skip to content

Conversation

pronebird
Copy link
Contributor

@pronebird pronebird commented Sep 24, 2025

This is a boilerplate PR for adding bindings for network framework.

objc-network

  • Add nw_object macros to create network (NW) objects
  • Introduce NWRetained type. Same as Retained but calls into nw_retain and nw_release.
  • Add placeholders for all NW types
  • Add sample wrapper for Path and PathMonitor types raising some questions on how we should handle pointers, bindings.
  • Generated Network framework bindings with Xcode 16.2 (I don't have newer): madsmtm/objc2-generated@3c04ac7

Changes to header-translator

  • Recognize nw_ types in order to use NWRetained instead of Retained
  • Handle NW_RETURNS_RETAINED, NW_NOESCAPE, NW_SWIFT_DISABLE_ASYNC, NW_ENUM

Questions

For what it's worth, it compiles. However I have raised some questions in the code, maybe you could help me to fix the missing bits. Of course in general I'd like to know if the direction I took is the one that you envisioned for network framework and other frameworks too.

Use of self in wrapper types

A lot of nw_* methods are generated with network objects passed as NonNull<T>. This is handled this way in header-translator and I had copied this bit verbatim assuming there is no difference with dispatch2. However It feels weird that i need to use NonNull::from(self), i.e:

pub fn prohibit_interface_type(&mut self, interface_type: InterfaceType) {
  unsafe {
    nw_path_monitor_prohibit_interface_type(
      // fixme: should be able to use &PathMonitor?
      NonNull::from(self),
      interface_type,
    );
  }
}

What woudl be the right approach?

Arrays generated as ArrayUnknownABI

A 6-char C-array was generated as ArrayUnknownABI. What should I specify and where to generate a normal C array? Currently skipping the symbol and providing it manually via lib.rs:

#[allow(non_camel_case_types)]
pub type nw_ethernet_address_t = [core::ffi::c_uchar; 6];

Type-aliases for generated types

I have to define type-aliases in lib.rs since I subtitute nw_*_t types with my own structs, for example:

#[allow(non_camel_case_types)]
type nw_advertise_descriptor_t = AdvertiseDescriptor;

Should I type-alias it wrapped in NWRetained, i.e NWRetained<AdvertiseDescriptor>?

@madsmtm madsmtm added enhancement New feature or request A-framework Affects the framework crates and the translator for them labels Sep 26, 2025
@madsmtm madsmtm added this to the frameworks v0.4.0 milestone Sep 26, 2025
Copy link
Owner

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! I had a quick look, I'll try to give a more detailed review later, but I'm a bit busy with v0.3.2 and such atm. (yeah, sorry, this won't go in that release).

I actually did a bit of work on this in the past, have pushed https://github.com/madsmtm/objc2/tree/wip-network if you want to take a look. IIRC I got stuck on the naming of things. I think I'd like the prefix NW, e.g. nw_path_t -> NWPath like Swift does it, but that's already used by NetworkExtension, so we might have to do something different here.

Use of self in wrapper types

This is handled in global_analysis.rs, I suspect it'll fix itself if you figure out the name translation stuff (it works in my branch above).

Arrays generated as ArrayUnknownABI

I'd actually rather lean towards skipping nw_ethernet_channel_send and nw_ethernet_channel_set_receive_handler altogether for now, arrays do have a weird ABI in function arguments, I can't recall the details but it's something like that they actually need to be passed by reference (and I'm not sure rustc handles this correctly?).

Type-aliases for generated types

No, I'd strongly prefer that, if we expose them, that type aliases like these aren't wrapped in NWRetained (even though that's technically incorrect, the type alias is a pointer).

Comment on lines 1 to 4
nw_object!(
#[doc(alias = "nw_protocol_definition_t")]
pub struct ProtocolDefinition;
);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm leaning towards modifying header-translator to output these instead of manually emitting them. I didn't do so for the Dispatch types, but there's ~30 types using NW_OBJECT_DECL, so here it might make more sense?

Besides, I am actually regretting it a bit for Dispatch, because we're missing the documentation comments for its types.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that means adding it as something like Stmt::NetworkDecl (with a lot of fields copied from Stmt::OpaqueDecl).

Comment on lines 15 to 17
pub fn new_with_interface_type(interface_type: InterfaceType) -> NWRetained<Self> {
nw_path_monitor_create_with_type(interface_type)
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a way for header-translator to automatically emit these as associated functions, you might need to modify the find_fn_implementor logic in name_translation.rs (it's kind of a mess tbh).

Copy link
Contributor Author

@pronebird pronebird Sep 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth to note that in your branch, you translate the following signature:

nw_txt_record_find_key_t nw_txt_record_find_key(nw_txt_record_t txt_record, const char * key);

into:

impl nw_txt_record_find_key_t {
  pub unsafe fn new(txt_record: &NWTxtRecord, key: NonNull<c_char>) -> nw_txt_record_find_key_t;
}

By looking at Swift, I think it's meant to be attached to NWTxtRecord instead. Some methods are also outliers, i.e in this case the function and the type very much share the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely familiar with the header-translation pipeline. Currently it seems that many things are handled in different places but maybe that's because I am not familiar with how it works.

For example:

nw_txt_record_t nw_txt_record_copy(nw_txt_record_t txt_record);

is generated into

impl NWTxtRecord {
  pub fn copy(txt_record: Option<&NWTxtRecord>) -> Option<NWRetained<NWTxtRecord>> { .. }
}

Given that we change implementer to NWTxtRecord, I'd have expect the first argument be replaced with &self but I am not sure where this transform should happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK so this can be done in global_analysis.rs

// Replace the first argument with self when translating standalone functions into instance methods
//
// nw_txt_record_t nw_txt_record_copy(nw_txt_record_t txt_record);
//
// To:
//
// impl NWTxtRecord {
//   fn copy(&self) -> NWRetained<NWTxtRecord>;
// }
//
if id.library_name() == "Network"
    && arguments
        .first()
        .and_then(|(_, first_arg_ty)| first_arg_ty.implementable())
        .is_some_and(|v| v == cf_item)
{
    *first_arg_is_self = true;
}

@PaulDance
Copy link
Contributor

PaulDance commented Sep 26, 2025

I'd like the prefix NW, e.g. nw_path_t -> NWPath like Swift does it, but that's already used by NetworkExtension

From what I can see in the docs, when searching for nw in the bottom left bar, almost everything that comes up is either some kind of Obj-C class around a Network-like type that is now deprecated in favor of the Network type or something directly from Network itself. Furthermore, these constitute a minority compared to the rest that starts with NE, as one would expect more. It would seem like these wrappers should have been directly in Network, hence the naming, but were made available from NetworkExtension instead for some obscure reasons.

Considering this, I think using the NW prefix here would be preferable in order to remain consistent with how other frameworks have their items named and how this one already has the nw_ prefix in most items, if it's technically feasible. It would introduce some slight confusion, but it would effectively represent what is exposed in a transparent manner, i.e. the confusion is more on them than on us. Also, there shouldn't be too much of a risk of clashing, except in downstream imports that can always be adapted, right?

@pronebird
Copy link
Contributor Author

pronebird commented Sep 28, 2025

I'd like the prefix NW, e.g. nw_path_t -> NWPath like Swift does it, but that's already used by NetworkExtension

From what I can see in the docs, when searching for nw in the bottom left bar, almost everything that comes up is either some kind of Obj-C class around a Network-like type that is now deprecated in favor of the Network type or something directly from Network itself. Furthermore, these constitute a minority compared to the rest that starts with NE, as one would expect more. It would seem like these wrappers should have been directly in Network, hence the naming, but were made available from NetworkExtension instead for some obscure reasons.

Considering this, I think using the NW prefix here would be preferable in order to remain consistent with how other frameworks have their items named and how this one already has the nw_ prefix in most items, if it's technically feasible. It would introduce some slight confusion, but it would effectively represent what is exposed in a transparent manner, i.e. the confusion is more on them than on us. Also, there shouldn't be too much of a risk of clashing, except in downstream imports that can always be adapted, right?

I think that class prefix is only relevant for Swift/Objc due semantics of how modules work in those languages. Rust does not have this problem, hence a few deprecated types from NetworkExtension that carry the same name shouldn't matter. On the other side I don't know why the prefix is even used in Rust bindings, it looks ugly. Personally I don't care all that much for prefix. Whichever way we go, my primary objective is to be able to use network framework from Rust :)

@pronebird
Copy link
Contributor Author

pronebird commented Sep 28, 2025

Alright so I kinda forced header translator to produce something decent. We might need to separate cf_fn_name() from NW because I have seen some odd side effects. My patch is quite terrible as it is right now, it needs a decent refactor.

Latest bindings:
https://github.com/pronebird/objc2-generated/blob/network-bindings/Network/mod.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Affects the framework crates and the translator for them enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants