Remove CrateNum parameter for queries that only work on local crate by cjgillot · Pull Request #85178 · rust-lang/rust

cjgillot · 2021-05-11T13:01:11Z

The pervasive CrateNum parameter is a remnant of the multi-crate rustc idea.

Using () as query key in those cases avoids having to worry about the validity of the query key.

rust-highfive · 2021-05-11T13:01:14Z

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in intra-doc-links.

cc @jyn514

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

rust-highfive · 2021-05-11T13:01:15Z

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

cjgillot · 2021-05-11T15:26:30Z

@bors try @rust-timer queue

rust-timer · 2021-05-11T15:26:32Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-05-11T15:26:39Z

⌛ Trying commit 744dece9b42ec88e849099716a81b1a99bcfaab1 with merge f690672c840f1ece42fcb14f6143579ce2f69016...

bors · 2021-05-11T16:12:39Z

☀️ Try build successful - checks-actions
Build commit: f690672c840f1ece42fcb14f6143579ce2f69016 (f690672c840f1ece42fcb14f6143579ce2f69016)

rust-timer · 2021-05-11T16:12:41Z

Queued f690672c840f1ece42fcb14f6143579ce2f69016 with parent 506e75c, future comparison URL.

marmeladema · 2021-05-11T16:20:41Z

I have tried something similar in #71648 but I gave up^^

rust-timer · 2021-05-11T18:15:00Z

Finished benchmarking try commit (f690672c840f1ece42fcb14f6143579ce2f69016): 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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

Xanewok · 2021-05-11T22:38:18Z

(...) remnant of the multi-crate rustc idea

Sorry for the noise here but is this idea abandoned/discussed recently somewhere?

bors · 2021-05-12T11:37:04Z

☔ The latest upstream changes (presumably #83610) made this pull request unmergeable. Please resolve the merge conflicts.

varkor · 2021-05-16T13:05:41Z

compiler/rustc_middle/src/hir/map/mod.rs

-pub(super) fn index_hir<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> &'tcx IndexedHir<'tcx> {
-    assert_eq!(cnum, LOCAL_CRATE);
-
+pub(super) fn index_hir<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> &'tcx IndexedHir<'tcx> {


Do we have to have a (): () parameter? Presumably this is a quirk of the query system?

The query system is designed to work with queries taking exactly two arguments: the TyCtxt and a key. I am not convinced that removing the extra () parameter is worth the added complexity in the query system. (I am not even sure this is possible without a sizeable modification to the query plumbing.)

varkor · 2021-05-16T13:16:00Z

I think it'd be better for someone more familiar with the query system to review here.

r? @oli-obk

oli-obk · 2021-05-16T23:43:29Z

(...) remnant of the multi-crate rustc idea

Sorry for the noise here but is this idea abandoned/discussed recently somewhere?

niko mentioned this in #71648 (comment)

considering the approval of niko and eddyb in #71648 let's go ahead with this idea

the changes themselves lgtm

@bors r+ rollup=never (bitrotty I presume)

bors · 2021-05-16T23:43:31Z

📌 Commit 1ebf6d1 has been approved by oli-obk

bors · 2021-05-17T01:42:07Z

⌛ Testing commit 1ebf6d1 with merge 3396a38...

bors · 2021-05-17T04:22:53Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 3396a38 to master...

rust-highfive · 2021-05-17T04:23:21Z

📣 Toolstate changed by #85178!

Tested on commit 3396a38.
Direct link to PR: #85178

💔 miri on windows: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb).
💔 miri on linux: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb).

@oli-obk

Tested on commit rust-lang/rust@3396a38. Direct link to PR: <rust-lang/rust#85178> 💔 miri on windows: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb). 💔 miri on linux: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb).

Remove CrateNum parameter for queries that only work on local crate The pervasive `CrateNum` parameter is a remnant of the multi-crate rustc idea. Using `()` as query key in those cases avoids having to worry about the validity of the query key.

rust-highfive assigned varkor May 11, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 11, 2021

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 11, 2021

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 11, 2021

cjgillot added 13 commits May 12, 2021 13:58

Use () in dependency_formats.

85a14d7

Use () in reachable_set.

3a72991

Use () for HIR queries.

601453a

Use () for entry_fn.

829a9d3

Use () for plugin_registrar_fn.

e9e1900

Use () for proc_macro_decls_static.

b7bf467

Use () for lint_levels.

9d15abe

Use () for mir_keys.

75f4f6e

Use () for crate_variances.

09f205a

Use () for inferred_outlives_crate.

3aa6f3e

Use () for inherent_impls.

837f3e3

Use () for typeck_item_bodies.

2c71ad4

Use () for privacy.

9849327

cjgillot added 6 commits May 12, 2021 13:58

Use () for lang items.

437a46d

Use () for visible_parent_map.

7f25b9f

Use () for all_traits.

4e8d4bd

Use () for codegen queries.

0bde3b1

Use () for analysis.

1fb9cad

Bless tests.

1ebf6d1

cjgillot force-pushed the local-crate branch from 2989b42 to 1ebf6d1 Compare May 12, 2021 12:02

varkor reviewed May 16, 2021

View reviewed changes

rust-highfive assigned oli-obk and unassigned varkor May 16, 2021

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 16, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label May 17, 2021

bors merged commit 3396a38 into rust-lang:master May 17, 2021

rustbot added this to the 1.54.0 milestone May 17, 2021

rust-highfive mentioned this pull request May 17, 2021

miri no longer builds after rust-lang/rust#85178 #85397

Closed

This was referenced May 17, 2021

Reduce the amount of untracked state in TyCtxt #85153

Merged

Plugin interface cleanup #85296

Merged

cjgillot deleted the local-crate branch May 17, 2021 19:40

Uh oh!

Conversation

cjgillot commented May 11, 2021

Uh oh!

rust-highfive commented May 11, 2021

Uh oh!

rust-highfive commented May 11, 2021

Uh oh!

This comment has been minimized.

cjgillot commented May 11, 2021

Uh oh!

rust-timer commented May 11, 2021

Uh oh!

bors commented May 11, 2021

Uh oh!

bors commented May 11, 2021

Uh oh!

rust-timer commented May 11, 2021

Uh oh!

marmeladema commented May 11, 2021

Uh oh!

rust-timer commented May 11, 2021

Uh oh!

Xanewok commented May 11, 2021

Uh oh!

bors commented May 12, 2021

Uh oh!

varkor May 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjgillot May 16, 2021

Choose a reason for hiding this comment

Uh oh!

varkor commented May 16, 2021

Uh oh!

oli-obk commented May 16, 2021

Uh oh!

bors commented May 16, 2021

Uh oh!

bors commented May 17, 2021

Uh oh!

bors commented May 17, 2021

Uh oh!

rust-highfive commented May 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

varkor May 16, 2021 •

edited

Loading