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

Fix typos in the external-symbols feature(bug) #199

Merged
merged 2 commits into from
Apr 4, 2020

Conversation

elichai
Copy link
Member

@elichai elichai commented Feb 9, 2020

Hi,
This was introduced in #169

There was a typo in some of the feature gates from external-symbols to external_symbols.
it wasn't caught because it was either not pub extern "C" (like secp256k1_context_create) or prefixed with the rustsecp256k1_v0_1_1_* which made it not collide with other symbols.

Second thing is we still have some leftovers from the dont_replace_c_symbols feature (#177) which we no longer support since #169 was merged and prefixed all the symbols so they're not really easily usable in C/C++ code anyway.
this feature isn't in our Cargo.toml anyway so these are no-ops anyway now.

@elichai elichai changed the title Fix typos in the external-symbols feature Fix typos in the external-symbols feature(bug) Feb 9, 2020
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK. Surprised the compiler didn't complain about our check on the feature dont_replace_c_symbols when no such feature is defined in our Cargo.tomls.

@apoelstra apoelstra merged commit ab59498 into rust-bitcoin:master Apr 4, 2020
@elichai
Copy link
Member Author

elichai commented Apr 5, 2020

ACK. Surprised the compiler didn't complain about our check on the feature dont_replace_c_symbols when no such feature is defined in our Cargo.tomls.

Sadly I think the current way cargo+rustc implements features this isn't possible,
rustc allows you to pass --cfg '****' with whatever cfg you want, cargo takes a --feature=MY_FEATURE and converts it into --cfg 'feature="MY_FEATURE"'.

so rustc will accept whatever cfg and will let you gate whatever cfg you want (even not via features ie see: https://github.com/alexcrichton/proc-macro2/blob/b7daef7967be1e71a3d5525e3298241c42a19215/src/fallback.rs#L8 )

I will open a cargo issue about it but I'm not sure this is even technically possible to implement with the current architecture.

@elichai elichai deleted the 2020-02-external-symbols branch April 5, 2020 07:41
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