Skip to content

ir: Track the codegen-reachable items, and use it instead of whitelisted_items() for code generation. #836

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 1 commit into from
Jul 21, 2017

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Jul 21, 2017

This standardizes the behavior change at #834, but without regressions.

I've added a few more tests for #833 here.

Closes #834.

@emilio
Copy link
Contributor Author

emilio commented Jul 21, 2017

r? @fitzgen

@emilio
Copy link
Contributor Author

emilio commented Jul 21, 2017

(Only the second third commit needs review, the other two are #835)

@fitzgen
Copy link
Member

fitzgen commented Jul 21, 2017

maybe I should finish reading my inbox before leaving code reviews suggesting exactly what the next item in my inbox does...

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.

r=me with nits below

Thanks :)

});
// The reversal preserves the expected ordering of traversal, resulting
// in more stable-ish bindgen-generated names for anonymous types (like
// unions).
Copy link
Member

Choose a reason for hiding this comment

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

Why did this comment move here? It should be with the reverse call.

self,
roots.clone(),
traversal::codegen_edges,
).collect::<ItemSet>()
Copy link
Member

Choose a reason for hiding this comment

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

Mildly surprised the type inference couldn't figure this out.

@@ -3299,7 +3298,7 @@ pub fn codegen(context: &mut BindgenContext) -> Vec<P<ast::Item>> {
}

context.resolve_item(context.root_module())
.codegen(context, &mut result, whitelisted_items, &());
.codegen(context, &mut result, codegen_items, &());
Copy link
Member

Choose a reason for hiding this comment

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

File a follow up E-Easy issue to remove this argument, and remove the whitelisted_items parameter from all the codegen functions using ctx.codegen_items() instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.

…ted_items() for code generation.

This standardizes the behavior change at rust-lang#834, but without regressions.

I've added a few more tests for rust-lang#833 here.
@emilio
Copy link
Contributor Author

emilio commented Jul 21, 2017

@bors-servo r=fitzgen

@bors-servo
Copy link

📌 Commit 4fee077 has been approved by fitzgen

@bors-servo
Copy link

⌛ Testing commit 4fee077 with merge ed4facf...

bors-servo pushed a commit that referenced this pull request Jul 21, 2017
 	ir: Track the codegen-reachable items, and use it instead of whitelisted_items() for code generation.

This standardizes the behavior change at #834, but without regressions.

I've added a few more tests for #833 here.

Closes #834.
@bors-servo
Copy link

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

@bors-servo bors-servo merged commit 4fee077 into rust-lang:master Jul 21, 2017
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