akhuang 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); ---------------- dblaikie wrote: > 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) ha, good question, I copied this without really looking at it. I am also not sure when a definition would not be a complete definition. ================ 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) ---------------- dblaikie wrote: > 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) I just tried adding a verifier check to see what happens, but it looks like incomplete arrays have locations but no size? I can split out 1 into another patch. 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