Remove ty_to_def_id
#52381
Merged
Remove ty_to_def_id
#52381
Conversation
This file contains 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
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
eddyb
reviewed
Jul 14, 2018
| !field.is_positional() | ||
| && !self.symbol_is_live(field.id, None) | ||
| && !is_marker_field | ||
| && !field_type.is_phantom_data() |
There was a problem hiding this comment.
Heh, are the only two lang item types, PhantomData and Box?
There was a problem hiding this comment.
Ugh. I don't see why Box should be special. Depending on the lang_itemnes of a type seems rather fragile.
There was a problem hiding this comment.
I agree it shouldn't be, but once upon a time there were many lang items, one for each of "{co,contra,in}variant {lifetime,type}" (so 6 at least), so maybe that's where it's leftover from.
I'm just amused only two types are left and the Box case actually hides some warnings!
eddyb
reviewed
Jul 14, 2018
| @@ -399,6 +399,9 @@ impl CodegenContext { | |||
| } | |||
|
|
|||
| struct DiagnosticHandlers<'a> { | |||
| #[allow(dead_code)] | |||
| // This value is not actually dead, llcx has pointers to it and needs these pointers to be alive | |||
| // until Drop is executed on this object | |||
| inner: Box<(&'a CodegenContext, &'a Handler)>, | |||
There was a problem hiding this comment.
Do you mean the heap allocation itself? This is really confusing, maybe a raw pointer should be kept instead, with a manual Drop impl that uses Box::from_raw.
There was a problem hiding this comment.
Also maybe the comment should be phrased to be about the callback having a copy of the pointer or something. It's not really "self-referential" insofar as much as it is scoping a resource for the duration C++ needs it.
|
@bors r+ |
|
📌 Commit ecab96f has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - status-appveyor, 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.
fixes #52341
The uses were mostly convenience and generally "too powerful" (would also have worked for types that weren't interesting at the use site)
r? @eddyb