Mark non-pub fields as private in c++ headers

[ejmahler]

If I have the following Rust struct:

#[repr(C)]
pub struct MyStruct {
    value1: i32,
    pub value2: i32
}

It would be very useful for the generated struct to reflect the field's visibility:

struct MyStruct {
private:
    int32_t value1;
public:
    int32_t value2;
};

My main motivation for this is to create exported structs where all fields are private. An example is a pointer wrapper: If I allocate a pointer for an opaque struct with Box and then return the raw pointer, there's nothing stopping the caller from modifying it before passing it back for destruction. If instead I make a struct to wrap it, and give it private fields, all the caller can do is copy the struct around and pass it back unchanged.

A change like this would enable APIs that were more type-safe and more in the spirit of Rust.

An alternate, lower-tech approach would be an annotation to just unconditionally mark all fields private. I imagine most use-cases will want all-private or all-public fields, so an annotation to set them all private might be a smarter solution, and save field-specific visibility for a use case that needs it.

What do you think? Is there interest in this? If so I'd be happy to tackle it.

[emilio]

This would need to be opt-in as having private fields breaks the standard layout contract in C++.

But more to the point, if you want a type to be opaque, then you can just not mark it #[repr(C)], and cbindgen should do the right thing and forward-declare it.

[ejmahler]

This would need to be opt-in as having private fields breaks the standard layout contract in C++.

Interesting, I didn't know about standard layout.

It says any struct where all fields have the same visibility is standard layout. I wrote a test:

#include <iostream>
#include <type_traits>

struct AllPublic {
public:
	int var1;
	int var2;
};

struct AllProtected {
protected:
	int var1;
	int var2;
};

struct AllPrivate {
private:
	int var1;
	int var2;
};

struct Mixed {
public:
	int var1;
private:
	int var2;
};

void main() {
	std::cout << "AllPublic    is standard layout: " << std::is_standard_layout<AllPublic>() << std::endl;
	std::cout << "AllProtected is standard layout: " << std::is_standard_layout<AllProtected>() << std::endl;
	std::cout << "AllPrivate   is standard layout: " << std::is_standard_layout<AllPrivate>() << std::endl;
	std::cout << "Mixed        is standard layout: " << std::is_standard_layout<Mixed>() << std::endl;
}

Output:

AllPublic    is standard layout: 1
AllProtected is standard layout: 1
AllPrivate   is standard layout: 1
Mixed        is standard layout: 0

So a struct with all-private fields is standard layout. But yes, we should probably axe the idea of making individual fields private or public, and stick with the simpler solution of an annotation that marks all fields unconditionally private.

But more to the point, if you want a type to be opaque, then you can just not mark it #[repr(C)], and cbindgen should do the right thing and forward-declare it.

Making the type opaque to C++ doesn't solve the problem I'm trying to solve. My goal is to prevent the caller from passing invalid pointers to my Rust library. If I make my type opaque, all the C++ side gets is a forward declaration, so I have to pass out a pointer to it, which the caller can then modify before passing back.

I want to prevent them from being able to modify the pointer, and making the pointer a private field of a struct is an idiomatic C++ way to prevent callers from editing it. This would mean that the only way to get one is A: to get it from a function in my library that returns one, or B: Make a copy of one that already exists. Or I guess C: invoke the default constructor (which it would be nice to be able to delete)

[emilio]

I want to prevent them from being able to modify the pointer, and making the pointer a private field of a struct is an idiomatic C++ way to prevent callers from editing it. This would mean that the only way to get one is A: to get it from a function in my library that returns one, or B: Make a copy of one that already exists. Or I guess C: invoke the default constructor (which it would be nice to be able to delete)

You can sort of do all those things with something like:

[export.body]
"YourPointerWrapper" = """
  YourPointerWrapper() = delete;
 private:
"""

I think.

[ejmahler]

This is almost exactly what I want!

Sadly, this adds the text at the bottom:

struct PointerWrapper {
  RustStruct *inner;
  PointerWrapper() = delete;
 private:
};

[emilio]

Ah that's too bad... Would be easy to to add a field-visibility annotation or such to cbindgen then, I guess, which effectively sets the visibility of all fields together.

[ejmahler]

How unreasonable would it to be to add:

[export.body_top]
"YourPointerWrapper" = """
  YourPointerWrapper() = delete;
 private:
"""

It's a little hacky, but it's more in line with what's already there, and would be infinitely more flexible than a feature that specifically marked fields private.

[emilio]

Probably fine as well, yeah.

[ejmahler]

Ok! I will look into that. I'll submit a PR in the next few days. Thanks for your advice.