[DebugInfo] Add size to class declarations in debug info.

[akhuang]

Summary

This adds the size to forward declared class DITypes, if the size is known.

Fixes an issue where we determine whether to emit fragments based on the
type size, so fragments would sometimes be incorrectly emitted if there
was no size.

Bug: https://bugs.llvm.org/show_bug.cgi?id=47338

[dblaikie]

clang/lib/CodeGen/CGDebugInfo.cpp

1035

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)

llvm/lib/Transforms/Scalar/SROA.cpp

4537–4538 ↗

(On Diff #289596)

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)

llvm/test/DebugInfo/X86/struct-fwd-decl.ll

19 ↗

(On Diff #289596)

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.

[akhuang]

clang/lib/CodeGen/CGDebugInfo.cpp

1035

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.

llvm/lib/Transforms/Scalar/SROA.cpp

4537–4538 ↗

(On Diff #289596)

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.

[akhuang]

remove assert; edit test case

[akhuang]

rebase

[dblaikie]

Looks good, thanks!

[akhuang]

clang/lib/CodeGen/CGDebugInfo.cpp

1035

Oh, I guess incomplete definition means it's in the process of being defined.

clang/lib/CodeGen/CGDebugInfo.cpp

if (llvm::DIType *T = getTypeOrNull(CGM.getContext().getRecordType(RD)))
     return cast<llvm::DICompositeType>(T);
   llvm::DIFile *DefUnit = getOrCreateFile(RD->getLocation());
   unsigned Line = getLineNumber(RD->getLocation());
   StringRef RDName = getClassName(RD);
 
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+    Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,
   // add the flag if a record has no definition because we don't know whether
   // it will be trivial or not.
   if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD))
     if (!CXXRD->hasDefinition() ||
         (CXXRD->hasDefinition() && !CXXRD->isTrivial()))

clang/test/CodeGenCXX/debug-info-class.cpp

 // CHECK: [[C_VPTR]] = !DIDerivedType(tag: DW_TAG_member, name: "_vptr$C"
 // CHECK-SAME:                        DIFlagArtificial
 // CHECK: [[C_S]] = !DIDerivedType(tag: DW_TAG_member, name: "s"
 // CHECK-SAME:                     baseType: ![[INT]]
 // CHECK-SAME:                     DIFlagStaticMember
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "D"
-// CHECK-NOT:              size:
+// CHECK-SAME:             size:
 // CHECK-SAME:             DIFlagFwdDecl
 // CHECK-NOT:              identifier:
 // CHECK-SAME:             ){{$}}
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "E"
 // CHECK-SAME:             DIFlagFwdDecl
 // CHECK-NOT:              identifier:
 // CHECK-SAME:             ){{$}}