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

Add generated_link_name_override method for ParseCallbacks. #2425

Merged
merged 4 commits into from Mar 19, 2023
Merged

Add generated_link_name_override method for ParseCallbacks. #2425

merged 4 commits into from Mar 19, 2023

Conversation

thb-sb
Copy link
Contributor

@thb-sb thb-sb commented Feb 28, 2023

generated_link_name_override lets the developer choose the link name for
a symbol.

If a link name is specified, the attribute #[link_name = "\u{1}VALUE"] will
be set, overriding any other potential link name, including the mangled name.

This commit also adds an option to the bindgen cli called prefix-link-name,
to prefix the link name of exported symbols.

I think this should properly solve #1375.

@thb-sb thb-sb marked this pull request as ready for review February 28, 2023 08:17
@pvdrz
Copy link
Contributor

pvdrz commented Mar 1, 2023

tests aren't passing. Do you have any idea why?

@bors-servo
Copy link

☔ The latest upstream changes (presumably a8c8638) made this pull request unmergeable. Please resolve the merge conflicts.

@thb-sb
Copy link
Contributor Author

thb-sb commented Mar 2, 2023

tests aren't passing. Do you have any idea why?

it seems that bindgen doesn't output the same rust code for the cpp tests for every host platforms. I'm investigating.

@thb-sb
Copy link
Contributor Author

thb-sb commented Mar 2, 2023

Got some update: it was failing because of the roundtrip test.
Then I undertood the purpose of the parse_callbacks mod, so I added a parse callback called PrefixLinkNameParseCallback to solve the issue.

@bors-servo
Copy link

☔ The latest upstream changes (presumably f019a9b) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo
Copy link

☔ The latest upstream changes (presumably a101063) made this pull request unmergeable. Please resolve the merge conflicts.

@pvdrz
Copy link
Contributor

pvdrz commented Mar 17, 2023

👋 hey I checked your PR and I was wondering if the --prefix-link-name flag is worth it considering that it will be used to just remove the \u{1} prefix. Do you actually need to prefix something to the link name of every function?

@thb-sb
Copy link
Contributor Author

thb-sb commented Mar 17, 2023

👋 hey I checked your PR and I was wondering if the --prefix-link-name flag is worth it considering that it will be used to just remove the \u{1} prefix. Do you actually need to prefix something to the link name of every function?

hey!
In fact it doesn't remove the \u{1} prefix, or did I misunderstand what you meant?

The idea behind prefixing the link name of every function and variables is to be able to link against a library that has been modified by objcopy.
For instance it could be use to avoid C symbol name collisions when you link a rust library that "embeds" a C library against another library that also embeds the very same library.

thb-sb and others added 4 commits March 17, 2023 08:14
`generated_link_name_override` lets the developer choose the link name for
a symbol.

If a link name is specified, the attribute `#[link_name = "\u{1}VALUE"]` will
be set, overriding any other potential link name, including the mangled name.

This commit also adds an option to the bindgen cli called `prefix-link-name`,
to prefix the link name of exported symbols.

I think this should properly solve #1375.
@pvdrz pvdrz merged commit 81e3df4 into rust-lang:main Mar 19, 2023
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.

None yet

3 participants