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