-
Notifications
You must be signed in to change notification settings - Fork 338
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,8 @@ use crate::syntax::report::Errors; | |
use crate::syntax::visit::{self, Visit}; | ||
use crate::syntax::{ | ||
error, ident, trivial, Api, Array, Enum, ExternFn, ExternType, Impl, Lang, Lifetimes, | ||
NamedType, Ptr, Receiver, Ref, Signature, SliceRef, Struct, Trait, Ty1, Type, TypeAlias, Types, | ||
NamedType, Ptr, Receiver, Ref, RustType, Signature, SliceRef, Struct, Trait, Ty1, Type, | ||
TypeAlias, Types, | ||
}; | ||
use proc_macro2::{Delimiter, Group, Ident, TokenStream}; | ||
use quote::{quote, ToTokens}; | ||
|
@@ -500,6 +501,25 @@ fn check_api_type_alias(cx: &mut Check, alias: &TypeAlias) { | |
let msg = format!("derive({}) on extern type alias is not supported", derive); | ||
cx.error(derive, msg); | ||
} | ||
|
||
if alias.lang == Lang::Rust { | ||
let ty = &alias.ty; | ||
if let RustType::Path(path) = &ty { | ||
// OK, we support path | ||
if let Some(last) = path.path.segments.last() { | ||
if last.ident != alias.name.rust { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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., 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, thanks - I misunderstood the intention. |
||
cx.error( | ||
&alias.name.rust, | ||
"`extern \"Rust\"` alias must have the same name as the target type", | ||
); | ||
} | ||
} else { | ||
cx.error(ty, "unsupported `extern \"Rust\"` alias target type"); | ||
} | ||
} else { | ||
cx.error(ty, "unsupported `extern \"Rust\"` alias target"); | ||
} | ||
} | ||
} | ||
|
||
fn check_api_impl(cx: &mut Check, imp: &Impl) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
error: type alias in extern "Rust" block is not supported | ||
--> tests/ui/type_alias_rust.rs:5:9 | ||
error: `extern "Rust"` alias must have the same name as the target type | ||
--> tests/ui/type_alias_rust.rs:5:14 | ||
| | ||
5 | type Alias = crate::Type; | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| ^^^^^ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
pub mod other_module { | ||
pub struct Source { | ||
member: u32, | ||
} | ||
} | ||
|
||
#[cxx::bridge] | ||
mod ffi { | ||
extern "Rust" { | ||
// Not allowed - the target is not `extern "Rust"`. | ||
type Source = crate::other_module::Source; | ||
} | ||
} | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
error[E0277]: the trait bound `other_module::Source: RustType` is not satisfied | ||
--> tests/ui/type_alias_rust2.rs:11:23 | ||
| | ||
11 | type Source = crate::other_module::Source; | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `RustType` is not implemented for `other_module::Source` | ||
| | ||
note: required by a bound in `verify_rust_type` | ||
--> src/rust_type.rs | ||
| | ||
| pub fn verify_rust_type<T: RustType>() {} | ||
| ^^^^^^^^ required by this bound in `verify_rust_type` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
struct Type; | ||
|
||
#[cxx::bridge] | ||
mod ffi { | ||
extern "Rust" { | ||
// Not allowed - the target is not a path. | ||
type Source = &crate::Type; | ||
} | ||
} | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
error: unsupported `extern "Rust"` alias target | ||
--> tests/ui/type_alias_rust3.rs:7:23 | ||
| | ||
7 | type Source = &crate::Type; | ||
| ^^^^^^^^^^^^ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged.