Skip to content

Add TypeFlags for TyKind in chalk-ir #639

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 11, 2020
Merged

Add TypeFlags for TyKind in chalk-ir #639

merged 3 commits into from
Dec 11, 2020

Conversation

chfont
Copy link
Contributor

@chfont chfont commented Oct 29, 2020 •
edited by jackh726
Loading

Closes #627

@bors
Copy link
Contributor

bors commented Oct 29, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

matthewjasper
matthewjasper reviewed Oct 29, 2020
View reviewed changes
chalk-ir/src/lib.rs
let ty_kind = data.cast(&interner);
let data = TyData {
kind: data.cast(interner),
flags: compute_flags(&ty_kind, &interner),
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably a large enough change to be in another PR, but I would expect the interner to compute the flags, so that in the case it is actually interning it only has to compute flags when it's allocating.

jackh726
jackh726 requested changes Nov 1, 2020
View reviewed changes
Copy link
Member

@jackh726 jackh726 left a comment

Choose a reason for hiding this comment

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

My comments can apply to the rest of the tests I didn't comment on

@jackh726
Copy link
Member

jackh726 commented Dec 9, 2020

@matthewjasper can you take another look at this? I've rebased and updated per your review.

matthewjasper
matthewjasper reviewed Dec 11, 2020
View reviewed changes
Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

r=me with comment addressed

@jackh726
Copy link
Member

@bors r=matthewjasper

@bors
Copy link
Contributor

bors commented Dec 11, 2020

📌 Commit a8b61ff has been approved by matthewjasper

@bors
Copy link
Contributor

bors commented Dec 11, 2020

⌛ Testing commit a8b61ff with merge a3a3e1e...

@jackh726 jackh726 mentioned this pull request Dec 11, 2020
Closed
@bors
Copy link
Contributor

bors commented Dec 11, 2020

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

@bors bors merged commit a3a3e1e into rust-lang:master Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jackh726 jackh726 jackh726 requested changes

+1 more reviewer

@matthewjasper matthewjasper matthewjasper left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add flags to InternedTy and InternedRegion types

4 participants

@chfont @bors @jackh726 @matthewjasper