dblaikie added inline comments.
================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1035
+ const RecordDecl *D = RD->getDefinition();
+ if (D && D->isCompleteDefinition())
+ Size = CGM.getContext().getTypeSize(Ty);
----------------
When is a definition not a complete definition? (sounds like a line from Alice
in Wonderland... but I'm genuinely curious/not sure I understand)
================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4537-4538
auto VarSize = DbgDeclare->getVariable()->getSizeInBits();
- if (VarSize) {
- if (Size > *VarSize)
- Size = *VarSize;
- if (Size == 0 || Start + Size > *VarSize)
- continue;
- }
+ assert(VarSize &&
+ "Any variable with a location must have a type with a size");
+ if (Size > *VarSize)
----------------
This is probably a sligthly larger/separate project/patch - @aprantl would it
be reasonable to add this as an LLVM IR debug info metadata invariant/tested in
the debug info verifier?
(in terms of patch order, I guess:
1) ignoring the size, if present, on declarations in the backend
2) adding the size in the frontend
3) adding an invariant/verifier check that the size is always specified on
types of variables with locations - and changing this code here to assert
rather than test
But happy to review at least 1 and 2 in this review, though they could/should
still be committed separately - the third's probably involved enough to merit a
separate review, I'd hazard a guess)
================
Comment at: llvm/test/DebugInfo/X86/struct-fwd-decl.ll:19
+; CHECK-NOT: DW_AT_byte_size
+; CHECK-NEXT: DW_AT_declaration
+!6 = !{!0}
----------------
Probably CHECK-NOT DW_AT_byte_size again, and CHECK: DW_TAG so that even if the
fields are reordered a byte_size can't slip in anywhere.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87062/new/
https://reviews.llvm.org/D87062
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
