Track perf regression "Branch Clause from Predicate #104846"

[spastorino]

A perf regression was introduced by #104846, #104846 (comment)

Right now we are moving (moderately large) Clauses around. What we often do in the compiler is to wrap such types in a newtype and an Interned (see ty::Ty, ty::Predicate and ty::Const) for examples. So what we could do is do the same thing with Clause. It does mean every time we match on it we need to add and call something like the kind method on Ty and Predicate to get at the inner Clause. There's also some naming issues, as you can't have both the interned type and the inner type be called ty::Clause. One solution could be to publicly reexport all Clause variants from the clause module, so we can write clause::Trait instead of Clause::Trait, and then rename the current Clause enum to ClauseKind. After that the Clause name is free again for use by the wrapper type.

each of these steps could be a separate PR, but should at least be a separate commit for easy review and rebases.

The plan again in a more structured manner (where each step is a commit):

• (E-easy) reexport Clause's variants from the clause module and change all uses of Clause::Foo to clause::Foo and import the clause module where necessary.
• (E-medium) rename Clause to ClauseKind and add the wrapper type called Clause, add Clause to the direct_interners! macro invocation, use mk_clause at all sites that create Clauses, add a kind method to the new Clause type, call kind at each site that matches on ClauseKind variants.

[spastorino]

r? @spastorino

cc @oli-obk

[sladyn98]

Just to confirm, #104846 (comment) Looking at this comment what do we mean by intern clause @spastorino, I am still getting familiarized with rust and learning it as I go on, getting some pointers would be really beneficial. Thanks

[oli-obk]

Right now we are moving (moderately large) Clauses around. What we often do in the compiler is to wrap such types in a newtype and an Interned (see ty::Ty, ty::Predicate and ty::Const) for examples. So what we could do is do the same thing with Clause. It does mean every time we match on it we need to add and call something like the kind method on Ty and Predicate to get at the inner Clause. There's also some naming issues, as you can't have both the interned type and the inner type be called ty::Clause. One solution could be to publicly reexport all Clause variants from the clause module, so we can write clause::Trait instead of Clause::Trait, and then rename the current Clause enum to ClauseKind. After that the Clause name is free again for use by the wrapper type.

each of these steps could be a separate PR, but should at least be a separate commit for easy review and rebases.

The plan again in a more structured manner (where each step is a commit):

1. reexport Clause's variants from the clause module and change all uses of Clause::Foo to clause::Foo and import the clause module where necessary.
2. rename Clause to ClauseKind and add the wrapper type called Clause, add Clause to the direct_interners! macro invocation, use mk_clause at all sites that create Clauses, add a kind method to the new Clause type, call kind at each site that matches on ClauseKind variants.

[sladyn98]

Yeah, I can try implementing it.

[lcnr]

reexport Clause's variants from the clause module and change all uses of Clause::Foo to clause::Foo and import the clause module where necessary.

I disagree with this step. We don't do that for constants or predicates and I believe that doing it for regions and types mostly happens due to historical reasons

[oli-obk]

I disagree with this step. We don't do that for constants or predicates and I believe that doing it for regions and types mostly happens due to historical reasons

I mostly dislike using ClauseKind everywhere. Since the interned clause type won't show up a lot, we could also just call it InternedClause and keep Clause as the name for the enum.