-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix unsized coercion of a struct element #1880
Conversation
Fix issues with how we are generating code for casting (model-checking#566, and model-checking#1528). Restructure the unsize casting to be done in one pass instead with deep recursion (model-checking#1531). This also reuses the code from the reachability analysis, so we don't have to keep two ways of traversing the same structure.
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.
minor comments
let mut field_type = src_goto_expr; | ||
CoerceUnsizedIterator::new(self.tcx, src_mir_type, dst_mir_type) | ||
.map(|info| { | ||
let expr = if let Some(field_symbol) = info.field { |
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.
What does it mean if info doesn't have a field?
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.
It means we are no longer traversing a structure. I.e.: we found the underlying pointer.
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 added some comments to see if it helps.
let dst_field_exprs = src_field_exprs | ||
.into_iter() | ||
.map(|(key, val)| { | ||
let new_val = if info.field.unwrap().as_str().intern() == key { |
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.
Does this potentially intern strings we didn't need to?
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 don't think so. Don't we intern the name of every field for every type already?
Description of changes:
Fix issues with how we are generating code for casting (#566, and #1528).
Restructure the unsize casting to be done in one pass instead with deep recursion (#1531). This also reuses the code from the reachability analysis, so we don't have to keep two ways of traversing the same structure.
Resolved issues:
Fixes #566
Fixes #1528
Fixes #1531
Related RFC:
Call-outs:
Testing:
How is this change tested? New tests
Is this a refactor change? Yes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.