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

Nested use trees are not reachable #46823

Closed
euclio opened this issue Dec 18, 2017 · 4 comments · Fixed by #47081
Closed

Nested use trees are not reachable #46823

euclio opened this issue Dec 18, 2017 · 4 comments · Fixed by #47081

Comments

@euclio
Copy link
Contributor

euclio commented Dec 18, 2017

This causes an issue where the imports are not dumped in the save analysis data if it is configured to contain only reachable defs.

Steps to reproduce:

lib.rs:

#![crate_type = "lib"]

pub use private::*;

mod private {
    pub struct Struct1;
    pub struct Struct2;
}

pub use private2::{Struct3, Struct4};

mod private2 {
    pub struct Struct3;
    pub struct Struct4;
}
$ RUST_SAVE_ANALYSIS_CONFIG='{ "reachable_only": true, "full_docs": true, "pub_only": false, "distro_crate": false, "signatures": false, "borrow_data": false }' rustc -Zsave-analysis lib.rs

Notice that pub use private::*; will appear in the save analysis data, but neither import of Struct3 or Struct4.

The relevant reachability testing is here.

It seems to me that the privacy_access_levels query needs to be updated to consider the nested use tree as reachable?

cc @pietroalbini

@pietroalbini
Copy link
Member

I'll look into this in a few hours.

@pietroalbini
Copy link
Member

Sorry, I'm a bit busy these days, I'll try looking at this in the next few days.

@euclio
Copy link
Contributor Author

euclio commented Dec 27, 2017

No problem. I need to take a closer look because using the analysis data seemed to work as expected.

@pietroalbini
Copy link
Member

Found the bug and fixed it locally. Tonight I'll add some tests and create a PR.

kennytm added a commit to kennytm/rust that referenced this issue Jan 12, 2018
…r=nrc

Fix nested imports not included in the save_analysis output

This PR fixes rust-lang#46823.

The bug was caused by the old access level checking code, which checked against the root UseTree even for nested trees. The problem with that is, for nested trees the root is lowered as an empty `ListStem`, which is not reachable by definition. The new code computes the access level with each tree's own ID, and with the root tree's visibility.

I tested this manually and it works, but I'm not really satisfied with that. I looked at the existing tests though, and no one checked for the save_analysis output as far as I can see. How should I proceed with that? I think having a test about this would be really nice.
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 a pull request may close this issue.

2 participants