[PATCH] D78249: Reland "[codeview] Reference types in type parent scopes"

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:323
+    // declaration or a complete type.
+    // Only call getTypeIndex if the type isn't already being defined.
+    if (const auto *Ty = dyn_cast<DICompositeType>(Scope)) {
----------------
Can you add a test that fails if this check is removed? You should be able to reduce one out of the crash bug Hans filed.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:325-327
+      auto I = CompleteTypeIndices.find(Ty);
+      if (I == CompleteTypeIndices.end() || I->second != TypeIndex())
+        (void)getTypeIndex(Ty);
----------------
As an alternative, I think we should be able to add this type to the DeferredCompleteTypes list. We should get the same behavior without the extra conditional.

All this type index lowering code is based on the idea that it is impossible to make a cycle in the type graph without looking through a complete struct type. i.e. you cannot have a type which points to itself. You can make a linked list that points to the struct, but that is implemented by pointing to a forward declaration.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78249/new/

https://reviews.llvm.org/D78249