Skip to content

A better fix for the calling convention madness. #1403

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 2 commits into from
Sep 28, 2018

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Sep 26, 2018

See the commit message for details.

I just realized that we don't really check for this but that clang does.
Seems like a better fix, which allows us to preserve typedefs properly, and
also to find the calling convention.

Fixes rust-lang#1402.
@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@emilio
Copy link
Contributor Author

emilio commented Sep 26, 2018

r? @fitzgen

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

LGTM! One question / suggestion below.

if opt.starts_with("--target=") {
let mut split = opt.split('=');
split.next();
return (split.next().unwrap().to_owned(), true);
}

if opt == "-target" {
Copy link
Member

Choose a reason for hiding this comment

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

should this also handle two -?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$ clang --target foo
clang-6.0: error: unsupported option '--target'

So i think not.

@emilio
Copy link
Contributor Author

emilio commented Sep 28, 2018

@bors-servo r=fitzgen

@bors-servo
Copy link

📌 Commit c75b62d has been approved by fitzgen

@bors-servo
Copy link

⌛ Testing commit c75b62d with merge c6f9fe0...

bors-servo pushed a commit that referenced this pull request Sep 28, 2018
 A better fix for the calling convention madness.

See the commit message for details.
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: fitzgen
Pushing c6f9fe0 to master...

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.

4 participants