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

Thu Mar 5 11:32:14 PST 2020

akhuang marked an inline comment as done and an inline comment as not done.
akhuang added inline comments.

================
Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:61
 HANDLE_DI_FLAG((1 << 29), AllCallsDescribed)
+HANDLE_DI_FLAG((1 << 30), CxxReturnUdt)

----------------
akhuang wrote:
> rnk wrote:
> > aprantl wrote:
> > > dblaikie wrote:
> > > > rnk wrote:
> > > > > @dblaikie @aprantl, does this seem like a reasonable flag to add, or should we mark record forward decls as trivial/nontrivial instead?
> > > > Currently we only have a trivial/nontrivial flag that goes one way, right? (ie: true/false, not three state true/false/unknown)
> > > > 
> > > > That would present a problem for forward declarations - because for a true forward decl you can't know if it's trivial/non-trivial for passing, right? so that'd present a subtle difference between trivial/non-trivial on a decl (where it might be trivial/unknown) and on a def (where it's trivial/non-trivial), yes?
> > > Should this perhaps be a DI_SPFLAG instead?
> > It's true that there is an ambiguity between known trivial, known non-trivial, and forward declared, unknown triviality. Amy's solution to this problem was to mark forward declarations as nontrivial, which matches the logic MSVC uses to set CxxReturnUdt, but might not be the right thing for other consumers.
> Alternatively, I guess we can FlagNonTrivial to FlagTrivial, and then we can mark all the unknown things with cxxreturnudt.
maybe this doesn't work because then it would also mark C structs with cxxreturnudt.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75215