Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge generic tag enums #784

Open
Benjamin-Davies opened this issue Sep 14, 2022 · 2 comments · May be fixed by #785
Open

Merge generic tag enums #784

Benjamin-Davies opened this issue Sep 14, 2022 · 2 comments · May be fixed by #785

Comments

@Benjamin-Davies
Copy link

At the moment, when a generic tagged enum is used, a tag enum is generated for every specialization of the generic. However, the possible tags and their values do not change based on the generic parameters, so these tag enums are identical.

There should be an option to merge these enums into one that ignores the generics, perhaps enum.merge_generic_tag.

This would shorten types and improve readability for common use cases such as result types, where the tag is often checked manually after a function call, especially in cases where the generic types are long.

Example

Rust code:

#[repr(C)]
pub enum COption<T> {
    Some(T),
    None,
}

#[no_mangle]
pub extern "C" fn example(_: COption<u8>, _: COption<u32>) {}

Current output:

typedef enum COption_u8_Tag {
  Some_u8,
  None_u8,
} COption_u8_Tag;

typedef struct COption_u8 {
  COption_u8_Tag tag;
  union {
    struct {
      uint8_t some;
    };
  };
} COption_u8;

typedef enum COption_u32_Tag {
  Some_u32,
  None_u32,
} COption_u32_Tag;

typedef struct COption_u32 {
  COption_u32_Tag tag;
  union {
    struct {
      uint32_t some;
    };
  };
} COption_u32;

Ideal output:

typedef enum COption_Tag {
  Some,
  None,
} COption_Tag;

typedef struct COption_u8 {
  COption_Tag tag;
  union {
    struct {
      uint8_t some;
    };
  };
} COption_u8;

typedef struct COption_u32 {
  COption_Tag tag;
  union {
    struct {
      uint32_t some;
    };
  };
} COption_u32;
@Benjamin-Davies Benjamin-Davies linked a pull request Sep 14, 2022 that will close this issue
@morrisonlevi
Copy link

Chiming in to say that I'd like this for results too, as well as better names for tags in general (though that may be another PR):

use std::marker::PhantomData;

#[repr(C)]
#[derive(Debug)]
pub struct Vec<T: Sized> {
    ptr: *const T,
    len: usize,
    capacity: usize,
    _marker: PhantomData<T>,
}

#[repr(C)]
pub struct Error(Vec<u8>);

#[repr(C)]
pub enum Result<T> {
    Ok(T),
    Err(Error),
}

pub type I32Result = Result<i32>;
pub type U32Result = Result<u32>;

#[no_mangle]
pub unsafe extern "C" fn iresult() -> I32Result {
    I32Result::Ok(42)
}

#[no_mangle]
pub unsafe extern "C" fn uresult() -> U32Result {
    I32Result::Ok(42)
}

Doing cbindgen --lang c .:

#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>

typedef struct Vec_u8 {
  const uint8_t *ptr;
  uintptr_t len;
  uintptr_t capacity;
} Vec_u8;

typedef struct Error {
  struct Vec_u8 _0;
} Error;

typedef enum Result_i32_Tag {
  Ok_i32,
  Err_i32,
} Result_i32_Tag;

typedef struct Result_i32 {
  Result_i32_Tag tag;
  union {
    struct {
      int32_t ok;
    };
    struct {
      struct Error err;
    };
  };
} Result_i32;

typedef struct Result_i32 I32Result;

typedef enum Result_u32_Tag {
  Ok_u32,
  Err_u32,
} Result_u32_Tag;

typedef struct Result_u32 {
  Result_u32_Tag tag;
  union {
    struct {
      uint32_t ok;
    };
    struct {
      struct Error err;
    };
  };
} Result_u32;

typedef struct Result_u32 U32Result;

I32Result iresult(void);

U32Result uresult(void);

Here, there are two problems:

  1. The tag is duplicated per-type, instead of Result_Tag.
  2. Even if I we don't have this merging feature, the fact that the tag name is Result_u32_Tag instead of U32Result_Tag is also wrong, imo: the rename needs to happen before _Tag is suffixed.

Anyway, point 1 is showing that I'd love to have this feature too. Your PR looks good to me!

@mxyns
Copy link

mxyns commented Oct 13, 2023

Hey there!
I'm in exactly the same use case as @Benjamin-Davies :)
It also looks like the PR was fine for the reviewer too. Maybe the reviewer just forgot to merge it ?
@emilio could you check it out again, please ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants