Skip to content

Conversation

ivmaykov
Copy link

Fixes #3286

@ivmaykov
Copy link
Author

ivmaykov commented Oct 2, 2025

r? @emilio @pvdrz

@ivmaykov ivmaykov force-pushed the issue-3286 branch 3 times, most recently from 5a2c0f4 to aa5e696 Compare October 2, 2025 16:31
@ivmaykov
Copy link
Author

Is bindgen effectively unmaintained? What can I do to get this PR merged? Is this a controversial change? If so, can I have some feedback?

@emilio
Copy link
Contributor

emilio commented Oct 10, 2025

Bindgen is a free-time project for me, this PR was opened two weeks ago.

Please be patient because I unfortunately don't have a lot of free time lately :)

Copy link
Contributor

@emilio emilio 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 but I don't think the hash set is going to be particularly worth it for derive lists in particular, and not doing that would simplify the code a bit, wdyt?

LMK if you disagree tho.

where
I: Iterator<Item = &'a str>,
{
// Use a HashSet to track already seen elements.
Copy link
Contributor

Choose a reason for hiding this comment

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

derive lists are usually pretty small, I don't think the copy + hashset overhead is going to be particularly worth it in this case...

That way the is_empty check on the caller can go. WDYT?

.derive_partialord(false)
.derive_ord(false)
.derive_hash(false)
.parse_callbacks(Box::new(AddDerivesCallback::new(add_derives)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be less code to extend bindgen-integration perhaps? But this is fine.

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.

ParseCallbacks::add_derives() should not cause a trait to be derived twice

2 participants