[PATCH] D75215: [DebugInfo] Fix for adding "returns cxx udt" option to functions in CodeView.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:992
       getTagForRecord(RD), RDName, Ctx, DefUnit, Line, 0, Size, Align,
       llvm::DINode::FlagFwdDecl, Identifier);
   if (CGM.getCodeGenOpts().DebugFwdTemplateParams)
----------------
One alternative would be to add `llvm::DINode::FlagNonTrivial` here if the type is complete and not trivial.

There is the case where the the compiler has to emit a method type when the class has been forward declared, and that could still lead to duplicate records. For example:
```
class Returned;
class Foo {
  Foo();
  Returned bar();
};
Foo::Foo() {}
```
In this example, clang will have to describe the method type of Foo::bar, but it will not have access to Returned. I think in practice this won't come up, because Foo::bar will be defined in the same file as the constructor.


================
Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:61
 HANDLE_DI_FLAG((1 << 29), AllCallsDescribed)
+HANDLE_DI_FLAG((1 << 30), CxxReturnUdt)
 
----------------
@dblaikie @aprantl, does this seem like a reasonable flag to add, or should we mark record forward decls as trivial/nontrivial instead?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:407
 static bool isNonTrivial(const DICompositeType *DCTy) {
   return ((DCTy->getFlags() & DINode::FlagNonTrivial) == DINode::FlagNonTrivial);
 }
----------------
Should we assert here that the composite type is not a forward decl, assuming we stick with the CxxReturn flag?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:420
   // DISubroutineType is unnamed. Use DISubprogram's i.e. SPName in comparison.
   if (ClassTy && isNonTrivial(ClassTy) && SPName == ClassTy->getName()) {
     FO |= FunctionOptions::Constructor;
----------------
We also use isNonTrivial here, but I think if we are emitting a method type, it means the class must not be a forward declaration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75215