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

Implement RFC 1560 behind #![feature(item_like_imports)] #35894

Merged
merged 18 commits into from
Sep 2, 2016

Conversation

jseyfried
Copy link
Contributor

This implements rust-lang/rfcs#1560 (cc #35120) behind the item_like_imports feature gate.

The RFC text describes the changes to name resolution enabled by #![feature(item_like_imports) in detail. To summarize,

  • Items and named imports shadow glob imports.
  • Multiple globs can import the same name if the name is unused or the imports are shadowed.
  • Multiple globs can import the same name if the imports are of the same item (following re-exports).
    • The visibility of such a name is the maximum visibility of the imports.
    • Equivalently, adding a glob import will never reduce the visibility of a name, nor will removing one increase it.
  • Non-prelude private imports can be used wherever we currently allow private items to be used.
    • Prelude-imported names are unaffected, i.e. they continue to be usable only in lexical scopes.
  • Globs import all visible names, not just public names.
    • Equivalently, glob importing from an ancestor module imports all of the ancestor's names, and glob importing from other modules is unchanged.

r? @nrc

@jseyfried jseyfried force-pushed the new_import_semantics branch 2 times, most recently from 540a351 to a1b7904 Compare August 22, 2016 09:24
@jseyfried
Copy link
Contributor Author

cc @petrochenkov

@bors
Copy link
Contributor

bors commented Aug 23, 2016

☔ The latest upstream changes (presumably #35908) made this pull request unmergeable. Please resolve the merge conflicts.

@@ -390,7 +388,7 @@ impl<'b> Resolver<'b> {
debug!("(building reduced graph for external crate) building module {} {:?}",
name, vis);
let parent_link = ModuleParentLink(parent, name);
let module = self.new_module(parent_link, Some(def), true);
let module = self.new_module(parent_link, Some(def), DUMMY_NODE_ID);
Copy link
Member

Choose a reason for hiding this comment

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

what does using DUMMY_NODE_ID mean? Seems like it is just ignored when searching, so it would probably be better to use an Option and None here.

Copy link
Contributor Author

@jseyfried jseyfried Aug 24, 2016

Choose a reason for hiding this comment

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

DUMMY_NODE_ID means that the module is from an extern crate, so its nearest normal ancestor doesn't have a node id. normal_ancestor_id is never used on modules from extern crates, so the the choice of node id here isn't particularly relevant.

Copy link
Member

Choose a reason for hiding this comment

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

Then I think it would be better to use Option here, rather than DUMMY_NODE_ID, this seems like the moral equivalent of using null. Put another way, it would be nice to enforce "normal_ancestor_id is never used on modules from extern crates" statically.

Copy link
Contributor Author

@jseyfried jseyfried Aug 28, 2016

Choose a reason for hiding this comment

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

Done.

@nrc
Copy link
Member

nrc commented Aug 28, 2016

r=me with the last comments addressed.

@bors
Copy link
Contributor

bors commented Aug 28, 2016

☔ The latest upstream changes (presumably #35984) made this pull request unmergeable. Please resolve the merge conflicts.

@jseyfried
Copy link
Contributor Author

@bors r=nrc

@bors
Copy link
Contributor

bors commented Aug 29, 2016

📌 Commit 58616c8 has been approved by nrc

@bors
Copy link
Contributor

bors commented Aug 29, 2016

⌛ Testing commit 58616c8 with merge 614a792...

@bors
Copy link
Contributor

bors commented Aug 29, 2016

💔 Test failed - auto-win-gnu-32-opt-rustbuild

@jseyfried
Copy link
Contributor Author

@bors retry

@bors
Copy link
Contributor

bors commented Aug 29, 2016

⌛ Testing commit 58616c8 with merge 7f96d62...

@bors
Copy link
Contributor

bors commented Aug 29, 2016

💔 Test failed - auto-mac-64-opt-rustbuild

@arielb1
Copy link
Contributor

arielb1 commented Aug 29, 2016

@bors retry

@jseyfried
Copy link
Contributor Author

@bors r=nrc

@bors
Copy link
Contributor

bors commented Sep 1, 2016

📌 Commit bba8b09 has been approved by nrc

@jseyfried
Copy link
Contributor Author

@bors r=nrc

@bors
Copy link
Contributor

bors commented Sep 2, 2016

📌 Commit 90ce504 has been approved by nrc

@bors
Copy link
Contributor

bors commented Sep 2, 2016

⌛ Testing commit 90ce504 with merge 8aeb15a...

bors added a commit that referenced this pull request Sep 2, 2016
Implement RFC 1560 behind `#![feature(item_like_imports)]`

This implements rust-lang/rfcs#1560 (cc #35120) behind the `item_like_imports` feature gate.

The [RFC text](https:/rust-lang/rfcs/blob/master/text/1560-name-resolution.md#changes-to-name-resolution-rules) describes the changes to name resolution enabled by `#![feature(item_like_imports)` in detail. To summarize,
 - Items and named imports shadow glob imports.
 - Multiple globs can import the same name if the name is unused or the imports are shadowed.
 - Multiple globs can import the same name if the imports are of the same item (following re-exports).
  - The visibility of such a name is the maximum visibility of the imports.
  - Equivalently, adding a glob import will never reduce the visibility of a name, nor will removing one increase it.
 - Non-prelude private imports can be used wherever we currently allow private items to be used.
  - Prelude-imported names are unaffected, i.e. they continue to be usable only in lexical scopes.
 - Globs import all visible names, not just public names.
  - Equivalently, glob importing from an ancestor module imports all of the ancestor's names, and glob importing from other modules is unchanged.

r? @nrc
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