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

rustc_resolve: overhaul #![feature(uniform_paths)] error reporting. #53427

Merged
merged 1 commit into from
Aug 17, 2018

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Aug 16, 2018

Fixes #53408 by only considering external crates to conflict within their (type/module) namespace, not with the value or macro namespaces, and also by adding a special-cased error for redundant use crate_name; imports (without actually allowing them).
Also, all canaries for a given import are grouped into one diagnostic per namespace, in order to make block-scoped ambiguities clearer.
See changed/added tests for more details.

r? @petrochenkov cc @aturon @joshtriplett

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 16, 2018
@eddyb
Copy link
Member Author

eddyb commented Aug 16, 2018

@bors p=10 (somewhat blocking Edition Preview 2 testing)

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@varkor
Copy link
Member

varkor commented Aug 16, 2018

These error messages are much better and the strategy looks good based on the evaluation in #53408. It's high-priority, so I'm going to r+ now to aim for the next Nightly.

@bors r+

@bors
Copy link
Contributor

bors commented Aug 16, 2018

📌 Commit bcf864db8993218f9e8a2affe0e83c5a9810d402 has been approved by varkor

@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 Aug 16, 2018
@bors
Copy link
Contributor

bors commented Aug 16, 2018

⌛ Testing commit bcf864db8993218f9e8a2affe0e83c5a9810d402 with merge 8d83a20b14dec23b2c82c9e2fa655ead377e8d04...

@bors
Copy link
Contributor

bors commented Aug 16, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 16, 2018
@rust-highfive

This comment has been minimized.

@eddyb
Copy link
Member Author

eddyb commented Aug 16, 2018

@bors retry

@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 Aug 16, 2018
@bors
Copy link
Contributor

bors commented Aug 16, 2018

⌛ Testing commit bcf864db8993218f9e8a2affe0e83c5a9810d402 with merge 60de86d7c8dece92a7c289a0924574b2cdef4c3e...

@bors
Copy link
Contributor

bors commented Aug 16, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 16, 2018
@eddyb
Copy link
Member Author

eddyb commented Aug 16, 2018

@bors r=varkor (sadly I don't think this will make the nightly, oops)

@bors
Copy link
Contributor

bors commented Aug 16, 2018

📌 Commit 7a87e30 has been approved by varkor

@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 Aug 16, 2018
@bors
Copy link
Contributor

bors commented Aug 17, 2018

⌛ Testing commit 7a87e30 with merge f34933b...

bors added a commit that referenced this pull request Aug 17, 2018
rustc_resolve: overhaul `#![feature(uniform_paths)]` error reporting.

Fixes #53408 by only considering external crates to conflict within their (type/module) namespace, *not* with the value or macro namespaces, and also by adding a special-cased error for redundant `use crate_name;` imports (without actually allowing them).
Also, all canaries for a given import are grouped into one diagnostic per namespace, in order to make block-scoped ambiguities clearer.
See changed/added tests for more details.

r? @petrochenkov cc @aturon @joshtriplett
@bors
Copy link
Contributor

bors commented Aug 17, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: varkor
Pushing f34933b to master...

@bors bors merged commit 7a87e30 into rust-lang:master Aug 17, 2018
@eddyb eddyb deleted the uniform-paths-diagnostics branch August 17, 2018 02:48
| refers to external crate `::std`
| defines `self::std`, shadowing itself
|
= help: remove or write `::std` explicitly instead
Copy link
Member

@joshtriplett joshtriplett Aug 18, 2018

Choose a reason for hiding this comment

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

Wouldn't use ::somecrate; be redundant as well, because extern_prelude makes somecrate already available?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's redundant for direct uses of the imported name but maybe you actually want use ::std as _; or something.

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.

6 participants