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

c: Use drop-flags when auto-dropping borrows #809

Merged

Conversation

elliottt
Copy link
Member

@elliottt elliottt commented Jan 9, 2024

To expand the cases where auto-dropping is possible in wit-bindgen-c, use drop flags to indicate when borrows need to be dropped. This enables us to also auto-drop borrows present in optional and variant types, leaving only lists as the outlier for values that cannot have their borrows automatically dropped.

This implementation takes advantage of 0 being a newly reserved resource id, allowing us to use that value to indicate when the borrow wasn't observed during lifting.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

If you'd like you can tag the new test as being ignored in Rust and I can investigate that afterwards, looks like you're uncovering a preexisting bug

@alexcrichton
Copy link
Member

Actually I pushed up a fix for that specific issue to this PR so if CI is green I'll merge.

let ty = dealias(self.gen.resolve, *id);

let name = self.locals.tmp("borrow");
uwriteln!(self.borrow_decls, "int32_t {name} = 0;");
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a better way to refer to the type of a handle here, or is using int32_t directly fine?

Copy link
Member

Choose a reason for hiding this comment

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

Nah int32_t is fine, I don't think there's anything "more official" at this time

@alexcrichton alexcrichton merged commit 89f6d36 into bytecodealliance:main Jan 10, 2024
9 checks passed
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