yihanaa added inline comments.

================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2046-2048
 static llvm::Value *dumpRecord(CodeGenFunction &CGF, QualType RType,
                                LValue RecordLV, CharUnits Align,
+                               bool dumpTypeName, llvm::FunctionCallee Func,
----------------
rsmith wrote:
> Instead of passing a flag that suppresses the first section of this function, 
> it would be clearer to split this up into two parts:
> 
> 1) A function that dumps a type name (`struct A` or `int` or whatever).
> 2) A function that dumps a struct value (`{x = 1, y = 2}`).
> 
> Then when printing the overall value, you can call (1) then (2), and when 
> printing each field, you can call (1), then print the field name, then call 
> (2) for a struct value (and do whatever else makes sense for other kinds of 
> values).
> Instead of passing a flag that suppresses the first section of this function, 
> it would be clearer to split this up into two parts:
> 
> 1) A function that dumps a type name (`struct A` or `int` or whatever).
> 2) A function that dumps a struct value (`{x = 1, y = 2}`).
> 
> Then when printing the overall value, you can call (1) then (2), and when 
> printing each field, you can call (1), then print the field name, then call 
> (2) for a struct value (and do whatever else makes sense for other kinds of 
> values).

Thanks for your review! I agree with your suggestion, and i try to improve code.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2071
   if (Types.empty()) {
     Types[Context.CharTy] = "%c";
     Types[Context.BoolTy] = "%d";
----------------
I have an idea, can we use  analyze_printf::PrintfSpecifier::fixType instead of 
this static DenseMap to find the printf format specifier? what do you all think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to