[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
                                LValue RecordLV, CharUnits Align,
-                               llvm::FunctionCallee Func, int Lvl) {
+                               bool dumpTypeName, llvm::FunctionCallee Func,
+                               int Lvl) {
----------------
yihanaa wrote:
> erichkeane wrote:
> > yihanaa wrote:
> > > erichkeane wrote:
> > > > Not a fan of bool parameters, they make calls harder to read.
> > > I can add comments for this parameter or this function, what do you think, or do you have a better idea? I would like to improve it.
> > I prefer using an 'enum' for these sorts of things, we do that elsewhere.  It seems that what you ACTUALLY care about here is whether this is a 'top level', and thus has a FieldName to print, right? So you actually want to dumpFieldName?  Or am I still misunderstanding something?
> > I prefer using an 'enum' for these sorts of things, we do that elsewhere.  It seems that what you ACTUALLY care about here is whether this is a 'top level', and thus has a FieldName to print, right? So you actually want to dumpFieldName?  Or am I still misunderstanding something?
> 
> Yeah, you are right. A little  different is that this patch will DO NOT dump the typename, field name and '=' in recursive dumpRecord call. 
Yeah, you are right. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920