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

Add crate:: to trait suggestions in Rust 2018. #54603

Merged
merged 3 commits into from
Oct 3, 2018

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Sep 26, 2018

Fixes #54559.

In the 2018 edition, when suggesting traits to import that implement a
given method that is being invoked, suggestions will now include the
crate:: prefix if the suggested trait is local to the current crate.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 26, 2018
@rust-highfive

This comment has been minimized.

src/librustc/ty/item_path.rs Show resolved Hide resolved
// In particular, don't recurse to print the crate root if we
// just printed `std`. In doing this, we are able to add
// `crate::` to trait import suggestions.
DefPathData::CrateRoot if data.as_interned_str() == "std" => {},
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not happy about special-casing std, but I guess it .. maybe makes sense?

In particular, is this only necessary because we stick the extern crate std in the prelude?

More generally, we could convert this into a check to see if this crate is in the list of "prelude crates".

Copy link
Contributor

Choose a reason for hiding this comment

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

@eddyb points to this code, which constructs the set of prelude crates:

let mut extern_prelude: FxHashSet<Name> =
session.opts.externs.iter().map(|kv| Symbol::intern(kv.0)).collect();
// HACK(eddyb) this ignore the `no_{core,std}` attributes.
// FIXME(eddyb) warn (elsewhere) if core/std is used with `no_{core,std}`.
// if !attr::contains_name(&krate.attrs, "no_core") {
// if !attr::contains_name(&krate.attrs, "no_std") {
extern_prelude.insert(Symbol::intern("core"));
extern_prelude.insert(Symbol::intern("std"));
extern_prelude.insert(Symbol::intern("meta"));

But we'd have to store it somewhere more accessible, I imagine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed this with the most recent commit.

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

src/librustc/session/mod.rs Outdated Show resolved Hide resolved
src/librustc/ty/item_path.rs Show resolved Hide resolved
@@ -981,6 +980,23 @@ impl Session {
pub fn edition(&self) -> Edition {
self.opts.edition
}

/// Get set of prelude crate names.
pub fn prelude_crate_names(&self) -> FxHashSet<Name> {
Copy link
Member

Choose a reason for hiding this comment

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

Should probably use the approach from #54650 - especially as that is r+'d.

Copy link
Member

Choose a reason for hiding this comment

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

(feel free to rebase on top of that PR)

Copy link
Member Author

Choose a reason for hiding this comment

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

Rebased.

@davidtwco davidtwco changed the title Add crate:: to trait suggestions in Rust 2018. WIP: Add crate:: to trait suggestions in Rust 2018. Sep 29, 2018
@davidtwco
Copy link
Member Author

I've rebased this atop @eddyb's #54650 and marked it as WIP so that it isn't merged until after that.

In the 2018 edition, when suggesting traits to import that implement a
given method that is being invoked, suggestions will now include the
`crate::` prefix if the suggested trait is local to the current crate.
Avoid hardcoding and special-casing the `std` crate name in the item
path logic by moving the prelude crate name logic into the `Session`
type so it can be reused in the item path logic and resolve module.
This commit takes a different approach to add the `crate::` prefix to
item paths than previous commits. Previously, recursion was stopped
after a prelude crate name was pushed to the path. It is theorized that
this was the cause of the linking issues since the same path logic is
used for symbol names and that not recursing meant that details were
being missed that affect symbol names. As of this commit, instead of
ceasing recursion, a flag is passed through to any subsequent recursive
calls so that the same effect can be achieved by checking that flag.
@davidtwco davidtwco changed the title WIP: Add crate:: to trait suggestions in Rust 2018. Add crate:: to trait suggestions in Rust 2018. Oct 1, 2018
@davidtwco
Copy link
Member Author

I've rebased this atop master now that @eddyb's #54650 has landed.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 2, 2018

📌 Commit 02357e4 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 2, 2018
Add `crate::` to trait suggestions in Rust 2018.

Fixes rust-lang#54559.

In the 2018 edition, when suggesting traits to import that implement a
given method that is being invoked, suggestions will now include the
`crate::` prefix if the suggested trait is local to the current crate.

r? @nikomatsakis
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 2, 2018
Add `crate::` to trait suggestions in Rust 2018.

Fixes rust-lang#54559.

In the 2018 edition, when suggesting traits to import that implement a
given method that is being invoked, suggestions will now include the
`crate::` prefix if the suggested trait is local to the current crate.

r? @nikomatsakis
bors added a commit that referenced this pull request Oct 2, 2018
Rollup of 10 pull requests

Successful merges:

 - #54269 (#53840: Consolidate pattern check errors)
 - #54458 (Allow both explicit and elided lifetimes in the same impl header)
 - #54603 (Add `crate::` to trait suggestions in Rust 2018.)
 - #54648 (Update Cargo's submodule)
 - #54680 (make run-pass tests with empty main just compile-pass tests)
 - #54687 (Use impl_header_lifetime_elision in libcore)
 - #54699 (Re-export `getopts` so custom drivers can reference it.)
 - #54702 (do not promote comparing function pointers)
 - #54728 (Renumber `proc_macro` tracking issues)
 - #54745 (make `CStr::from_bytes_with_nul_unchecked()` a const fn)

Failed merges:

r? @ghost
@bors bors merged commit 02357e4 into rust-lang:master Oct 3, 2018
@davidtwco davidtwco deleted the issue-54559 branch October 3, 2018 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

suggesting for traits to import are not prefaced with crate in Rust 2018
5 participants