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

proc-macro: Add support for fallible functions #1408

Closed
wants to merge 11 commits into from

Conversation

jplatte
Copy link
Collaborator

@jplatte jplatte commented Nov 22, 2022

Part of #1257.

Remaining tasks:

  • Error derive macro
  • Documentation
  • Testing

@jplatte jplatte force-pushed the jplatte/proc-macro-errors branch 3 times, most recently from bdd9532 to cd1bac4 Compare November 29, 2022 09:40
@jplatte jplatte force-pushed the jplatte/proc-macro-errors branch 3 times, most recently from 31b8d51 to f272c9d Compare December 12, 2022 17:37
@jplatte jplatte marked this pull request as ready for review December 12, 2022 17:38
@jplatte jplatte requested a review from a team as a code owner December 12, 2022 17:38
@jplatte jplatte requested review from mhammond and bendk and removed request for a team December 12, 2022 17:38
@jplatte jplatte closed this Dec 13, 2022
@jplatte jplatte reopened this Dec 13, 2022
@jplatte
Copy link
Collaborator Author

jplatte commented Dec 13, 2022

I was able to rerun from the CircleCI website but it doesn't seem to change anything 🤷🏼

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.

Code wise this looks good.
I have a couple of comments on the docs and one question regarding the implementation (but that's mostly because I'm not as familiar with the code base as I maybe should be)

Once that's tackled I think this can land.

docs/manual/src/proc_macro/index.md Outdated Show resolved Hide resolved
docs/manual/src/proc_macro/index.md Show resolved Hide resolved
Comment on lines +64 to +65
#[automatically_derived]
impl ::uniffi::FfiError for #ident {}
Copy link
Member

Choose a reason for hiding this comment

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

I have been trying to understand why we have this trait.
Is this just a marker?
It doesn't seem to be used anywhere or am I missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, just a marker AFAICT.

docs/manual/src/proc_macro/index.md Outdated Show resolved Hide resolved
docs/manual/src/proc_macro/index.md Outdated Show resolved Hide resolved
@jplatte
Copy link
Collaborator Author

jplatte commented Dec 13, 2022

GitHub is being a bit weird, this is going to be merged as #1423.

@jplatte jplatte closed this Dec 13, 2022
@jplatte jplatte deleted the jplatte/proc-macro-errors branch December 13, 2022 17:09
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