Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind` #112772

compiler-errors · 2023-06-18T19:57:03Z

Does two basic things before I put up a more delicate set of PRs (along the lines of #112714, but hopefully much cleaner) that migrate existing usages of ty::Predicate to ty::Clause (predicates_of/item_bounds/ParamEnv::caller_bounds).

Rename Clause to ClauseKind, so it's parallel with PredicateKind.
Add a new Clause type which is parallel to Predicate.
- This type exposes Clause::kind(self) -> Binder<'tcx, ClauseKind<'tcx>> which is parallel to Predicate::kind 😸

The new Clause type essentially acts as a newtype wrapper around Predicate that asserts that it is specifically a PredicateKind::Clause. Turns out from experimentation¹ that this is not negative performance-wise, which is wonderful, since this a much simpler design than something that requires encoding the discriminant into the alignment bits of a predicate kind, or something else like that...

r? @lcnr or @oli-obk

https://github.com/rust-lang/rust/pull/112714#issuecomment-1595653910 ↩

rustbot · 2023-06-18T19:57:11Z

Some changes occurred in const_evaluatable.rs

cc @BoxyUwU

Some changes occurred to the core trait solver

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

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

lcnr

r=me after nits

lcnr · 2023-06-19T07:09:40Z

compiler/rustc_middle/src/ty/mod.rs

        }
    }
+
+    pub fn as_clause_unchecked(self) -> Clause<'tcx> {
+        debug_assert!(matches!(self.kind().skip_binder(), PredicateKind::Clause(_)));


this assert is perf critical? and assuming that it is, would it not be faster to make this an assert and make using the clause use unreachable_unchecked?

I don't think it's perf critical, just seems redundant since we already assert that it's a Clause in Clause::kind.

Anyways I made it an assert, and we can make it unreachable_unchecked on the read path if perf warrants it later.

oli-obk · 2023-06-19T09:30:10Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

-            if let ty::PredicateKind::Clause(ty::Clause::Trait(pred)) = pred.kind().skip_binder()
+            if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) = pred.kind().skip_binder()


What's your opinion on reexporting the ClauseKind variants from a clause module and using clause::Trait instead?

Eh, doesn't really seem worthwhile tbh. Or at least, I think I want to do it as a follow-up 😸

bors · 2023-06-19T14:39:16Z

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

oli-obk · 2023-06-19T17:59:55Z

@bors r=lcnr

bors · 2023-06-19T17:59:58Z

📌 Commit dcee3ab has been approved by lcnr

It is now in the queue for this repository.

Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind` Does two basic things before I put up a more delicate set of PRs (along the lines of rust-lang#112714, but hopefully much cleaner) that migrate existing usages of `ty::Predicate` to `ty::Clause` (`predicates_of`/`item_bounds`/`ParamEnv::caller_bounds`). 1. Rename `Clause` to `ClauseKind`, so it's parallel with `PredicateKind`. 2. Add a new `Clause` type which is parallel to `Predicate`. * This type exposes `Clause::kind(self) -> Binder<'tcx, ClauseKind<'tcx>>` which is parallel to `Predicate::kind` 😸 The new `Clause` type essentially acts as a newtype wrapper around `Predicate` that asserts that it is specifically a `PredicateKind::Clause`. Turns out from experimentation[^1] that this is not negative performance-wise, which is wonderful, since this a much simpler design than something that requires encoding the discriminant into the alignment bits of a predicate kind, or something else like that... r? `@lcnr` or `@oli-obk` [^1]: rust-lang#112714 (comment)

Rollup of 6 pull requests Successful merges: - rust-lang#112632 (Implement PartialOrd for `Vec`s over different allocators) - rust-lang#112759 (Make closure_saved_names_of_captured_variables a query. ) - rust-lang#112772 (Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind`) - rust-lang#112790 (Syntactically accept `become` expressions (explicit tail calls experiment)) - rust-lang#112830 (More codegen cleanups) - rust-lang#112844 (Add retag in MIR transform: `Adt` for `Unique` may contain a reference) r? `@ghost` `@rustbot` modify labels: rollup

Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind` Does two basic things before I put up a more delicate set of PRs (along the lines of rust-lang#112714, but hopefully much cleaner) that migrate existing usages of `ty::Predicate` to `ty::Clause` (`predicates_of`/`item_bounds`/`ParamEnv::caller_bounds`). 1. Rename `Clause` to `ClauseKind`, so it's parallel with `PredicateKind`. 2. Add a new `Clause` type which is parallel to `Predicate`. * This type exposes `Clause::kind(self) -> Binder<'tcx, ClauseKind<'tcx>>` which is parallel to `Predicate::kind` 😸 The new `Clause` type essentially acts as a newtype wrapper around `Predicate` that asserts that it is specifically a `PredicateKind::Clause`. Turns out from experimentation[^1] that this is not negative performance-wise, which is wonderful, since this a much simpler design than something that requires encoding the discriminant into the alignment bits of a predicate kind, or something else like that... r? ``@lcnr`` or ``@oli-obk`` [^1]: rust-lang#112714 (comment)

Rollup of 6 pull requests Successful merges: - rust-lang#112632 (Implement PartialOrd for `Vec`s over different allocators) - rust-lang#112759 (Make closure_saved_names_of_captured_variables a query. ) - rust-lang#112772 (Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind`) - rust-lang#112790 (Syntactically accept `become` expressions (explicit tail calls experiment)) - rust-lang#112830 (More codegen cleanups) - rust-lang#112844 (Add retag in MIR transform: `Adt` for `Unique` may contain a reference) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned lcnr Jun 18, 2023

lcnr reviewed Jun 19, 2023

View reviewed changes

oli-obk reviewed Jun 19, 2023

View reviewed changes

s/Clause/ClauseKind

fca56a8

compiler-errors force-pushed the clauses-1 branch from 3f05556 to 2395086 Compare June 19, 2023 15:44

compiler-errors added 2 commits June 19, 2023 15:46

Fully fledged Clause type

21226ee

doc

dcee3ab

compiler-errors force-pushed the clauses-1 branch from 2395086 to dcee3ab Compare June 19, 2023 15:46

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

compiler-errors mentioned this pull request Jun 20, 2023

Rollup of 6 pull requests #112851

Closed

Noratrieb mentioned this pull request Jun 21, 2023

Rollup of 6 pull requests #112877

Merged

bors merged commit a98c14f into rust-lang:master Jun 21, 2023

rustbot added this to the 1.72.0 milestone Jun 21, 2023

compiler-errors deleted the clauses-1 branch August 11, 2023 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind` #112772

Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind` #112772

compiler-errors commented Jun 18, 2023 •

edited

Loading

rustbot commented Jun 18, 2023

lcnr left a comment

lcnr Jun 19, 2023

compiler-errors Jun 19, 2023

compiler-errors Jun 19, 2023

oli-obk Jun 19, 2023

compiler-errors Jun 19, 2023

bors commented Jun 19, 2023

oli-obk commented Jun 19, 2023

bors commented Jun 19, 2023

		if let ty::PredicateKind::Clause(ty::Clause::Trait(pred)) = pred.kind().skip_binder()
		if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) = pred.kind().skip_binder()

Add a fully fledged Clause type, rename old Clause to ClauseKind #112772

Add a fully fledged Clause type, rename old Clause to ClauseKind #112772

Conversation

compiler-errors commented Jun 18, 2023 • edited Loading

Footnotes

rustbot commented Jun 18, 2023

lcnr left a comment

Choose a reason for hiding this comment

lcnr Jun 19, 2023

Choose a reason for hiding this comment

compiler-errors Jun 19, 2023

Choose a reason for hiding this comment

compiler-errors Jun 19, 2023

Choose a reason for hiding this comment

oli-obk Jun 19, 2023

Choose a reason for hiding this comment

compiler-errors Jun 19, 2023

Choose a reason for hiding this comment

bors commented Jun 19, 2023

oli-obk commented Jun 19, 2023

bors commented Jun 19, 2023

Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind` #112772

Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind` #112772

compiler-errors commented Jun 18, 2023 •

edited

Loading