Explicit vtable pointer refactor #1084
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
emilio
approved these changes
Oct 21, 2017
Contributor
emilio
left a comment
emilio
left a comment
There was a problem hiding this comment.
r=me, sorry for the lag getting to this :(
src/ir/analysis/has_vtable.rs
Outdated
|
|
||
| TypeKind::Comp(ref info) => { | ||
| trace!(" comp considers its own methods and bases"); | ||
| let mut result = HasVtableResult::default(); |
Contributor
There was a problem hiding this comment.
I think it's clearer and shorter if you use No here directly instead of default.
We didn't previously handle this case until the `HasVtableAnalysis` was updated to determine which items have explicit vtable pointers.
fitzgen
force-pushed
the
explicit-vtable-pointer-refactor
branch
from
28ebbc6 to
6f87f0b
Compare
Member
Author
|
Addressed feedback and rebased -- thanks @emilio ! @bors-servo r+ |
|
📌 Commit 6f87f0b has been approved by |
bors-servo
pushed a commit
that referenced
this pull request
Oct 23, 2017
…zgen Explicit vtable pointer refactor r? @emilio This should make it easier to move padding into its own pass, rather than inside codegen, which should in turn let us start handling (or at least intelligently determining when we *can't* handle) `#pragma pack(..)` and other things that affect layout in exotic ways that we can only indirectly observe. See each commit for details. The reason for the first commit is this: when we compare, we rustfmt both expected and actual, so the expectations don't get updated to be formatted nicely until some patch that changes what gets generated. This is annoying, however, when debugging some minor difference, and not being able to see what it is easily. Best to just bite the bullet and format all the expectations the once and make the problem go away.
|
☀️ Test successful - status-travis |
This was referenced Oct 23, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @emilio
This should make it easier to move padding into its own pass, rather than inside codegen, which should in turn let us start handling (or at least intelligently determining when we can't handle)
#pragma pack(..)and other things that affect layout in exotic ways that we can only indirectly observe.See each commit for details.
The reason for the first commit is this: when we compare, we rustfmt both expected and actual, so the expectations don't get updated to be formatted nicely until some patch that changes what gets generated. This is annoying, however, when debugging some minor difference, and not being able to see what it is easily. Best to just bite the bullet and format all the expectations the once and make the problem go away.