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

Resolve absolute paths as extern under a feature flag #46613

Merged
merged 2 commits into from
Dec 13, 2017

Conversation

petrochenkov
Copy link
Contributor

let ImportDirective { ref module_path, span, .. } = *directive;

// Extern crate mode for absolute paths needs some
// special support for single-segment imports.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The part about single-segment imports (use my_crate;/use crate;) works, but it is kinda hacked into.
Solving it properly requires larger refactoring that would also solve a number of pre-existing problems (#29036 and similar).

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 10, 2017
@@ -2959,6 +2960,16 @@ impl<'a> Resolver<'a> {
// `$crate::a::b`
module = Some(self.resolve_crate_root(ident.node.ctxt));
continue
} else if i == 1 && self.session.features.borrow().extern_absolute_paths &&
path[0].node.name == keywords::CrateRoot.name() &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: isn't there some helper function we can use here for this test against keywords::CrateRoot.name()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no such existing function.
(Also there are too many "crate roots" with different properties now, it's better to be explicit.)

// Extern crate mode for absolute paths needs some
// special support for single-segment imports.
let extern_absolute_paths = self.session.features.borrow().extern_absolute_paths;
if module_path.len() == 1 && module_path[0].node.name == keywords::CrateRoot.name() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: as above, helper function?

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 11, 2017

📌 Commit 675581a has been approved by nikomatsakis

@kennytm kennytm 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 Dec 11, 2017
@bors
Copy link
Contributor

bors commented Dec 12, 2017

⌛ Testing commit 675581a290571ea38141b1f6e202bf66d444122a with merge 4ae1d40e143c9a1a3722d1b921255c4b4e2bdc74...

@bors
Copy link
Contributor

bors commented Dec 12, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Dec 12, 2017

The new gate test failed on i686-unknown-linux-musl.

`i686-unknown-linux-musl`

[00:39:46] ---- [ui] ui/feature-gate-extern_absolute_paths.rs stdout ----
[00:39:46] 	
[00:39:46] error: ui test compiled successfully!
[00:39:46] status: exit code: 0
[00:39:46] thread 'main' panicked at 'Some tests failed', /checkout/src/tools/compiletest/src/main.rs:475:21
[00:39:46] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/feature-gate-extern_absolute_paths.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=i686-unknown-linux-musl" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/feature-gate-extern_absolute_paths.stage2-i686-unknown-linux-musl" "-Crpath" "-O" "-Lnative=/checkout/obj/build/i686-unknown-linux-musl/native/rust-test-helpers" "-Clinker=/musl-i686/bin/musl-gcc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/feature-gate-extern_absolute_paths.stage2-i686-unknown-linux-musl.aux" "-A" "unused"
[00:39:46] stdout:
[00:39:46] ------------------------------------------
[00:39:46] 
[00:39:46] ------------------------------------------
[00:39:46] stderr:
[00:39:46] ------------------------------------------
[00:39:46] 
[00:39:46] ------------------------------------------
[00:39:46] 
[00:39:46] thread '[ui] ui/feature-gate-extern_absolute_paths.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2774:8
[00:39:46] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:39:46] 
[00:39:46] 
[00:39:46] failures:
[00:39:46]     [ui] ui/feature-gate-extern_absolute_paths.rs

@kennytm kennytm 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 12, 2017
@@ -430,6 +430,9 @@ declare_features! (

// generic associated types (RFC 1598)
(active, generic_associated_types, "1.23.0", Some(44265)),

// Resolve absolute paths as paths from other crates
(active, extern_absolute_paths, "1.23.0", Some(44660)),
Copy link
Member

Choose a reason for hiding this comment

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

This should be 1.24.0?

// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn main() {}
Copy link
Member

Choose a reason for hiding this comment

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

Something is missing?

@petrochenkov
Copy link
Contributor Author

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 12, 2017

📌 Commit b07e26e has been approved by nikomatsakis

@kennytm kennytm 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 12, 2017
@bors
Copy link
Contributor

bors commented Dec 13, 2017

⌛ Testing commit b07e26e with merge 691f022...

bors added a commit that referenced this pull request Dec 13, 2017
Resolve absolute paths as extern under a feature flag

cc #44660
r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Dec 13, 2017

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

@bors bors merged commit b07e26e into rust-lang:master Dec 13, 2017
bors added a commit that referenced this pull request Jan 7, 2018
Support `extern` in paths

Implement the primary alternative to #46613 + #45771, achieving the same effect without requiring changes to other imports.
Both need to be experimentally evaluated before making further progress.

The PR also adds docs for all these related features into the unstable book.

cc #44660
r? @nikomatsakis
@petrochenkov petrochenkov deleted the absext branch June 5, 2019 15:58
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.

4 participants