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

Minimal support for Rust type aliases #1181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

schreter
Copy link
Contributor

The intention is to provide support to re-import Rust types already exported elsewhere into other bridges, creating equivalence of extern "Rust" types on the C++ side.

Currently, the support is limited to re-exporting the type under the same name and the C++ namespace must be explicitly specified. Then, C++ functions can use the same type.

The intention is to provide support to re-import Rust types already
exported elsewhere into other bridges, creating equivalence of
`extern "Rust"` types on the C++ side.

Currently, the support is limited to re-exporting the type under the
same name and the C++ namespace must be explicitly specified. Then,
C++ functions can use the same type.
@schreter schreter mentioned this pull request Mar 1, 2023
@@ -6,9 +6,10 @@ impl Api {
match self {
Api::CxxFunction(efn) | Api::RustFunction(efn) => &efn.name.namespace,
Api::CxxType(ety) | Api::RustType(ety) => &ety.name.namespace,
Api::TypeAlias(ety) => &ety.name.namespace,
Copy link

Choose a reason for hiding this comment

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

Merge the branch with the above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, that's not possible, since those have different inner types.

Copy link

Choose a reason for hiding this comment

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

Acknowledged.

if let RustType::Path(path) = &ty {
// OK, we support path
if let Some(last) = path.path.segments.last() {
if last.ident != alias.name.rust {
Copy link

Choose a reason for hiding this comment

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

Not sure if we really need to prevent this - some may really want to use different names for the same Rust type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is the C++ side. Effectively, what we want to do is to publish a forward declaration of the struct in question in the C++ code in the proper namespace (that's why also the namespace should match, otherwise the types are different on C++ side - currently, this is not checked, but probably should be).

If we'd like to support renaming the object, that would be possible, but again, that would require materializing both the forward declaration and some kind of type alias (e.g., using statement), so we would have both. The name would be accessible by both names and the original name taken anyway.

I.e., someone trying to rename the type to prevent name clashes would still end up with name clashes. Therefore, better to simply not allow renames.

The only way to do this properly would be to expose the original type in the original namespace and the alias name in a new namespace. However, due to the restriction on the Rust side, we only have access to tokens within our bridge, so there is no access to the original namespace (that's also the reason why the namespace must be re-specified).

Copy link

Choose a reason for hiding this comment

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

I see, thanks - I misunderstood the intention.

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 this pull request may close these issues.

2 participants