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

Fix type equality for dyn Trait #4021

Merged
merged 1 commit into from
Apr 17, 2020
Merged

Conversation

flodiebold
Copy link
Member

Fixes a lot of false type mismatches.

(And as always when touching the unification code, I have to say I'm looking forward to replacing it by Chalk's...)

Fixes a lot of false type mismatches.

(And as always when touching the unification code, I have to say I'm looking
forward to replacing it by Chalk's...)
@matklad
Copy link
Member

matklad commented Apr 17, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 17, 2020

@bors bors bot merged commit 8a4ceba into rust-lang:master Apr 17, 2020
@bjorn3
Copy link
Member

bjorn3 commented Apr 17, 2020

Fixes a lot of false type mismatches.

How much?

@edwin0cheng
Copy link
Member

edwin0cheng commented Apr 17, 2020

From:

Project Name Mismatches
cargo 318
clap-rs 236
cranelift-codegen 651
futures 20
helloworld 0
html5ever 65
hyper 381
syn 95

To:

Project Name Mismatches
cargo 296
clap-rs 222
cranelift-codegen 469
futures 20
helloworld 0
html5ever 61
hyper 381
syn 95

https://edwin0cheng.github.io/github-action-usage-test/

@flodiebold flodiebold deleted the dyn-trait-unify branch April 18, 2020 09:16
@flodiebold
Copy link
Member Author

In rust-analyzer itself the difference was even bigger, since we use trait objects a lot for the database.

@matklad
Copy link
Member

matklad commented Apr 18, 2020

Still seems like something is missing:

image

image

@flodiebold
Copy link
Member Author

Chalk currently doesn't consider dyn Trait to implement the supertraits. Apparently there's some complicated soundness problems: rust-lang/chalk#203 But maybe they're amenable to a simple implementation as a starting point 🤔

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.

4 participants