Merge generic enum tags

[Benjamin-Davies]

See attached issue.

Resolves #784

[morrisonlevi]

Looks good to me as a total outsider. @emilio, are you the right person to ping these days? You have the most commits last year.

[emilio]

Sorry for the lag here, I haven't been able to take a look lately at cbindgen stuff.

In general the feature makes sense, but instead of the Rc<Cell<>> shenanigans and having to keep the unmangled paths around, it seems it could be implemented more cleanly and efficiently in instantiate_monomorph. At that point, you know you have a generic, and you're going to monomorphize it. It should be feasible to synthesize a tag enum there, and just carry an is_monomorphic: bool or so to avoid generating the tag if needed.

[emilio]

Thanks! Looks much better. Some final requests, and also can you add the new option to the docs?

With that it looks good.

[emilio]

Thanks, if CI is green this looks good. Can you squash your commits in the way you best see fit so that we don't have "Tidy up" commits? Or would you be fine with me squashing before merging?

[mxyns]

hi all, i am interested in getting this PR merged as it would greatly help me in my project 🙂 Is there something I can do to make this happen?
@emilio are you still the person to ping or should is there someone else you can refer us to?

[mxyns]

hi @emilio
Do you think this could be merged soon?
It looks like @Benjamin-Davies has addressed your last comment about tidying up commits.

[morrisonlevi]

I emailed Emilio and Ben, and Ben gave approval for squash-merging! So we just need to get conflicts resolved, and we should good to go.

[morrisonlevi]

Hmm, I merged in master (PR) and I think something is still not quite right. I'll work on a simpler reproducer. When mixing this with a few other things, I can get some strange results (ha), e.g.:

typedef enum Result_Tag {
  PROFILE_EXPORTER_RESULT_OK_HANDLE_PROFILE_EXPORTER,
  PROFILE_EXPORTER_RESULT_ERR_HANDLE_PROFILE_EXPORTER,
} Result_Tag;

[morrisonlevi]

Reproducer for the issues. Example rust code:

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

#[no_mangle]
pub extern "C" fn fn1() -> Result<u32, u64> {
    Result::Ok(0)
}

#[no_mangle]
pub extern "C" fn fn2() -> Result<u64, u32> {
    Result::Ok(0)
}

With this cbindgen.toml:

language = "C"
include_guard = "DDOG_CBINDGEN_TEST_H"
no_includes = true
sys_includes = ["stdbool.h", "stddef.h", "stdint.h"]

[export]
prefix = "ddog_cbindgen_"

[enum]
merge_generic_tags = true
prefix_with_name = true
rename_variants = "ScreamingSnakeCase"

Results in this output (except I've annotated certain things):

#ifndef DDOG_CBINDGEN_TEST_H
#define DDOG_CBINDGEN_TEST_H

#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

typedef enum ddog_cbindgen_Result_Tag {
  DDOG_CBINDGEN_RESULT_OK,
  DDOG_CBINDGEN_RESULT_ERR,
} ddog_cbindgen_Result_Tag;

// NOTE: Missed the rename on the enum here.
typedef enum Result_Tag {
  // NOTE: missed merging generics here.
  DDOG_CBINDGEN_RESULT_U32_U64_OK_U32_U64,
  DDOG_CBINDGEN_RESULT_U32_U64_ERR_U32_U64,
} Result_Tag;

typedef struct ddog_cbindgen_Result_u32__u64 {
  Result_Tag tag;
  union {
    struct {
      uint32_t ok;
    };
    struct {
      uint64_t err;
    };
  };
} ddog_cbindgen_Result_u32__u64;

// NOTE: Missed the rename on the enum here.
typedef enum Result_Tag {
  // NOTE: missed merging generics here.
  DDOG_CBINDGEN_RESULT_U64_U32_OK_U64_U32,
  DDOG_CBINDGEN_RESULT_U64_U32_ERR_U64_U32,
} Result_Tag;

typedef struct ddog_cbindgen_Result_u64__u32 {
  Result_Tag tag;
  union {
    struct {
      uint64_t ok;
    };
    struct {
      uint32_t err;
    };
  };
} ddog_cbindgen_Result_u64__u32;

struct ddog_cbindgen_Result_u32__u64 fn1(void);

struct ddog_cbindgen_Result_u64__u32 fn2(void);

#endif  /* DDOG_CBINDGEN_TEST_H */