Skip to content

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Oct 4, 2024

Redux of #128744.

Splits out relate for the new solver so that implementors don't need to implement it themselves.

r? lcnr

@rustbot 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 Oct 4, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 4, 2024

changes to the core type system

cc @compiler-errors, @lcnr

@compiler-errors compiler-errors changed the title Solver relating Introduce SolverRelating type relation to the new solver Oct 4, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member Author

but until we get rid of LUB/GLB this won't be fully possible

We could also, perhaps, get rid of CombineFields and just call infcx.at.eq when needing to equate in LUB. I could do that as a follow-up, especially because it has some overhead with perf since we'd need to clone obligationcauses.

@bors
Copy link
Collaborator

bors commented Oct 7, 2024

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

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 7, 2024
…s, r=<try>

Remove `CombineFields`

This conflicts with rust-lang#131263, but if this one lands first then perhaps rust-lang#131263 could then go ahead and remove all the branching on solver in `TypeRelating`. We could perhaps then rename `TypeRelating` to `OldSolverRelating` or something, idk.

r? lcnr
@compiler-errors compiler-errors force-pushed the solver-relating branch 2 times, most recently from 5fa0570 to 7ac5804 Compare October 7, 2024 11:18
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 7, 2024
…s, r=lcnr

Remove `CombineFields`

This conflicts with rust-lang#131263, but if this one lands first then perhaps rust-lang#131263 could then go ahead and remove all the branching on solver in `TypeRelating`. We could perhaps then rename `TypeRelating` to `OldSolverRelating` or something, idk.

r? lcnr
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 7, 2024
…s, r=lcnr

Remove `CombineFields`

This conflicts with rust-lang#131263, but if this one lands first then perhaps rust-lang#131263 could then go ahead and remove all the branching on solver in `TypeRelating`. We could perhaps then rename `TypeRelating` to `OldSolverRelating` or something, idk.

r? lcnr
@compiler-errors
Copy link
Member Author

Now that the other PR landed, I believe this is ready

@lcnr
Copy link
Contributor

lcnr commented Oct 10, 2024

r=me after review

@lcnr
Copy link
Contributor

lcnr commented Oct 10, 2024

@bors r+ rollup=never

@bors
Copy link
Collaborator

bors commented Oct 10, 2024

📌 Commit 8715bfb has been approved by lcnr

It is now in the queue for this repository.

@bors 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 Oct 10, 2024
@bors
Copy link
Collaborator

bors commented Oct 10, 2024

⌛ Testing commit 8715bfb with merge 8d94e06...

@bors
Copy link
Collaborator

bors commented Oct 10, 2024

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 8d94e06 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 10, 2024
@bors bors merged commit 8d94e06 into rust-lang:master Oct 10, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Oct 10, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8d94e06): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
0.2% [0.2%, 0.2%] 1
Regressions ❌
(secondary)
0.5% [0.2%, 2.7%] 9
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [0.2%, 0.2%] 1

Max RSS (memory usage)

Results (primary -1.2%, secondary -0.1%)

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.4% [0.4%, 0.4%] 1
Regressions ❌
(secondary)
2.0% [1.9%, 2.1%] 3
Improvements ✅
(primary)
-1.9% [-2.4%, -1.5%] 2
Improvements ✅
(secondary)
-2.2% [-2.9%, -1.5%] 3
All ❌✅ (primary) -1.2% [-2.4%, 0.4%] 3

Cycles

Results (primary 0.7%, secondary -2.1%)

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.7% [0.7%, 0.7%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.1% [-2.1%, -2.1%] 1
All ❌✅ (primary) 0.7% [0.7%, 0.7%] 1

Binary size

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

Bootstrap: 774.326s -> 779.131s (0.62%)
Artifact size: 329.54 MiB -> 332.04 MiB (0.76%)

@rustbot rustbot added the perf-regression Performance regression. label Oct 10, 2024
@compiler-errors compiler-errors deleted the solver-relating branch October 16, 2024 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants