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

Separate the analysis framework in a different file. #797

Merged
merged 1 commit into from
Jul 7, 2017
Merged

Separate the analysis framework in a different file. #797

merged 1 commit into from
Jul 7, 2017

Conversation

photoszzt
Copy link
Contributor

@highfive
Copy link

highfive commented Jul 7, 2017

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @fitzgen (or someone else) soon.

@fitzgen
Copy link
Member

fitzgen commented Jul 7, 2017

Looks good to me, but just need to fix a compilation error before this can land:

   Compiling bindgen v0.26.3 (file:///home/travis/build/servo/rust-bindgen)
error: function `analyze` is private
 --> src/ir/context.rs:8:44
  |
8 | use super::named::{UsedTemplateParameters, analyze};
  |                                            ^^^^^^^
error: aborting due to previous error
error: Could not compile `bindgen`.

Thanks!

@fitzgen
Copy link
Member

fitzgen commented Jul 7, 2017

analyze needs to be imported from super::ir::analysis not super::ir::named

@fitzgen
Copy link
Member

fitzgen commented Jul 7, 2017

error: missing documentation for a module
 --> src/ir/mod.rs:7:1
  |
7 | pub mod analysis;
  | ^^^^^^^^^^^^^^^^^
  |
note: lint level defined here
 --> src/lib.rs:8:9
  |
8 | #![deny(missing_docs)]
  |         ^^^^^^^^^^^^
error: aborting due to previous error
error: Could not compile `bindgen`.

Also needs a //! comment at the top of the new analysis.rs file. Perhaps something like:

//! Fix-point analyses on the IR using the monotone framework.

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.

Thanks :)

@fitzgen
Copy link
Member

fitzgen commented Jul 7, 2017

@bors-servo r+

@bors-servo
Copy link

📌 Commit d3bb41a has been approved by fitzgen

@bors-servo
Copy link

⌛ Testing commit d3bb41a with merge c93b1de...

bors-servo pushed a commit that referenced this pull request Jul 7, 2017
Separate the analysis framework in a different file.

r? @fitzgen #765
@bors-servo
Copy link

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

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