Skip to content

Add the --override-abi option #2329

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

Merged
merged 8 commits into from
Nov 2, 2022
Merged

Conversation

pvdrz
Copy link
Contributor

@pvdrz pvdrz commented Nov 1, 2022

This option can be used from the CLI with the : syntax and it overrides the ABI of a function if it matches .

Fixes #2257 as the ".*" regex can be used to match every function.

This option can be used from the CLI with the <abi>:<regex> syntax and
it overrides the ABI of a function if it matches <regex>.

Fixes rust-lang#2257 as the `".*"` regex can be used to match every function.
@pvdrz pvdrz requested a review from emilio November 1, 2022 20:01
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.

r=me with that simplification.

@pvdrz
Copy link
Contributor Author

pvdrz commented Nov 2, 2022

@emilio before we move forward with this, we have this question in #2328 where eventually people would like to have another option with the shape <extra_arg><separator><regex>. Maybe we should agree on the separator before moving forward? I used : here but for the other use of this : won't work because <extra_arg> contains :.

Another suggested option is / but I don't know if that's clear enough

@pvdrz pvdrz merged commit 9c32b46 into rust-lang:master Nov 2, 2022
@pvdrz pvdrz deleted the override-abi branch November 2, 2022 20:30
@hcldan
Copy link
Contributor

hcldan commented Dec 1, 2022

@pvdrz The option doc was not updated with the new = char change you made.
Also I think you have the abi=regex backwards in the doc. Is it not regex=abi based on the examples in the tests?

@hcldan
Copy link
Contributor

hcldan commented Dec 1, 2022

Looks like this got caught in another PR. Nevermind.

@pvdrz
Copy link
Contributor Author

pvdrz commented Dec 5, 2022

@hcldan thanks for checking this up! I'm trying to keep those changes in a single PR to avoid this confusion in the future :)

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.

default ABI
3 participants