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

Remove unused abi attributes. #9606

Merged
merged 2 commits into from
Oct 14, 2013
Merged

Conversation

steveklabnik
Copy link
Member

They've been replaced by putting the name on the extern block.

#[abi = "foo"]

goes to

extern "foo" { }

Closes #9483.

@huonw
Copy link
Member

huonw commented Sep 29, 2013

See #9604 also (I'm inclined to close this one, since @wakandan has been working on it (publically) for a day or so, but this one updates the documentation too).

@steveklabnik
Copy link
Member Author

Oh no! I didn't know that. :(

Close whichever one makes sense. I just picked a random issue from easy and did it.

@alexcrichton
Copy link
Member

It looks like this one covers the FFI dox when the other doesn't, so how about this one takes care of the dox and the other takes care of the compiler?

@steveklabnik
Copy link
Member Author

Seems fine. Should I rebase to remove all of that?

@alexcrichton
Copy link
Member

#9604 hasn't seen a lot of action in the past week or so, so I think it's ok to just rebase and continue on with this as it was originally.

@wakandan
Copy link

wakandan commented Oct 9, 2013

yeah pls do, looking into the compiler code proves to be very challenging to me :(

@steveklabnik
Copy link
Member Author

Done!

@alexcrichton
Copy link
Member

That needed a rebase quickly :(, Also I think that cdecl has become C, so it may just need to get updated from what it once was. Although I also believe that the default is C, so it's possible to omit it now.

@steveklabnik
Copy link
Member Author

Okay!

summary of 26 test runs: 5974 passed; 0 failed; 347 ignored

I think this is good to go now.

@luqmana
Copy link
Member

luqmana commented Oct 11, 2013

I'm pretty sure you should just omit all the cdecl ones and just leave it as a simple extern block.

@steveklabnik
Copy link
Member Author

What im most confused about is why 'make check' Works On My Machine (tm)
but not here.

@@ -107,8 +106,7 @@ pub mod c_float_utils {
use libc::{c_float, c_int};

#[link_name = "m"]
#[abi = "cdecl"]
extern {
extern "cdecl" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this caused build error on android: cdecl is only available on X86/64, not on ARM.

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotcha. is this something that changed? Since I'm just moving things around...

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we started to differentiate cdecl from C when extern "abi" syntax has landed (#5631).
I agree with @luqmana that we should omit cdecl and just use extern { (= extern "C") for all cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Roger!

They've been replaced by putting the name on the extern block.

  #[abi = "foo"]

goes to

  extern "foo" { }

Closes rust-lang#9483.
@steveklabnik
Copy link
Member Author

Okay. I removed the declarations. I didn't have time to wait for full compilation, because I'm getting on a plane, but it got through the first few stages. If bors isn't busy... ;)

bors added a commit that referenced this pull request Oct 14, 2013
They've been replaced by putting the name on the extern block.

  #[abi = "foo"]

goes to

  extern "foo" { }

Closes #9483.
@bors bors closed this Oct 14, 2013
@bors bors merged commit 309ab95 into rust-lang:master Oct 14, 2013
@steveklabnik steveklabnik deleted the abi_removal branch October 25, 2017 18:25
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.

Remove #[abi = "foo"] attributes from codebase.
7 participants