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

Fix error in Rust 2018 + no_core environment #59462

Merged
merged 2 commits into from
Mar 29, 2019
Merged

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Mar 27, 2019

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2019
@petrochenkov petrochenkov self-assigned this Mar 27, 2019
@taiki-e taiki-e mentioned this pull request Mar 27, 2019
@Centril
Copy link
Contributor

Centril commented Mar 27, 2019

@petrochenkov Is this possibly due to hard-coding paths in HIR lowering? (can we stop doing that and instead introduce lang_items which are at least "principled hacks"? cc @eddyb)

@eddyb
Copy link
Member

eddyb commented Mar 27, 2019

@Centril You'd need to move lang item collection to before HIR lowering. Doable, but AFAIK it currently is still happening after HIR lowering.

src/libsyntax/std_inject.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Mar 27, 2019

@Centril You'd need to move lang item collection to before HIR lowering. Doable, but AFAIK it currently is still happening after HIR lowering.

@eddyb What are the primary reasons why we haven't done so hitherto? perf? "no one has put in the work"? something else? Otherwise... It seems like something we should do then?

@eddyb
Copy link
Member

eddyb commented Mar 27, 2019

It seems like something we should do then?

Yes. I think @durka poked at this a year or two ago, but, yeah, I agree we should try again.

@petrochenkov
Copy link
Contributor

Should built-in macros also search traits like Default/Debug/etc as lang items?
I don't see much difference with the for case, all those items are not tied to the language so much as "real" lang items like e.g. Drop, they only need to be found somehow.

@petrochenkov
Copy link
Contributor

Proc macros face a very similar problem when they need to find items from a certain crate, and we also have extern crate self as name when we are inside that certain crate.

Same approach can be used here, I believe - extern crate self as core + ::core::option/::core::iter/etc.

@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 28, 2019

The issue with this lowering-time desugaring is that the paths it generates not only need to be resolved, but they also should be "presentable" and look differently depending on no_std mode.

::std::option::Option should actually say std rather than core where it would be found as a lang item, that's why we have all this dance with injected_crate_name.

In this sense what this PR does is the correct solution.
I could perhaps be done a bit cleaner though, e.g. with crate_root being a &str rather than Option<&str> and initialized with std_inject::injected_crate_name().unwrap_or("crate").

@petrochenkov petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2019
@petrochenkov petrochenkov changed the title [WIP] Fix error in Rust 2018 + no_core environment Fix error in Rust 2018 + no_core environment Mar 28, 2019
@petrochenkov
Copy link
Contributor

Nah, this is good as is.
crate::foo works on both editions unlike ::core::foo and ::crate doesn't work so PathRoot needs to be removed anyway, so it probably can't be made more uniform.

@bors r+

@bors
Copy link
Contributor

bors commented Mar 28, 2019

📌 Commit 8033aefeef4c8f1625c26c1888d67d5498a79548 has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 28, 2019
src/librustc_resolve/lib.rs Outdated Show resolved Hide resolved
@eddyb
Copy link
Member

eddyb commented Mar 29, 2019

@bors r- (see #59462 (comment))

@bors bors 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 Mar 29, 2019
@petrochenkov
Copy link
Contributor

@bors r+ (#59462 (comment) is addressed)

@bors
Copy link
Contributor

bors commented Mar 29, 2019

📌 Commit 362d243 has been approved by petrochenkov

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 29, 2019
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 29, 2019
Centril added a commit to Centril/rust that referenced this pull request Mar 29, 2019
bors added a commit that referenced this pull request Mar 29, 2019
Rollup of 9 pull requests

Successful merges:

 - #59366 (Update books)
 - #59436 (Update jemalloc-sys to version 0.3.0)
 - #59454 (Update rustfmt to 1.2.0)
 - #59462 (Fix error in Rust 2018 + no_core environment)
 - #59467 (Better diagnostic for binary operation on BoxedValues)
 - #59473 (Do not emit incorrect borrow suggestion involving macros and fix overlapping multiline spans)
 - #59480 (Update stdsimd)
 - #59486 (Visit `ImplItem` in `dead_code` lint)
 - #59510 (Rename `type_parameters` to `generics` and so on)

Failed merges:

 - #59516 (Update cargo)

r? @ghost
bors added a commit that referenced this pull request Mar 29, 2019
Rollup of 9 pull requests

Successful merges:

 - #59366 (Update books)
 - #59436 (Update jemalloc-sys to version 0.3.0)
 - #59454 (Update rustfmt to 1.2.0)
 - #59462 (Fix error in Rust 2018 + no_core environment)
 - #59467 (Better diagnostic for binary operation on BoxedValues)
 - #59473 (Do not emit incorrect borrow suggestion involving macros and fix overlapping multiline spans)
 - #59480 (Update stdsimd)
 - #59486 (Visit `ImplItem` in `dead_code` lint)
 - #59510 (Rename `type_parameters` to `generics` and so on)

Failed merges:

 - #59516 (Update cargo)

r? @ghost
@bors bors merged commit 362d243 into rust-lang:master Mar 29, 2019
@taiki-e taiki-e deleted the no-core branch March 30, 2019 05:33
@Centril Centril added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 30, 2019
@Centril
Copy link
Contributor

Centril commented Mar 30, 2019

Beta nominating for bootstrap compiler due to #58702 (comment).

@pnkfelix
Copy link
Member

pnkfelix commented Apr 4, 2019

discussed at T-compiler meeting. Declining to beta-backport: The only client a backport would benefit would be rustc bootstraps (and not the stable channel), and we're already going to have nightly roll to beta next week anyway, so the risk/reward ratio is bad.

@pnkfelix pnkfelix removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 4, 2019
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants