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

use tuple style struct in C binding #5167

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

asukaminato0721
Copy link
Contributor

Which issue does this PR close?

Closes #5161

Rationale for this change

What changes are included in this PR?

use tuple to avoid inner.

Are there any user-facing changes?

@asukaminato0721 asukaminato0721 changed the title example use tuple style struct in C binding Oct 7, 2024
@asukaminato0721
Copy link
Contributor Author

the change in readme: https://stackoverflow.com/a/21594/13040423

@tisonkun
Copy link
Member

tisonkun commented Oct 7, 2024

Fair enough to move forward in my opinion.

What do you think? @Xuanwo @yuchanns @xyjixyjixyji

@Xuanwo
Copy link
Member

Xuanwo commented Oct 7, 2024

It seems like it's just changing from .inner to .0. I'm not sure if it's better to stick with the old style for a more readable and consistent approach when we have more than one field like in opendal_bytes.

Does it make a difference in understanding or performance in a public API on the C side?

@yuchanns
Copy link
Member

yuchanns commented Oct 7, 2024

It seems like it's just changing from .inner to .0. I'm not sure if it's better to stick with the old style for a more readable and consistent approach when we have more than one field like in opendal_bytes.

Does it make a difference in understanding or performance in a public API on the C side?

+1

Does it impact the performance?

@asukaminato0721
Copy link
Contributor Author

Does it make a difference in understanding or performance in a public API on the C side?

no.

Tuple style is just a syntax sugar that has 0 instead of inner as a field.

@tisonkun
Copy link
Member

tisonkun commented Oct 8, 2024

So we may suspend this request and perhaps review it again before we build the release process for the C binding.

@xyjixyjixyji
Copy link
Contributor

IMO inner name provides better semantics when developers are reading the actual header files.

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.

5 participants