Reference lang items during AST lowering by davidtwco · Pull Request #75145 · rust-lang/rust

davidtwco · 2020-08-04T14:16:10Z

Fixes #60607 and fixes #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.
~~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 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

petrochenkov · 2020-08-09T18:50:53Z

(I'll review this next weekend or during the week.)

src/librustc_passes/lang_items.rs

src/librustc_hir/hir.rs

src/librustc_typeck/check/mod.rs

src/librustc_hir/hir.rs

This commit adds support for lang items (`#[lang = "..."]` attributes) on enum variants. Signed-off-by: David Wood <david@davidtw.co>

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>

This commit adds new lang items which will be used in AST lowering once `QPath::LangItem` is introduced. Co-authored-by: Matthew Jasper <mjjasper1@gmail.com> Signed-off-by: David Wood <david@davidtw.co>

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>

This commit implements support for `QPath::LangItem` and `GenericBound::LangItemTrait` in save analysis. Signed-off-by: David Wood <david@davidtw.co>

This commit simplifies `is_range_literal` by checking for `QPath::LangItem` containing range-related lang items, rather than using a heuristic. Co-authored-by: Matthew Jasper <mjjasper1@gmail.com> Signed-off-by: David Wood <david@davidtw.co>

This commit modifies name resolution to ensure that new scopes are introduced from lang-item generic bounds. Co-authored-by: Matthew Jasper <mjjasper1@gmail.com> Signed-off-by: David Wood <david@davidtw.co>

This commit adds support for cleaning `QPath::LangItem` and `hir::GenericBound::LangItemTrait` in rustdoc. `QPath::LangItem` does not require lowering, and `hir::GenericBound::LangItemTrait` is lowered to a `GenericBound::TraitBound`. Signed-off-by: David Wood <david@davidtw.co>

petrochenkov · 2020-08-16T18:41:46Z

@bors r+

bors · 2020-08-16T18:41:47Z

📌 Commit bde529f has been approved by petrochenkov

@matthewjasper

…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

eddyb · 2020-08-16T21:39:01Z

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.

You can use ./x.py test src/tools/clippy at the very least. Suspecting this is what broke a rollup (#75606), so:
@bors rollup=never

eddyb · 2020-08-16T21:41:44Z

Btw the title still says "Preallocate DefIds for lang items" but it doesn't look like that's the strategy taken?
Instead, lang items are referenced by the HIR without using their DefIds.

(the alternative to this would be doing lang item collection before the HIR, but I think this solution is nicer)

Also, surely this has perf implications?

@bors try @rust-timer queue

bors · 2020-08-16T21:41:45Z

🙅 Please do not try after a pull request has been r+ed. If you need to try, unapprove (r-) it first.

rust-timer · 2020-08-16T21:41:46Z

Awaiting bors try build completion

bors · 2020-08-16T21:42:34Z

⌛ Trying commit bde529f with merge 48b5808e9529407d15e12c40168e01ce15a6e9db...

bors · 2020-08-16T22:25:15Z

☀️ Try build successful - checks-actions, checks-azure
Build commit: 48b5808e9529407d15e12c40168e01ce15a6e9db (48b5808e9529407d15e12c40168e01ce15a6e9db)

rust-timer · 2020-08-16T22:25:17Z

Queued 48b5808e9529407d15e12c40168e01ce15a6e9db with parent 9b4db69, future comparison URL.

rust-timer · 2020-08-17T01:45:43Z

Finished benchmarking try commit (48b5808e9529407d15e12c40168e01ce15a6e9db): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

eddyb · 2020-08-17T02:21:50Z

Perhaps unsurprisingly, this is a nice win for await-heavy code.

petrochenkov · 2020-08-17T06:21:41Z

You can use ./x.py test src/tools/clippy at the very least.

It's ./x.py test src/tools/clippy --stage 2 now that the defaults were changed.

This commit updates clippy with the introduction of `QPath::LangItem` so that it still compiles. Signed-off-by: David Wood <david@davidtw.co>

davidtwco · 2020-08-17T12:55:45Z

Alright - I've ran the clippy tests now and fixed all of the issues that this introduced.

petrochenkov · 2020-08-17T20:28:28Z

@bors r+

bors · 2020-08-17T20:28:29Z

📌 Commit f1ce294 has been approved by petrochenkov

bors · 2020-08-17T20:52:05Z

⌛ Testing commit f1ce294 with merge 792c645...

bors · 2020-08-17T23:15:59Z

☀️ Test successful - checks-actions, checks-azure
Approved by: petrochenkov
Pushing 792c645 to master...

ecstatic-morse · 2020-08-24T21:19:17Z

Final perf results show a small win as expected. Nice work!

rust-highfive assigned oli-obk Aug 4, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 4, 2020

davidtwco changed the title ~~Issue 60607 preallocate defid for lang items~~ Preallocate DefIds for lang items Aug 4, 2020

This was referenced Aug 4, 2020

Preallocate DefIds for lang items and use lang items in lowering #60607

Closed

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

Closed

petrochenkov self-assigned this Aug 4, 2020

petrochenkov reviewed Aug 15, 2020

View reviewed changes

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 Aug 15, 2020

davidtwco and others added 8 commits August 16, 2020 15:42

lang_items: add support for lang items on variants

764dc3d

This commit adds support for lang items (`#[lang = "..."]` attributes) on enum variants. Signed-off-by: David Wood <david@davidtw.co>

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>

hir: introduce lang items for AST lowering

7dee5f8

This commit adds new lang items which will be used in AST lowering once `QPath::LangItem` is introduced. Co-authored-by: Matthew Jasper <mjjasper1@gmail.com> Signed-off-by: David Wood <david@davidtw.co>

save_analysis: support QPath::LangItem

1e2f350

This commit implements support for `QPath::LangItem` and `GenericBound::LangItemTrait` in save analysis. Signed-off-by: David Wood <david@davidtw.co>

hir: simplify is_range_literal

664ecf1

This commit simplifies `is_range_literal` by checking for `QPath::LangItem` containing range-related lang items, rather than using a heuristic. Co-authored-by: Matthew Jasper <mjjasper1@gmail.com> Signed-off-by: David Wood <david@davidtw.co>

resolve: support GenericBound::LangItemTrait

8367af4

This commit modifies name resolution to ensure that new scopes are introduced from lang-item generic bounds. Co-authored-by: Matthew Jasper <mjjasper1@gmail.com> Signed-off-by: David Wood <david@davidtw.co>

davidtwco force-pushed the issue-60607-preallocate-defid-for-lang-items branch from 8d886c4 to bde529f Compare August 16, 2020 15:13

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 Aug 16, 2020

Dylan-DPC-zz mentioned this pull request Aug 16, 2020

Rollup of 10 pull requests #75606

Closed

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 Aug 16, 2020

clippy: support QPath::LangItem

f1ce294

This commit updates clippy with the introduction of `QPath::LangItem` so that it still compiles. Signed-off-by: David Wood <david@davidtw.co>

davidtwco force-pushed the issue-60607-preallocate-defid-for-lang-items branch from bde529f to f1ce294 Compare August 17, 2020 12:55

davidtwco changed the title ~~Preallocate DefIds for lang items~~ Reference lang items during AST lowering Aug 17, 2020

davidtwco added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 17, 2020

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 17, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 17, 2020

bors merged commit 792c645 into rust-lang:master Aug 17, 2020

davidtwco deleted the issue-60607-preallocate-defid-for-lang-items branch August 17, 2020 23:38

bors mentioned this pull request Aug 17, 2020

Moved coverage counter injection from BasicBlock to Statement. #75563

Merged

davidtwco mentioned this pull request Aug 26, 2020

Range is not a lang item #53720

Closed

Veykril mentioned this pull request Aug 8, 2021

fix: Look for enum variants and trait assoc functions when looking for lang items rust-lang/rust-analyzer#9808

Merged

bjorn3 mentioned this pull request May 15, 2022

Make lang items usable from libsyntax #20541

Closed

cuviper added this to the 1.47.0 milestone May 2, 2024

Uh oh!

Conversation

davidtwco commented Aug 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Aug 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

petrochenkov commented Aug 16, 2020

Uh oh!

bors commented Aug 16, 2020

Uh oh!

eddyb commented Aug 16, 2020

Uh oh!

eddyb commented Aug 16, 2020

Uh oh!

bors commented Aug 16, 2020

Uh oh!

rust-timer commented Aug 16, 2020

Uh oh!

bors commented Aug 16, 2020

Uh oh!

bors commented Aug 16, 2020

Uh oh!

rust-timer commented Aug 16, 2020

Uh oh!

rust-timer commented Aug 17, 2020

Uh oh!

eddyb commented Aug 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Aug 17, 2020

Uh oh!

davidtwco commented Aug 17, 2020

Uh oh!

petrochenkov commented Aug 17, 2020

Uh oh!

bors commented Aug 17, 2020

Uh oh!

bors commented Aug 17, 2020

Uh oh!

bors commented Aug 17, 2020

Uh oh!

ecstatic-morse commented Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

davidtwco commented Aug 4, 2020 •

edited

Loading

eddyb commented Aug 17, 2020 •

edited

Loading

ecstatic-morse commented Aug 24, 2020 •

edited

Loading