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

generalize the builtin struct type to adt to match rustc #453

Closed
nikomatsakis opened this issue May 12, 2020 · 2 comments · Fixed by #454
Closed

generalize the builtin struct type to adt to match rustc #453

nikomatsakis opened this issue May 12, 2020 · 2 comments · Fixed by #454
Assignees
Labels
current-sprint Being worked on in the current sprint

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented May 12, 2020

Chalk currently has a notion of struct types but rustc has the more general adt. Also, chalk expects a DefId but in rustc we use an AdtDef (a pointer to an interned structure).

We should align with rustc here:

  • Rename TypeName::Struct to TypeName::Adt
  • Rename StructId<I> to AdtId<I>
  • Extend Interner trait to have I::InternedAdtId and adapt AdtId<I> to be a wrapper around InternedAdtId (instead of I::DefId, as today)
  • We can map InternedAdtId to the same type as DefId in our integration, rustc can use &'tcx AdtDef
  • Rename StructDatum and StructDatumBound to use the Adt terminology -- we don't actually have to change the list of fields for now, though eventually we may want to do so to allow rustc to more cheaply give access from the AdtDef

This issue has been assigned to @Mcat12 via this comment.

@nikomatsakis nikomatsakis added the current-sprint Being worked on in the current sprint label May 12, 2020
@AzureMarker
Copy link
Member

AzureMarker commented May 13, 2020

This seems straightforward enough, I'll give it a shot.

@rustbot claim

@rustbot
Copy link
Collaborator

rustbot commented May 13, 2020

Error: Parsing assign command in comment failed: ...'bot assign' | error: specify user to assign to at >| ''...

Please let @rust-lang/release know if you're having trouble with this bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
current-sprint Being worked on in the current sprint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants