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

Allow UDL to avoid the [Rust=...] attribute by using a plain-old ty… #2199

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

mhammond
Copy link
Member

…pedef

This is a WIP and needs a little cleaning up, but thought I'd throw it out there. It allows people to stop using the [Rust=...] attribute in UDL.

This doesn't seem that interesting now, but it will really make sense once Ben's other work lands and we could take this further and use it with [External] (to, eg, drop the entire ExternalInterface attribute) and with [Remote].

@mhammond mhammond force-pushed the push-ukorpwrxznzy branch from 4ecbac7 to 1e53592 Compare July 30, 2024 02:27
Copy link
Member

@badboy badboy left a comment

Choose a reason for hiding this comment

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

I am all here for the simplification!

docs/manual/src/udl/ext_types.md Outdated Show resolved Hide resolved
@mhammond mhammond force-pushed the push-ukorpwrxznzy branch 2 times, most recently from c30c046 to d9dd9ff Compare July 30, 2024 13:35
name,
},
"custom" => panic!("don't know builtin"),
"interface" | "impl" => Type::Object {
Copy link
Member Author

Choose a reason for hiding this comment

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

I should call out that I added a couple of names compared to the Rust attribute. In particular, struct as an alias for record above, and impl as an alias for interface here. My thinking was that these names more closely match what a Rust programmer is likely to think of - ie, a derive(uniffi::Record) lives on a struct defn while uniffi::export lives on an impl block.

On the negative side, impl is also used for traits.

I personally quite like these names, but don't feel too strongly about it, so will happily concede to someone who strongly dislikes them!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm +1 on this.

@mhammond mhammond force-pushed the push-ukorpwrxznzy branch 2 times, most recently from 34baacc to b600a0b Compare August 2, 2024 00:30
@mhammond mhammond marked this pull request as ready for review August 2, 2024 02:30
@mhammond mhammond requested a review from a team as a code owner August 2, 2024 02:30
@mhammond mhammond requested review from bendk and badboy and removed request for a team August 2, 2024 02:30
Copy link
Contributor

@bendk bendk left a comment

Choose a reason for hiding this comment

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

Looks great to me.

name,
},
"custom" => panic!("don't know builtin"),
"interface" | "impl" => Type::Object {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm +1 on this.

@mhammond mhammond force-pushed the push-ukorpwrxznzy branch from b600a0b to 032e873 Compare August 2, 2024 19:51
@mhammond
Copy link
Member Author

mhammond commented Aug 2, 2024

Note that I snuck in a change to move a previously misplaced CHANGELOG entry here.

@mhammond mhammond merged commit 7ff7584 into mozilla:main Aug 2, 2024
5 checks passed
@mhammond mhammond deleted the push-ukorpwrxznzy branch August 2, 2024 20:18
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.

3 participants