aprantl added inline comments.
================ Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2187 + DEFINE_MDNODE_GET(DITemplateTypeParameter, + (MDString * Name, Metadata *Type, bool IsDefault), + (Name, Type, IsDefault)) ---------------- clang-format, please. `(MDString *Name` ================ Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1685 + (Context, getMDString(Record[1]), + getDITypeRefOrNull(Record[2]), false)), + NextMetadataNo); ---------------- ``` MetadataList.assignValue( GET_OR_DISTINCT(DITemplateTypeParameter, (Context, getMDString(Record[1]), getDITypeRefOrNull(Record[2]), false)), Record.size() == 4 ? getMDOrNull(Record[3]) : false), NextMetadataNo); ``` ================ Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1708 + getDITypeRefOrNull(Record[3]), false, + getMDOrNull(Record[4]))), + NextMetadataNo); ---------------- Same here. ================ Comment at: llvm/test/Bitcode/DITemplateParameter-5.0.ll:50 +; CHECK: !DITemplateTypeParameter({{.*}} +!14 = !DITemplateTypeParameter(name: "T", type: !10) + ---------------- I think you can reduce the size of the binary bitcode file significantly by just having a raw TemplateParameter in the file, like so: ``` !named = !{!0, !1} !0 = !DITemplateTypeParameter(name: "T", type: !10) !1 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) ``` ================ Comment at: llvm/unittests/IR/MetadataTest.cpp:2078 StringRef Name = "template"; DIType *Type = getBasicType("basic"); ---------------- For symmetry: `bool defaulted = false;` ================ Comment at: llvm/unittests/IR/MetadataTest.cpp:2080 - auto *N = DITemplateTypeParameter::get(Context, Name, Type); + auto *N = DITemplateTypeParameter::get(Context, Name, Type, false); ---------------- `get(Context, Name, Type, defaulted);` ================ Comment at: llvm/unittests/IR/MetadataTest.cpp:2085 EXPECT_EQ(Type, N->getType()); - EXPECT_EQ(N, DITemplateTypeParameter::get(Context, Name, Type)); + EXPECT_EQ(N, DITemplateTypeParameter::get(Context, Name, Type, false)); ---------------- same here ... ================ Comment at: llvm/unittests/IR/MetadataTest.cpp:2090 + getBasicType("other"), false)); + EXPECT_NE(N, DITemplateTypeParameter::get(Context, Name, Type, true)); ---------------- This can stay. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73462/new/ https://reviews.llvm.org/D73462 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits