Skip to content

Conversation

@flodiebold
Copy link
Member

Quite a few changes, because Chalk got rid of the ApplicationTy nesting.

Quite a few changes, because Chalk got rid of the `ApplicationTy` nesting.
@lnicola
Copy link
Member

lnicola commented Oct 30, 2020

LGTM, but take that with a grain of salt.

Unrelated, is the plan still to move to chalk types and does anything block that?

@flodiebold
Copy link
Member Author

Yeah. My plan is to first finish the "resolved types" IR that I've been working on for a while now; then the current Ty should mostly only be used in inference, and can be replaced by Chalk types there. At least that's the idea 😄

@flodiebold
Copy link
Member Author

I'll take the LGTM as a

bors r+

😉

@bors
Copy link
Contributor

bors bot commented Oct 30, 2020

@bors bors bot merged commit 590fdba into rust-lang:master Oct 30, 2020
@flodiebold flodiebold deleted the chalk-36 branch October 30, 2020 20:22
@jackh726
Copy link
Member

jackh726 commented Nov 9, 2020

@flodiebold do we know if there was a perf diff here?

@lnicola
Copy link
Member

lnicola commented Nov 9, 2020

There's https://rust-analyzer.github.io/metrics/, but I haven't dug up the right commit to check (590fdba).

It seems a bit inconclusive:

image

@jackh726
Copy link
Member

jackh726 commented Nov 9, 2020

Hmm okay. Was just a bit curious if the reduced substitutions we use in chalk now helped anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants