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.

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

Add TypeFlags for TyKind in chalk-ir #639

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 Outdated Show resolved Hide resolved
chalk-integration/src/test_macros.rs Outdated Show resolved Hide resolved
chalk-ir/src/lib.rs
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.

chalk-ir/src/lib.rs Outdated Show resolved Hide resolved
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

tests/test/type_flags.rs Show resolved Hide resolved
tests/test/type_flags.rs Outdated Show resolved Hide resolved
tests/test/type_flags.rs Outdated Show resolved Hide resolved
tests/test/type_flags.rs Outdated Show resolved Hide resolved
tests/test/type_flags.rs Outdated Show resolved Hide resolved
@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

chalk-ir/src/lib.rs Outdated Show resolved Hide resolved
@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

@matthewjasper matthewjasper matthewjasper left review comments

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