Skip to content

Some function pointers, typedefs, and OSX's stdlib. #58

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 5 commits into from
Sep 22, 2016

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Sep 21, 2016

r? @nox

The change from indexing cursors to index USRs was because in the inner_template_self test, the next pointer pointed to a clang-generated class declaration that caused us to not catch the type as already resolved.

Also, that's the recommended way to check against same symbols from different translation units, though now we don't use it yet.

@highfive
Copy link

warning Warning warning

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

Copy link
Contributor

@nox nox left a comment

Choose a reason for hiding this comment

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

Two remarks, otherwise r=me.

if !declaration.is_valid() {
if let Some(location) = location {
if location.kind() == CXCursor_ClassTemplate ||
location.kind() == CXCursor_ClassTemplatePartialSpecialization {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe use a match here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I want to use the matches! crate instead, here and in a few other places, but I'll do that in a follow-up.



extern "C" {
pub fn atexit_b(arg1: ::std::option::Option<unsafe extern "C" fn()>);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should just bind blocks to *mut c_void given their ABI is weird.

Copy link
Contributor

@nox nox left a comment

Choose a reason for hiding this comment

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

Forgot one comment.

}
}
declaration = declaration.canonical();
if !declaration.is_valid() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you rebound declaration to the canonical one, which is invalid.

What does the debug! message end up saying, given the declaration you passed to it is the invalid one now?

@nox
Copy link
Contributor

nox commented Sep 22, 2016

@bors-servo r+

@bors-servo
Copy link

📌 Commit cc8ed87 has been approved by nox

@bors-servo
Copy link

⚡ Test exempted - status

@bors-servo bors-servo merged commit cc8ed87 into rust-lang:master Sep 22, 2016
bors-servo pushed a commit that referenced this pull request Sep 22, 2016
Some function pointers, typedefs, and OSX's stdlib.

r? @nox

The change from indexing cursors to index USRs was because in the `inner_template_self` test, the `next` pointer pointed to a clang-generated class declaration that caused us to not catch the type as already resolved.

Also, that's the recommended way to check against same symbols from different translation units, though now we don't use it yet.
@emilio emilio deleted the funcs branch September 22, 2016 10:41
bors-servo pushed a commit that referenced this pull request Sep 23, 2016
Union support

Built on top of #58, this adds support for untagged unions if not running under the `--no-unstable-rust-flag`.

The last commit is an example try run passing with this, but should be reverted (and the `--no-unstable-rust` flag passed by default to our harness).

This depends on serde-deprecated/aster#109.

r? @nox
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