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

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I think we should commit the change to clang separately from the first one you made to LLVM to handle method types. Setting flags on forward declarations is something I'd like to get additional input on before landing.



================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:989
+  llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
+  if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD))
+    if (!CXXRD->hasDefinition() ||
----------------
I think this code block needs a comment about the behavior we are trying to implement. It could be made CodeView-specific, but I don't see a strong reason not to set these flags when emitting DWARF.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:990
+  if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD))
+    if (!CXXRD->hasDefinition() ||
+        (CXXRD->hasDefinition() && !CXXRD->isTrivial()))
----------------
So, we're marking forward declarations here as nontrivial to match MSVC, but we don't really know if they will end up being trivial or not. I guess that's fine.


================
Comment at: clang/test/CodeGenCXX/debug-info-flag-nontrivial.cpp:2
+// RUN: %clang_cc1 -emit-llvm -gcodeview -debug-info-kind=limited %s -o - | FileCheck %s
+// Test for CxxReturnUdt flags in debug info.
+
----------------
Please add a C-only test like this:
```
struct Incomplete;
struct Incomplete (*func_ptr)() = 0;
```
We shouldn't mark this struct as nontrivial, for example. The code you wrote is correct, but I wanted to test this corner case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75215