Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Structurally normalize weak and inherent in new solver #114594

Merged
merged 2 commits into from
Aug 8, 2023

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Aug 7, 2023

It seems pretty obvious to me that we should be normalizing weak and inherent aliases too, since they can always be normalized. This PR still leaves open the question of what to do with opaques, though 💀

Also, we need to structurally resolve the target of a coercion, for the UI test to work.

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. labels Aug 7, 2023
@compiler-errors
Copy link
Member Author

I thought I already had this included somewhere in some other PR, but I may have rebased it out by accident. Maybe I totally forgor and this is already up in some other PR.

@lcnr lcnr added the WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) label Aug 7, 2023
@lcnr
Copy link
Contributor

lcnr commented Aug 7, 2023

@bors r+ rollup (should only affect new solver)

@bors
Copy link
Contributor

bors commented Aug 7, 2023

📌 Commit ba4a2f7 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 Aug 7, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 8, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#114376 (Avoid exporting __rust_alloc_error_handler_should_panic more than once.)
 - rust-lang#114413 (Warn when #[macro_export] is applied on decl macros)
 - rust-lang#114497 (Revert rust-lang#98333 "Re-enable atomic loads and stores for all RISC-V targets")
 - rust-lang#114500 (Remove arm crypto target feature)
 - rust-lang#114566 (Store the laziness of type aliases in their `DefKind`)
 - rust-lang#114594 (Structurally normalize weak and inherent in new solver)
 - rust-lang#114596 (Rename method in `opt-dist`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 418b91a into rust-lang:master Aug 8, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 8, 2023
@@ -1012,6 +1012,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
cause: Option<ObligationCause<'tcx>>,
) -> RelateResult<'tcx, Ty<'tcx>> {
let source = self.try_structurally_resolve_type(expr.span, expr_ty);
let target = self.try_structurally_resolve_type(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it back -- @lqd: this line may have changed the order we process obligations, re: perf #114604.

I'd be surprised if this had a meaningful effect on perf, tho.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll check if cachegrind has anything interesting to say

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also just getting rid of this one line and running perf in #114648.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is the culprit, I guess I may be able to work around it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Processing obligations seems to be the main source of change, but hopefully we'll learn more in the new PR if it's not some kind of noise.

260,711,737  ???:<rustc_data_structures::obligation_forest::ObligationForest<rustc_trait_selection::traits::fulfill::PendingPredicateObligation>>::process_obligations::<rustc_trait_selection::traits::fulfill::FulfillProcessor>

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 10, 2023
Only resolve target type in `try_coerce` in new solver

Only needed in new solver, seems to affect perf in old solver.

cc rust-lang#114604/rust-lang#114594
@compiler-errors compiler-errors deleted the new-solver-resolve-aliases branch August 11, 2023 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants