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.

Sign up for GitHub

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

Jump to bottom

extern crate ... as std overrides the injected crate on 2018 edition #61019

Closed
matthewjasper opened this issue May 21, 2019 · 2 comments · Fixed by #75145
Closed

extern crate ... as std overrides the injected crate on 2018 edition #61019

matthewjasper opened this issue May 21, 2019 · 2 comments · Fixed by #75145
Assignees
@davidtwco
Labels
A-HIR Area: The high-level intermediate representation (HIR) A-resolve Area: Name resolution C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@matthewjasper
Copy link
Contributor

matthewjasper commented May 21, 2019 •
edited
Loading

On the 2018 edition the following code:

extern crate some_crate as std; // or as core with #![no_std]

will:

  • Try to resolve prelude::v1::* in some_crate, failing if it doesn't exist. This probably limits the plausible cases where this could happen to extern crate core as std, and extern crate core as std with #![no_std].
  • Use some_crate in any built-in derives and for HIR lowering.

I would expect that the prelude and derives/HIR lowering to always use the injected std/core. Paths starting with std should still resolve to the in the crate the user provided.

In the 2015 edition it creates a name conflict with the implicit extern crate std.

cc @petrochenkov @eddyb
@matthewjasper matthewjasper mentioned this issue May 21, 2019
Closed
@jonas-schievink jonas-schievink added A-HIR Area: The high-level intermediate representation (HIR) A-resolve Area: Name resolution labels May 21, 2019
@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label May 21, 2019
@matthewjasper matthewjasper mentioned this issue Aug 26, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Sep 7, 2019
@Centril
Rollup merge of rust-lang#63919 - matthewjasper:remove-gensymmed, r=p…
db493ef 
…etrochenkov

Use hygiene for AST passes

AST passes are now able to have resolve consider their expansions as if they were opaque macros defined either in some module in the current crate, or a fake empty module with `#[no_implicit_prelude]`.

* Add an ExpnKind for AST passes.
* Remove gensyms in AST passes.
* Remove gensyms in`#[test]`, `#[bench]` and `#[test_case]`.
* Allow opaque macros to define tests.
* Move tests for unit tests to their own directory.
* Remove `Ident::{gensym, is_gensymed}` - `Ident::gensym_if_underscore` still exists.

cc rust-lang#60869, rust-lang#61019

r? @petrochenkov
@Centril
Copy link
Contributor

Centril commented Dec 1, 2019

This would be fixed by #60607.

@Centril Centril added the C-bug Category: This is a bug. label Dec 1, 2019
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 1, 2019
@matthewjasper matthewjasper mentioned this issue Jun 23, 2020
Closed
@davidtwco davidtwco mentioned this issue Aug 4, 2020
Merged
@davidtwco davidtwco self-assigned this Aug 4, 2020
@davidtwco
Copy link
Member

Opened #75145 which fixes this.

davidtwco added a commit to davidtwco/rust that referenced this issue Aug 16, 2020
@davidtwco
tests: add test for rust-lang#61019's current behaviour
734441c 
This commit adds a test for rust-lang#61019 where a extern crate is imported as
`std` which results in name resolution to fail due to the uses of `std`
types introduced from lowering.

Signed-off-by: David Wood <david@davidtw.co>
davidtwco added a commit to davidtwco/rust that referenced this issue Aug 16, 2020
@davidtwco @matthewjasper
hir: introduce QPath::LangItem
762137e 
This commit introduces `QPath::LangItem` to the HIR and uses it in AST
lowering instead of constructing a `hir::Path` from a slice of symbols.

This might be better for performance, but is also much cleaner as the
previous approach is fragile. In addition, it resolves a bug (rust-lang#61019)
where an extern crate imported as "std" would result in the paths
created during AST lowering being resolved incorrectly (or not at all).

Co-authored-by: Matthew Jasper <mjjasper1@gmail.com>
Signed-off-by: David Wood <david@davidtw.co>
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Aug 16, 2020
@Dylan-DPC
Rollup merge of rust-lang#75145 - davidtwco:issue-60607-preallocate-d…
3e5ce7b 
…efid-for-lang-items, r=petrochenkov

Preallocate `DefId`s for lang items

Fixes rust-lang#60607 and fixes rust-lang#61019.

This PR introduces `QPath::LangItem` to the HIR and uses it in AST lowering instead of constructing a `hir::Path` from a slice of symbols:

- Credit for much of this work goes to @matthewjasper, I basically just [rebased their earlier work](matthewjasper@a227c70#diff-c0f791ead38d2d02916faaad0f56f41d).
- Changes to Clippy might not be correct, they compile but attempting to run tests through `./x.py` produced failures which appeared spurious, so I didn't run any clippy tests.
- Changes to save analysis might not be correct - tests pass but I don't have a lot of confidence in those changes being correct.
- I've used `GenericBounds::LangItemTrait` rather than changing `PolyTraitRef`, as suggested by @matthewjasper [in this comment](matthewjasper@a227c70#r40107992) but I'd prefer that be left for a follow-up.
- I've split things into smaller commits fairly arbitrarily to make the diff easier to review, each commit should compile but might not pass tests until the final commit.

r? @oli-obk
cc @matthewjasper
@bors bors closed this as completed in 792c645 Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@davidtwco davidtwco

Labels
A-HIR Area: The high-level intermediate representation (HIR) A-resolve Area: Name resolution C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reference lang items during AST lowering davidtwco/rust
4 participants
@Centril @davidtwco @jonas-schievink @matthewjasper