lazily "compute" anon const default substs #87280

lcnr · 2021-07-19T14:23:15Z

Continuing the work of #83086, this implements the discussed solution for the unused substs problem. As of now, anonymous constants inherit all of their parents generics, even if they do not use them, e.g. in fn foo<T, const N: usize>() -> [T; N + 1], the array length has T as a generic parameter even though it doesn't use it. These unused substs cause some backwards incompatible, and imo incorrect behavior, e.g. #78369.

We do not actually filter any generic parameters here and the default_anon_const_substs query still a dummy which only checks that

we now prevent the previously existing query cycles and are able to call predicates_of(parent) when computing the substs of anonymous constants
the default anon consts substs only include the typeflags we assume it does.

Implementing that filtering will be left as future work.

The idea of this PR is to delay the creation of the anon const substs until after we've computed predicates_of for the parent of the anon const. As the predicates of the parent can however contain the anon const we still have to create a ty::Const for it.

We do this by changing the substs field of ty::Unevaluated to an option and modifying accesses to instead call the method unevaluated.substs(tcx) which returns the substs as before. If the substs - now substs_ - of ty::Unevaluated are None, it means that the anon const currently has its default substs, i.e. the substs it has when first constructed, which are the generic parameters it has available. To be able to call unevaluated.substs(tcx) in a TypeVisitor, we add the non-defaulted method fn tcx_for_anon_const_substs(&self) -> Option<TyCtxt<'tcx>>. In case tcx_for_anon_const_substs returns None, unknown anon const default substs are skipped entirely.

Even when substs_ is None we still have to treat the constant as if it has its default substs. To do this, TypeFlags are modified so that it is clear whether they can still change when exposing any anon const default substs. A new flag, HAS_UNKNOWN_DEFAULT_CONST_SUBSTS, is added in case some default flags are missing.

The rest of this PR are some smaller changes to either not cause cycles by trying to access the default anon const substs too early or to be able to access the tcx in previously unused locations.

cc @rust-lang/project-const-generics
r? @nikomatsakis

rust-highfive · 2021-07-19T14:23:18Z

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

lcnr · 2021-07-20T07:28:37Z

@bors try @rust-timer queue

rust-timer · 2021-07-20T07:28:38Z

Awaiting bors try build completion.

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

bors · 2021-07-20T07:28:47Z

⌛ Trying commit ae49d0183a9a652553f901f75932d78af30c3483 with merge e38962e0c798b6fd647d7306b9f7904a89a4fe6c...

bors · 2021-07-20T08:16:13Z

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

rust-timer · 2021-07-20T08:16:14Z

Queued e38962e0c798b6fd647d7306b9f7904a89a4fe6c with parent c9aa259, future comparison URL.

rust-timer · 2021-07-20T11:34:39Z

Finished benchmarking try commit (e38962e0c798b6fd647d7306b9f7904a89a4fe6c): comparison url.

Summary: This change led to significant regressions 😿 in compiler performance.

Very large regression in instruction counts (up to 16.5% on full builds of ctfe-stress-4-check)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

rust-log-analyzer · 2021-08-26T15:00:42Z

The job dist-mips-linux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2021-08-26T15:01:12Z

💔 Test failed - checks-actions

lcnr · 2021-08-26T15:47:39Z

@bors retry

bors · 2021-08-26T22:26:26Z

⌛ Testing commit 7cbfa2e with merge 517c28e...

bors · 2021-08-27T01:07:09Z

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 517c28e to master...

rylev · 2021-09-01T08:36:11Z

The performance hit was justified here so marking it as such.

@rustbot label +perf-regression-triaged

Prevent duplicate caller bounds candidates by exposing default substs in Unevaluated Fixes rust-lang#89334 The changes introduced in rust-lang#87280 allowed for "duplicate" caller bounds candidates to be assembled that only differed in their default substs having been "exposed" or not and resulted in an ambiguity error during trait selection. To fix this we expose the defaults substs during the creation of the ParamEnv. r? `@lcnr`

This manifistated in rust-lang#90195 with compiler being unable to keep one candidate for a trait impl, if where is a global impl and more than one trait bound in the where clause. Before rust-lang#87280 `candidate_should_be_dropped_in_favor_of` was using `TypeFoldable::is_global()` that was enough to discard the two `ParamCandidate`s. But rust-lang#87280 changed it to use `TypeFoldable::is_known_global()` instead, which is pessimistic, so now the compiler drops the global impl instead (because `is_known_global` is not sure) and then can't decide between the two `ParamCandidate`s. Switching it to use `is_global` again solves the issue. Fixes rust-lang#90195.

Use `is_global` in `candidate_should_be_dropped_in_favor_of` This manifistated in rust-lang#90195 with compiler being unable to keep one candidate for a trait impl, if where is a global impl and more than one trait bound in the where clause. Before rust-lang#87280 `candidate_should_be_dropped_in_favor_of` was using `TypeFoldable::is_global()` that was enough to discard the two `ParamCandidate`s. But rust-lang#87280 changed it to use `TypeFoldable::is_known_global()` instead, which is pessimistic, so now the compiler drops the global impl instead (because `is_known_global` is not sure) and then can't decide between the two `ParamCandidate`s. Switching it to use `is_global` again solves the issue. Fixes rust-lang#90195.

…, r=lcnr partially revertish `lazily "compute" anon const default substs` reverts rust-lang#87280 except for some of the changes around `ty::Unevaluated` having a visitor and a generic for promoted why revert: <rust-lang#92805 (comment)> r? `@lcnr`

lcnr added A-const-generics Area: const generics (parameters and arguments) A-lazy-normalization Area: Lazy normalization (tracking issue: #60471) labels Jul 19, 2021

rust-highfive assigned nikomatsakis Jul 19, 2021

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

This comment has been minimized.

Sign in to view

lcnr force-pushed the lazy-anon-const-default-substs branch from ddae395 to 78a2a47 Compare July 19, 2021 14:34

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 Jul 19, 2021

This comment has been minimized.

Sign in to view

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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 19, 2021

lcnr force-pushed the lazy-anon-const-default-substs branch from 9aac53b to ae49d01 Compare July 20, 2021 07:27

This comment has been minimized.

Sign in to view

lcnr force-pushed the lazy-anon-const-default-substs branch from ae49d01 to 892e471 Compare July 20, 2021 09:19

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

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 26, 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 Aug 26, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 27, 2021

bors merged commit 517c28e into rust-lang:master Aug 27, 2021

rustbot added this to the 1.56.0 milestone Aug 27, 2021

This was referenced Aug 27, 2021

Fixed #85845: Added a note in E0369 if the missing trait is PartialEq #85929

Closed

Pretty print assertion failures in tests #79001

Closed

update const generics feature gates #88369

Merged

lcnr deleted the lazy-anon-const-default-substs branch August 27, 2021 07:23

rustbot added the perf-regression-triaged The performance regression has been triaged. label Sep 1, 2021

JohnTitor added a commit to JohnTitor/rust-semverver that referenced this pull request Oct 13, 2021

Rustup to rust-lang/rust#88149 and rust-lang/rust#87280

523a3c7

hkratz mentioned this pull request Oct 23, 2021

rkyv in Rust 1.56: type annotations needed #90195

Closed

b-naber mentioned this pull request Oct 25, 2021

Prevent duplicate caller bounds candidates by exposing default substs in Unevaluated #90266

Merged

yanok mentioned this pull request Oct 28, 2021

Use is_global in candidate_should_be_dropped_in_favor_of #90375

Merged

lcnr mentioned this pull request Dec 11, 2021

Tracking issue for const generics (RFC 2000) #44580

Closed

16 tasks

BoxyUwU mentioned this pull request Jan 12, 2022

partially revertish lazily "compute" anon const default substs #92805

Merged

lcnr mentioned this pull request Mar 21, 2022

update section for type system constants rust-lang/rustc-dev-guide#1329

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lazily "compute" anon const default substs #87280

lazily "compute" anon const default substs #87280

lcnr commented Jul 19, 2021

rust-highfive commented Jul 19, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

lcnr commented Jul 20, 2021

rust-timer commented Jul 20, 2021

bors commented Jul 20, 2021

This comment has been minimized.

bors commented Jul 20, 2021

rust-timer commented Jul 20, 2021

rust-timer commented Jul 20, 2021

rust-log-analyzer commented Aug 26, 2021

bors commented Aug 26, 2021

lcnr commented Aug 26, 2021

bors commented Aug 26, 2021

bors commented Aug 27, 2021

rylev commented Sep 1, 2021

lazily "compute" anon const default substs #87280

lazily "compute" anon const default substs #87280

Conversation

lcnr commented Jul 19, 2021

rust-highfive commented Jul 19, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

lcnr commented Jul 20, 2021

rust-timer commented Jul 20, 2021

bors commented Jul 20, 2021

This comment has been minimized.

bors commented Jul 20, 2021

rust-timer commented Jul 20, 2021

rust-timer commented Jul 20, 2021

rust-log-analyzer commented Aug 26, 2021

bors commented Aug 26, 2021

lcnr commented Aug 26, 2021

bors commented Aug 26, 2021

bors commented Aug 27, 2021

rylev commented Sep 1, 2021