Module experiments: Add one more prelude layer for extern crate names passed with `--extern` #49789

petrochenkov · 2018-04-08T16:06:19Z

Implements one item from https://internals.rust-lang.org/t/the-great-module-adventure-continues/6678/183

When some name is looked up in lexical scope (name, i.e. not module-relative scope some_mod::name or ::name), it's searched roughly in the next order:

local variables
items in unnamed blocks
items in the current module
✨ NEW! ✨ crate names passed with --extern ("extern prelude")
standard library prelude (Vec, drop)
language prelude (built-in types like u8, str, etc)

The last two layers contain a limited set of names controlled by us and not arbitrary user-defined names like upper layers. We want to be able to add new names into these two layers without breaking user code, so "extern prelude" names have higher priority than std prelude and built-in types.
This is a one-time breaking change, that's why it would be nice to run this through crater.
Practical impact is expected to be minimal though due to stylistic reasons (there are not many Uppercase crates) and due to the way how primitive types are resolved (#32131).

rust-highfive · 2018-04-08T16:06:22Z

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

petrochenkov · 2018-04-08T16:06:55Z

r? @nikomatsakis
cc @aturon

petrochenkov · 2018-04-08T16:07:16Z

Needs a check-only crater run.

Mark-Simulacrum · 2018-04-08T22:00:34Z

@bors try

bors · 2018-04-08T22:00:45Z

⌛ Trying commit 554c875651ebc3556fa1864a414b26a28affcf05 with merge 2b913419c4e8baefbf1d4ed00b2623cdccb803fd...

bors · 2018-04-08T23:46:32Z

☀️ Test successful - status-travis
State: approved= try=True

nikomatsakis · 2018-04-10T16:23:53Z

@petrochenkov nice!

nikomatsakis · 2018-04-10T16:25:45Z

We want to be able to add new names into these two layers without breaking user code, so "extern prelude" names have higher priority than std prelude and built-in types.

Ah, I was going to ask why you did it that way -- but of course, that makes sense.

nikomatsakis · 2018-04-10T16:25:55Z

r=me pending crater results

cramertj · 2018-04-18T18:11:20Z

@Mark-Simulacrum Are the crater results for this available yet?

Mark-Simulacrum · 2018-04-18T23:44:44Z

Unfortunately, they aren't; I'd anticipate about 4 more days. This wasn't marked as check-only in the crater spreadsheet so it was started as a normal run :/

Manishearth · 2018-04-20T00:47:38Z

src/librustc_resolve/lib.rs

+                                "access to extern crates through prelude is experimental").emit();
+                }
+
+                let crate_id = self.crate_loader.resolve_crate_from_path(ident.name, ident.span);


Can we use process_path_extern here?

(Just for uniformity with the extern_absolute_paths code above)

nikomatsakis · 2018-04-23T20:38:41Z

src/test/run-make-fulldeps/extern-prelude/Makefile

+	$(RUSTC) shadow-prelude.rs --extern Vec=$(TMPDIR)/libep_vec.rlib
+	$(RUSTC) feature-gate.rs --extern ep_lib=$(TMPDIR)/libep_lib.rlib 2>&1 | $(CGREP) "access to extern crates through prelude is experimental"
+	$(RUSTC) relative-only.rs --extern ep_lib=$(TMPDIR)/libep_lib.rlib 2>&1 | $(CGREP) "unresolved import"
+	$(RUSTC) relative-only.rs --extern ep_lib=$(TMPDIR)/libep_lib.rlib 2>&1 | $(CGREP) "failed to resolve"


why is this using run-make and not auxiliary? ah, well, I guess we'd have to modify the auxiliary mechanism to supply --extern? (doesn't seem that hard... but maybe we leave it for another PR)

nikomatsakis · 2018-04-23T20:39:40Z

@Mark-Simulacrum ping re: crater

Mark-Simulacrum · 2018-04-24T01:06:17Z

Crater is done, sorry for the delay: http://cargobomb-reports.s3.amazonaws.com/pr-49789/index.html.

petrochenkov · 2018-04-24T10:30:42Z

There are a lot of regression, but all regressions I've checked are caused by something that looks like an implementation bug - iovec in libc::iovec is interpreted as a crate iovec.
Why on earth resolve_ident_in_lexical_scope is even called for something that is not in lexical scope?
I need to investigate.

Manishearth · 2018-04-26T00:15:50Z

I think the issue is that the unused_qualifications lint is barging in and trying to resolve this against the root, hitting the feature gate. We should probably silently return None when stuff is being resolved speculatively.

Manishearth · 2018-04-26T00:26:58Z

Actually, can we just not throw feature errors in this case? Ideally the module system would pervasively track when it's resolving user-defined paths vs speculative resolution (in case we wish to lint or throw better messages). But this ideally will be done by creating a better abstraction for paths over &[Ident]. Due to #50100 we already track this in part with the node_id argument to resolve_path but that isn't perfect.

I'm going to remove the feature error here ; we have plenty of speculative resolution happening in resolve. A longer term solution would be to implement said abstraction (something that contains an &[Ident] and Option<NodeId>) -- this can be mentored.

Manishearth · 2018-04-26T00:30:16Z

Rebased and pushed an update.

bors · 2018-05-01T16:12:06Z

💔 Test failed - status-travis

petrochenkov · 2018-05-01T16:19:06Z

Looks like a spurious network failure
@bors retry

bors · 2018-05-01T16:58:33Z

⌛ Testing commit c1492fe with merge 4d7bbdd...

Module experiments: Add one more prelude layer for extern crate names passed with `--extern` Implements one item from https://internals.rust-lang.org/t/the-great-module-adventure-continues/6678/183 When some name is looked up in lexical scope (`name`, i.e. not module-relative scope `some_mod::name` or `::name`), it's searched roughly in the next order: - local variables - items in unnamed blocks - items in the current module - ✨ NEW! ✨ crate names passed with `--extern` ("extern prelude") - standard library prelude (`Vec`, `drop`) - language prelude (built-in types like `u8`, `str`, etc) The last two layers contain a limited set of names controlled by us and not arbitrary user-defined names like upper layers. We want to be able to add new names into these two layers without breaking user code, so "extern prelude" names have higher priority than std prelude and built-in types. This is a one-time breaking change, that's why it would be nice to run this through crater. Practical impact is expected to be minimal though due to stylistic reasons (there are not many `Uppercase` crates) and due to the way how primitive types are resolved (#32131).

bors · 2018-05-01T19:14:30Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 4d7bbdd to master...

aturon · 2018-05-01T19:38:26Z

Woohoo! Thanks @petrochenkov and @Manishearth for seeing this through!

nikomatsakis · 2018-05-03T10:44:15Z

Is this in the latest nightly? If so, it doesn't seem to be working for me (this commit doesn't build):

nikomatsakis/polonius@fd3e48f

I see:

-*- mode: compilation; default-directory: "~/versioned/borrow-check/" -*-
Compilation started at Thu May  3 06:43:32

cargo test
   Compiling assert_cli v0.5.4
   Compiling borrow-check v0.1.0 (file:///home/nmatsakis/versioned/borrow-check)
error[E0432]: unresolved import `abomonation_derive`
 --> src/facts.rs:1:5
  |
1 | use abomonation_derive::Abomonation;
  |     ^^^^^^^^^^^^^^^^^^ Maybe a missing `extern crate abomonation_derive;`?

error[E0432]: unresolved import `fxhash`
  --> src/output/mod.rs:14:5
   |
14 | use fxhash::FxHashMap;
   |     ^^^^^^ Maybe a missing `extern crate fxhash;`?

error[E0432]: unresolved import `fxhash`
 --> src/output/dump.rs:3:5
  |
3 | use fxhash::FxHashMap;
  |     ^^^^^^ Maybe a missing `extern crate fxhash;`?

error[E0433]: failed to resolve. Maybe a missing `extern crate differential_dataflow;`?
  --> src/output/timely.rs:15:5
   |
15 | use differential_dataflow::collection::Collection;
   |     ^^^^^^^^^^^^^^^^^^^^^ Maybe a missing `extern crate differential_dataflow;`?

...

petrochenkov · 2018-05-03T10:49:57Z

@nikomatsakis
Prelude doesn't affect resolution for absolute paths (including imports).
Resolve-the-first-segment-as-extern-crate for absolute paths is enabled by feature(extern_absolute_paths).

nikomatsakis · 2018-05-03T12:17:23Z

Hmm, it seems that the commits appear in the git log, so maybe I'm doing something wrong?

nikomatsakis · 2018-05-03T12:17:34Z

Ah, I see, duh. Thanks!

@nikomatsakis

idiom lints for removing `extern crate` Based off of #49789 This contains two lints: - One that suggests replacing pub extern crates with pub use, and removing non-pub extern crates entirely - One that suggests rewriting `use modulename::...::cratename::foo` as `cratename::foo` The latter is a bit tricky to emit suggestions for; for one this involves splicing spans (never a good idea), and it also won't be able to correctly handle `use module::{cratename, foo}` and use-trees. I'm not sure how to proceed here. Currently it doesn't suggest anything at all. Perhaps we can go the other way and suggest removal of all extern crates _except_ those used through modules (stash node ids somewhere) and suggest replacing those with `<visibility> use`? r? @nikomatsakis fixes #48719

@nikomatsakis

idiom lints for removing `extern crate` Based off of #49789 This contains two lints: - One that suggests replacing pub extern crates with pub use, and removing non-pub extern crates entirely - One that suggests rewriting `use modulename::...::cratename::foo` as `cratename::foo` The latter is a bit tricky to emit suggestions for; for one this involves splicing spans (never a good idea), and it also won't be able to correctly handle `use module::{cratename, foo}` and use-trees. I'm not sure how to proceed here. Currently it doesn't suggest anything at all. Perhaps we can go the other way and suggest removal of all extern crates _except_ those used through modules (stash node ids somewhere) and suggest replacing those with `<visibility> use`? r? @nikomatsakis fixes #48719

rust-highfive assigned cramertj Apr 8, 2018

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

rust-highfive assigned nikomatsakis and unassigned cramertj Apr 8, 2018

petrochenkov added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Apr 8, 2018

pietroalbini removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 12, 2018

Manishearth reviewed Apr 20, 2018

View reviewed changes

Manishearth mentioned this pull request Apr 22, 2018

Allow crate:: in local paths #50131

Merged

nikomatsakis approved these changes Apr 23, 2018

View reviewed changes

TimNN added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Apr 24, 2018

Manishearth force-pushed the prelext branch from 554c875 to bda1ce2 Compare April 26, 2018 00:30

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 May 1, 2018

This comment has been minimized.

Sign in to view

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 May 1, 2018

bors merged commit c1492fe into rust-lang:master May 1, 2018

This was referenced May 1, 2018

Implement RFC 2056 trivial constraints in where clauses #48557

Merged

implement #[panic_implementation] #50338

Merged

Macros: Add a 'literal' fragment specifier #49835

Merged

Implement tool_attributes feature (RFC 2103) #50030

Merged

petrochenkov mentioned this pull request Jun 11, 2018

Fix extern prelude failure in rustdoc #50617

Merged

petrochenkov mentioned this pull request Jul 18, 2018

Macro path resolution ignores preludes #52512

Closed

This was referenced Oct 1, 2018

Updated RELEASES.md for 1.30.0 #54300

Merged

resolve: Scale back hard-coded extern prelude additions on 2015 edition #54671

Merged

petrochenkov deleted the prelext branch June 5, 2019 16:05

petrochenkov mentioned this pull request Jan 18, 2021

Start documenting name resolution. rust-lang/reference#937

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Module experiments: Add one more prelude layer for extern crate names passed with `--extern` #49789

Module experiments: Add one more prelude layer for extern crate names passed with `--extern` #49789

petrochenkov commented Apr 8, 2018 •

edited

Loading

rust-highfive commented Apr 8, 2018

petrochenkov commented Apr 8, 2018 •

edited

Loading

petrochenkov commented Apr 8, 2018

Mark-Simulacrum commented Apr 8, 2018

bors commented Apr 8, 2018

bors commented Apr 8, 2018

nikomatsakis commented Apr 10, 2018

nikomatsakis commented Apr 10, 2018

nikomatsakis commented Apr 10, 2018

cramertj commented Apr 18, 2018

Mark-Simulacrum commented Apr 18, 2018

Manishearth Apr 20, 2018 •

edited

Loading

nikomatsakis Apr 23, 2018

nikomatsakis commented Apr 23, 2018

Mark-Simulacrum commented Apr 24, 2018

petrochenkov commented Apr 24, 2018

Manishearth commented Apr 26, 2018

Manishearth commented Apr 26, 2018

Manishearth commented Apr 26, 2018

bors commented May 1, 2018

This comment has been minimized.

This comment has been minimized.

petrochenkov commented May 1, 2018

bors commented May 1, 2018

bors commented May 1, 2018

aturon commented May 1, 2018

nikomatsakis commented May 3, 2018

petrochenkov commented May 3, 2018

nikomatsakis commented May 3, 2018

nikomatsakis commented May 3, 2018

Module experiments: Add one more prelude layer for extern crate names passed with --extern #49789

Module experiments: Add one more prelude layer for extern crate names passed with --extern #49789

Conversation

petrochenkov commented Apr 8, 2018 • edited Loading

rust-highfive commented Apr 8, 2018

petrochenkov commented Apr 8, 2018 • edited Loading

petrochenkov commented Apr 8, 2018

Mark-Simulacrum commented Apr 8, 2018

bors commented Apr 8, 2018

bors commented Apr 8, 2018

nikomatsakis commented Apr 10, 2018

nikomatsakis commented Apr 10, 2018

nikomatsakis commented Apr 10, 2018

cramertj commented Apr 18, 2018

Mark-Simulacrum commented Apr 18, 2018

Manishearth Apr 20, 2018 • edited Loading

Choose a reason for hiding this comment

nikomatsakis Apr 23, 2018

Choose a reason for hiding this comment

nikomatsakis commented Apr 23, 2018

Mark-Simulacrum commented Apr 24, 2018

petrochenkov commented Apr 24, 2018

Manishearth commented Apr 26, 2018

Manishearth commented Apr 26, 2018

Manishearth commented Apr 26, 2018

bors commented May 1, 2018

This comment has been minimized.

This comment has been minimized.

petrochenkov commented May 1, 2018

bors commented May 1, 2018

bors commented May 1, 2018

aturon commented May 1, 2018

nikomatsakis commented May 3, 2018

petrochenkov commented May 3, 2018

nikomatsakis commented May 3, 2018

nikomatsakis commented May 3, 2018

Module experiments: Add one more prelude layer for extern crate names passed with `--extern` #49789

Module experiments: Add one more prelude layer for extern crate names passed with `--extern` #49789

petrochenkov commented Apr 8, 2018 •

edited

Loading

petrochenkov commented Apr 8, 2018 •

edited

Loading

Manishearth Apr 20, 2018 •

edited

Loading