Derive debug and opaque types #847
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
|
|
|
I think I may need to update some libclang version specific tests once I get word back from Travis CI... |
emilio
approved these changes
Jul 24, 2017
src/ir/analysis/derive_debug.rs
Outdated
| let layout_can_derive = ty.layout(self.ctx).map_or(true, |l| { | ||
| l.opaque().can_trivially_derive_debug(self.ctx, ()) | ||
| }); | ||
| if layout_can_derive { |
| }; | ||
|
|
||
| // This should not end up deriving Debug because its `mBlah` field cannot derive | ||
| // Debug because the instantiation's definition cannot derive Debug. | ||
| class ContainsOpaqueTemplate { |
There was a problem hiding this comment.
Part of the point of the test is that we derived debug for the instantiation if we could, could you add that back, maybe as another different test-case?
| /* automatically generated by rust-bindgen */ | ||
|
|
||
|
|
||
| #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] |
There was a problem hiding this comment.
Oh, sure, disregard my previous comment :-)
There are lots of different ways that a type can end up being opaque, and it is best to handle all the ways in one fell swoop, rather than check each different ways (for example, if a struct has non-type template params, if a template instantiation's template definition has them, etc...) for each different type kind.
Useful for debugging when constraints go wrong.
And also not just fields, but also base members.
fitzgen
force-pushed
the
derive-debug-opaque-types
branch
from
162a34c to
684fc38
Compare
|
@bors-servo r=emilio Thanks for review :) |
|
📌 Commit 684fc38 has been approved by |
bors-servo
pushed a commit
that referenced
this pull request
Jul 24, 2017
Derive debug and opaque types See each commit for details. Follow up to #842. r? @emilio or @Manishearth
|
☀️ Test successful - status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See each commit for details.
Follow up to #842.
r? @emilio or @Manishearth