-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Support ADT types in type info reflection #151142
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
base: main
Are you sure you want to change the base?
Conversation
Support {bool,char,int,uint,float,str} primitive types for feature
`type_info` reflection.
|
|
1bd8449 to
0b20893
Compare
| sym::name => { | ||
| if let Some(name) = name { | ||
| let name_place = self.allocate_str_dedup(name.as_str())?; | ||
| let ptr = self.mplace_to_ref(&name_place)?; | ||
| self.write_immediate(*ptr, &field_place)? | ||
| } else { | ||
| let (variant, _) = self.downcast(&field_place, sym::None)?; | ||
| self.write_discriminant(variant, &field_place)?; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like I should write sym::Some discriminant for name? 🤔 Writing ptr directly seems to work, but this relies on how Option<&'static str> is underlyingly represented IIUC.
But const eval interpreter doesn't complain about this -- perhaps it should?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the interpreter doesn't care if the value is exactly the same as if it had its discriminant written. There is some talk about SetDiscriminant needing to happen always, but it's not really important for us here, this will be fixed by whoever changes it
also irrelevant if we make the field just always have a name (rendering the index)
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| #[non_exhaustive] | ||
| #[unstable(feature = "type_info", issue = "146922")] | ||
| pub struct Field { | ||
| /// The name of the struct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the field's name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also make it non-optional and produce integer names for tuples and tuple structs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that was a typo.
could also make it non-optional and produce integer names for tuples and tuple structs
Good idea.
| sym::name => { | ||
| if let Some(name) = name { | ||
| let name_place = self.allocate_str_dedup(name.as_str())?; | ||
| let ptr = self.mplace_to_ref(&name_place)?; | ||
| self.write_immediate(*ptr, &field_place)? | ||
| } else { | ||
| let (variant, _) = self.downcast(&field_place, sym::None)?; | ||
| self.write_discriminant(variant, &field_place)?; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the interpreter doesn't care if the value is exactly the same as if it had its discriminant written. There is some talk about SetDiscriminant needing to happen always, but it's not really important for us here, this will be fixed by whoever changes it
also irrelevant if we make the field just always have a name (rendering the index)
|
☔ The latest upstream changes (presumably #151210) made this pull request unmergeable. Please resolve the merge conflicts. |
Tracking issue: #146922
#![feature(type_info)]This PR supports ADT types for feature
type_inforeflection.(It's still a draft PR, with implementation in progress)
Note that this PR does not take SemVer into consideration (I left a FIXME comment). As discussed earlier (comment), this requires further discussion. However, I hope we could get an initial implementation to land first, so we can start playing with it.
Progress / Checklist
mainbranch(It's currently based on PR Support primitives in type info reflection #151123, so here's an extra commit)
r? @oli-obk