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

cleanup: remove field is_import from def::Export #47100

Closed
jseyfried opened this issue Jan 1, 2018 · 3 comments
Closed

cleanup: remove field is_import from def::Export #47100

jseyfried opened this issue Jan 1, 2018 · 3 comments
Labels
A-HIR Area: The high-level intermediate representation (HIR) C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jseyfried
Copy link
Contributor

Remove field is_import from rustc::def::Export.
The field is only used is rustdoc and can be refactored away.

@pietroalbini pietroalbini added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-dev-tools-rustdoc A-HIR Area: The high-level intermediate representation (HIR) labels Jan 30, 2018
@Mark-Simulacrum
Copy link
Member

@eddyb or @jseyfried This seems like a good candidate for an easy/mentor issue, perhaps one of you could provide what !item.is_import could be replaced with in rustdoc? The comment on the PR suggests tcx.def_key(export.def.def_id()).parent, but I'm not personally sure what one would do with that to determine whether the item is in an import statement or not.

@eddyb
Copy link
Member

eddyb commented Jun 2, 2018

@Mark-Simulacrum You'd compare it with the DefId of the module you found the Export in.
That is, is_import signifies a re-export, where the definition is somewhere else than the module you're currently listing the Exports of.

@eddyb
Copy link
Member

eddyb commented Jun 2, 2018

Depending on what is desired, the name of the original item might also be compared (because a reexport from the same module, aka pub use self::foo as bar;, is also possible).

bors added a commit that referenced this issue Jun 2, 2018
Remove rustdoc-specific is_import field from HIR

Fixes #47100.

I believe that there is no need to check for the name being the same, as this
part of rustdoc seems to be strictly interested in exploring "public modules."
Re-exports from the same module cannot visit another module; and, re-exports
cannot export items with a greater visibility than that item declares.
Therefore, I think this code is either sufficient, or in fact does more than
is necessary, depending on whether rustdoc cares about the re-export itself.

r? @eddyb
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 4, 2018
…=eddyb

Remove rustdoc-specific is_import field from HIR

Fixes rust-lang#47100.

I believe that there is no need to check for the name being the same, as this
part of rustdoc seems to be strictly interested in exploring "public modules."
Re-exports from the same module cannot visit another module; and, re-exports
cannot export items with a greater visibility than that item declares.
Therefore, I think this code is either sufficient, or in fact does more than
is necessary, depending on whether rustdoc cares about the re-export itself.

r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-HIR Area: The high-level intermediate representation (HIR) C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants