analysis: Account for template instantiations of opaque types in the derive debug analysis. #842
Conversation
|
|
…derive debug analysis. We have a special-case for them in codegen to generate a blob, that can derive debug. This is a regression from rust-lang#824, and hit stylo.
emilio
force-pushed
the
derive-debug-opaque-inst
branch
from
7e80988 to
bff026c
Compare
|
r? @fitzgen |
|
@bors-servo r+ |
|
📌 Commit bff026c has been approved by |
analysis: Account for template instantiations of opaque types in the derive debug analysis. We have a special-case for them in codegen to generate a blob, that can derive debug. This is a regression from #824, and hit stylo.
|
☀️ Test successful - status-travis |
|
This wasn't enough to fix the bug. I poked around and it seems like when handling the It also seems like we generate a The following diff fixes it but I'm not really sure if that's the correct solution. diff --git a/src/ir/analysis/derive_debug.rs b/src/ir/analysis/derive_debug.rs
index b9b0be1..57ac6df 100644
--- a/src/ir/analysis/derive_debug.rs
+++ b/src/ir/analysis/derive_debug.rs
@@ -226,7 +226,18 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
.any(|f| {
match *f {
Field::DataMember(ref data) => {
- self.cannot_derive_debug.contains(&data.ty())
+ if self.cannot_derive_debug.contains(&data.ty()) {
+ let item = self.ctx.resolve_item(data.ty());
+ if item.is_opaque(self.ctx, &()) {
+ if let Some(layout) = item.as_type().and_then(|t| t.layout(&self.ctx)) {
+
+ return !layout.opaque().can_trivially_derive_debug(&self.ctx, ());
+ }
+ }
+ true
+ } else {
+ false
+ }
}
Field::Bitfields(ref bfu) => {
bfu.bitfields()Thoughts? |
| fn default() -> Self { unsafe { ::std::mem::zeroed() } } | ||
| } | ||
| #[repr(C)] | ||
| #[derive(Debug, Default, Copy)] |
There was a problem hiding this comment.
The whole point was to ensure that this doesn't derive debug, right?
| template<typename T> | ||
| class OpaqueTemplate { | ||
| T mData; | ||
| bool mCannotDebug[40]; |
There was a problem hiding this comment.
I think instantiating with T = bool so that alignment is lower, or replace bool mCabnnotDebug[40] with unsigned long long mCannotDebug[40] should do the trick.
This is close, but not quite exactly what I'd write. I think we want to push this I'll make a PR in a bit. |
Derive debug and opaque types See each commit for details. Follow up to #842. r? @emilio or @Manishearth
We have a special-case for them in codegen to generate a blob, that can derive
debug.
This is a regression from #824, and hit stylo.