Skip to content

Generalize with variance - #673

Merged
bors merged 2 commits into
rust-lang:masterfrom
jackh726:generalize-variance
Jan 19, 2021
Merged

Generalize with variance#673
bors merged 2 commits into
rust-lang:masterfrom
jackh726:generalize-variance

Conversation

@jackh726

@jackh726 jackh726 commented Jan 5, 2021

Copy link
Copy Markdown
Member

Closes #667

} else if let Some(ty) = self.table.normalize_ty_shallow(interner, ty) {
self.generalize_ty(&ty, universe_index)
self.generalize_ty(&ty, universe_index, variance)
} else if matches!(variance, Variance::Invariant) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The rustc code here also checks that this variable can name universe_index here, although that's maybe only for error messages.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm. So should I change this here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that we can keep the simpler code for now.

Comment thread chalk-solve/src/infer/unify.rs
@matthewjasper

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Jan 19, 2021

Copy link
Copy Markdown
Contributor

📌 Commit 9bf2ba0 has been approved by matthewjasper

@bors

bors commented Jan 19, 2021

Copy link
Copy Markdown
Contributor

⌛ Testing commit 9bf2ba0 with merge 1e096f6...

@bors

bors commented Jan 19, 2021

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-actions
Approved by: matthewjasper
Pushing 1e096f6 to master...

@bors
bors merged commit 1e096f6 into rust-lang:master Jan 19, 2021
@jackh726
jackh726 deleted the generalize-variance branch January 19, 2021 23:29
@lnicola

lnicola commented Jan 20, 2021

Copy link
Copy Markdown
Member

If it's not too much of a hassle, would it be possible to publish a new version to crates.io? There's one or two rust-analyzer that this should fix and while I can use a git dependency to test, it wouldn't hurt to let it bake a little until the next release.

@jackh726

Copy link
Copy Markdown
Member Author

@lnicola just kicked off a publish

@lnicola

lnicola commented Jan 20, 2021

Copy link
Copy Markdown
Member

Note to self: we should do chalk upgrades on Monday.

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.

Endless loop in recursive solver

4 participants