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

save_analysis: Dump data only if get_path_data doesn't fail to resolve a path. #37144

Merged
merged 1 commit into from
Oct 27, 2016

Conversation

eulerdisk
Copy link
Contributor

Solves #37126

Dump data only if get_path_data doesn't fail to resolve a path.
get_path_data returns None when it have to deals with Def::Err, which is used as placeholder for a failed resolution.

Tell me if this is good enough, maybe I have to add some tests ?

r? @nrc

@nrc
Copy link
Member

nrc commented Oct 13, 2016

maybe I have to add some tests ?

Sadly, its a pain to test save-analysis in any meaningful way. If you wanted a follow-up thing to work on, you could add a version of the run-make smoke test that has an error in it so that we're at least testing the error case to some extent.

scope: vrd.scope,
qualname: String::new()
}.lower(self.tcx));
if let Some(path_data) = path_data {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than an if let here, it is probably better to do an early return - I don't think we want to hit the following match statement and it means there is less rightward drift.

@nrc
Copy link
Member

nrc commented Oct 25, 2016

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Oct 25, 2016

📌 Commit 88b031e has been approved by nrc

Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 26, 2016
save_analysis: Dump data only if get_path_data doesn't fail to resolve a path.

Solves rust-lang#37126

Dump data only if `get_path_data` doesn't fail to resolve a path.
`get_path_data` returns `None` when it have to deals with `Def::Err`, which is used as placeholder for a failed resolution.

Tell me if this is good enough, maybe I have to add some tests ?

r? @nrc
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 26, 2016
save_analysis: Dump data only if get_path_data doesn't fail to resolve a path.

Solves rust-lang#37126

Dump data only if `get_path_data` doesn't fail to resolve a path.
`get_path_data` returns `None` when it have to deals with `Def::Err`, which is used as placeholder for a failed resolution.

Tell me if this is good enough, maybe I have to add some tests ?

r? @nrc
bors added a commit that referenced this pull request Oct 26, 2016
Rollup of 7 pull requests

- Successful merges: #36206, #37144, #37391, #37394, #37396, #37398, #37414
- Failed merges:
@bors bors merged commit 88b031e into rust-lang:master Oct 27, 2016
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.

3 participants