[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

[Adrian Prantl via Phabricator via llvm-commits]

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