normalize in `trait_ref_is_knowable` in new solver #114457

lcnr · 2023-08-04T10:20:33Z

fixes rust-lang/trait-system-refactor-initiative#51

Alternatively we could avoid normalizing the self type and do this at the end of the assemble_candidates_via_self_ty stack by splitting candidates into:

applicable without normalizing self type
applicable for aliases, even if they can be normalized
applicable for stuff which cannot get normalized further

I don't think this would have any significant benefits and it also seems non-trivial to avoid normalizing only the self type in trait_ref_is_knowable.

r? @compiler-errors

rustbot · 2023-08-04T10:20:40Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

compiler-errors · 2023-08-04T15:57:56Z

compiler/rustc_trait_selection/src/solve/assembly/mod.rs

Do you think lazy_normalize_ty should just return this FailureKind?

hmm, I think it's valuable to guarantee that trait_ref_is_knowable always returns the outer Ok when used outside of the new solver

compiler-errors

r=me nit or not

bors · 2023-08-05T06:26:59Z

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

lcnr · 2023-08-07T20:16:17Z

@bors r=compiler-errors rollup

bors · 2023-08-07T20:16:20Z

📌 Commit ff150f9 has been approved by compiler-errors

It is now in the queue for this repository.

compiler-errors · 2023-08-07T20:58:48Z

@bors r-

looks like we're hitting that stability assertion :(

lcnr · 2023-08-08T07:58:42Z

that sure is a nice way to get a minimized test ✨

lcnr · 2023-08-08T14:34:07Z

lowered the stability check to a warn! and accepted it with a FIXME. It is non-trivial to fix and by keeping it as a warn we should be able to actually keep it enabled while testing stuff.

lcnr · 2023-08-08T15:48:06Z

see rust-lang/trait-system-refactor-initiative#54 for an issue tracking the evaluate goal instability bug.

@bors r=compiler-errors rollup (new solver)

bors · 2023-08-08T15:48:08Z

📌 Commit 48b5dbd has been approved by compiler-errors

It is now in the queue for this repository.

…ze, r=compiler-errors normalize in `trait_ref_is_knowable` in new solver fixes rust-lang/trait-system-refactor-initiative#51 Alternatively we could avoid normalizing the self type and do this at the end of the `assemble_candidates_via_self_ty` stack by splitting candidates into: - applicable without normalizing self type - applicable for aliases, even if they can be normalized - applicable for stuff which cannot get normalized further I don't think this would have any significant benefits and it also seems non-trivial to avoid normalizing only the self type in `trait_ref_is_knowable`. r? `@compiler-errors`

compiler-errors · 2023-08-08T18:54:52Z

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs

        Ok((has_changed, certainty, nested_goals))
    }

+    fn check_evaluate_goal_stable_result(


This check should even when debug assertions are enabled -- otherwise, there's UI test instability between rustc w/ debug_assertions and not.

compiler-errors · 2023-08-08T18:55:14Z

Failed in rollup

@bors r-

#114632 (comment)

lcnr · 2023-08-10T12:45:09Z

rebased on top of #114694 so evaluate_goal is stable now. Marking it as blocked on this PR

compiler-errors · 2023-08-12T18:51:54Z

@bors r+

bors · 2023-08-12T18:51:58Z

📌 Commit 5176288 has been approved by compiler-errors

It is now in the queue for this repository.

…ze, r=compiler-errors normalize in `trait_ref_is_knowable` in new solver fixes rust-lang/trait-system-refactor-initiative#51 Alternatively we could avoid normalizing the self type and do this at the end of the `assemble_candidates_via_self_ty` stack by splitting candidates into: - applicable without normalizing self type - applicable for aliases, even if they can be normalized - applicable for stuff which cannot get normalized further I don't think this would have any significant benefits and it also seems non-trivial to avoid normalizing only the self type in `trait_ref_is_knowable`. r? `@compiler-errors`

bors · 2023-08-13T05:18:30Z

⌛ Testing commit 5176288 with merge 7455aa5...

bors · 2023-08-13T07:03:22Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 7455aa5 to master...

rust-timer · 2023-08-13T08:24:32Z

Finished benchmarking commit (7455aa5): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.4%	[0.4%, 0.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.1%	[2.0%, 4.7%]	9
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 632.32s -> 633.837s (0.24%)

lcnr added the A-coherence Area: Coherence label Aug 4, 2023

rustbot assigned compiler-errors Aug 4, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 4, 2023

compiler-errors reviewed Aug 4, 2023

View reviewed changes

compiler-errors approved these changes Aug 4, 2023

View reviewed changes

compiler-errors 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 4, 2023

lcnr force-pushed the trait_ref_is_knowable-normalize branch from d2d3fd4 to ff150f9 Compare August 7, 2023 20:11

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 7, 2023

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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 7, 2023

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 8, 2023

matthiaskrgr mentioned this pull request Aug 8, 2023

Rollup of 8 pull requests #114632

Closed

compiler-errors reviewed Aug 8, 2023

View reviewed changes

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 8, 2023

lcnr force-pushed the trait_ref_is_knowable-normalize branch 3 times, most recently from bf154dc to d502fff Compare August 10, 2023 12:44

lcnr added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 10, 2023

lcnr added 2 commits August 12, 2023 20:37

normalize in trait_ref_is_knowable in new solver

9eeaf1f

lower evaluate_goal stability check to warn

5176288

lcnr force-pushed the trait_ref_is_knowable-normalize branch from d502fff to 5176288 Compare August 12, 2023 18:50

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-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 12, 2023

matthiaskrgr mentioned this pull request Aug 12, 2023

Rollup of 5 pull requests #114765

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 13, 2023

bors merged commit 7455aa5 into rust-lang:master Aug 13, 2023

rustbot added this to the 1.73.0 milestone Aug 13, 2023

bors mentioned this pull request Aug 13, 2023

Rework negative coherence to properly consider impls that only partly overlap #112875

Merged

lcnr deleted the trait_ref_is_knowable-normalize branch August 14, 2023 07:34

compiler-errors mentioned this pull request Aug 15, 2023

Tracking issue for -Znext-solver=coherence (next solver enablement in coherence) #114862

Closed

20 tasks

normalize in trait_ref_is_knowable in new solver #114457

normalize in trait_ref_is_knowable in new solver #114457

Uh oh!

Conversation

lcnr commented Aug 4, 2023

Uh oh!

rustbot commented Aug 4, 2023

Uh oh!

compiler-errors Aug 4, 2023

Choose a reason for hiding this comment

Uh oh!

lcnr Aug 7, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

bors commented Aug 5, 2023

Uh oh!

lcnr commented Aug 7, 2023

Uh oh!

bors commented Aug 7, 2023

Uh oh!

This comment has been minimized.

compiler-errors commented Aug 7, 2023

Uh oh!

lcnr commented Aug 8, 2023

Uh oh!

lcnr commented Aug 8, 2023

Uh oh!

lcnr commented Aug 8, 2023

Uh oh!

bors commented Aug 8, 2023

Uh oh!

compiler-errors Aug 8, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Aug 8, 2023

Uh oh!

lcnr commented Aug 10, 2023

Uh oh!

compiler-errors commented Aug 12, 2023

Uh oh!

bors commented Aug 12, 2023

Uh oh!

bors commented Aug 13, 2023

Uh oh!

bors commented Aug 13, 2023

Uh oh!

rust-timer commented Aug 13, 2023

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

normalize in `trait_ref_is_knowable` in new solver #114457

normalize in `trait_ref_is_knowable` in new solver #114457