dblaikie added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3842-3846
+ if (EmitDwarf &&
+ Args.hasFlag(options::OPT_fno_eliminate_unused_debug_types,
+ options::OPT_feliminate_unused_debug_types, false) &&
+ DebugInfoKind >= codegenoptions::DebugInfoConstructor)
+ DebugInfoKind = codegenoptions::UnusedTypeInfo;
----------------
Would this be tidier if it were rolled into the if/checking around 380 since
it's a very similar option?
================
Comment at: clang/test/CodeGen/debug-info-unused-types.cpp:24-30
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
+// NODBG-NOT: !DIEnumerator(name: "BAZ"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// NODBG-NOT: !DIEnumerator(name: "Z"
+// NODBG-NOT: !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_class_type, name: "y"
----------------
Maybe simpler to test the NODBG by `NODBG-NOT: DI{{[a-zA-Z]*}}Type` ? Not sure
if that'd quite work, but might be adequate.
================
Comment at: clang/test/CodeGen/debug-info-unused-types.cpp:15-21
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
+// CHECK: !DIEnumerator(name: "BAZ"
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: !DIEnumerator(name: "Z"
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "y"
----------------
nickdesaulniers wrote:
> dblaikie wrote:
> > nickdesaulniers wrote:
> > > dblaikie wrote:
> > > > Probably good to check these end up in the retained types list
> > > What's the best way to check this? In particular, I worry about the
> > > `!<number>` number changing in the future, resulting in this test
> > > spuriously failing. For this test case, we have:
> > >
> > > ```
> > > !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1,
> > > producer: "Nick Desaulniers clang version 12.0.0
> > > (g...@github.com:llvm/llvm-project.git
> > > e541558d11ca82b3664b907b350da5187f23c0c9)", isOptimized: false,
> > > runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes:
> > > !15, splitDebugInlining: false, nameTableKind: None)
> > > ```
> > > which tells us the `retainedTypes` is `!15`. `!15` looks like:
> > > ```
> > > !15 = !{!16, !17, !3, !5, !18, !8}
> > > ```
> > > In this case, should I just add the numbers? If we used regex patterns
> > > `[0-9]`, I worry that we couldn't ensure the list would match. Is it ok
> > > to just hardcode these?
> > You can use captures in FileCheck to make references a little more
> > flexible. Surprised there aren't /any/ tests in LLVM testing the
> > retainedTypes list... test/CodeGen/debug-info-extern-call.c used to use the
> > retained types list & still has some testing remnants of it you could draw
> > from, something like::
> > ```
> > // CHECK: !DICompileUnit({{.*}}retainedTypes: [[RETTYPES:![0-9]+]]
> > // CHECK: [[RETTYPES]] = !{[[T1:![0-9]+]], [[T2:![0-9]+]], ...}
> > ...
> > // CHECK: [[T1]] = !DICompositeType(...
> > ```
> >
> > It doesn't protect the test from changes in the order of the retained types
> > list but otherwise resilient to extraneous metadata nodes being added or
> > removed.
> So one issue I'm running into with this approach is that the retained type
> list is emitted in between the other debug nodes (some before it, some after
> it). The capture logic seems to have an issue referring to captures that
> have yet to be defined yet.
>
> note: uses undefined variable(s): "TYPE7"
>
> but if I move the `CHECK` for the retained types last, then `FileCheck`
> errors because it seems it only doesn't one pass for the `CHECK` statements
> (IIUC), so it's "too late" to parse.
>
> Is there another way to test this, or am I holding it wrong?
Order dependence is a currently unavoidable part of FileCheck tests, so if the
content you're trying to test (I'm not entirely clear from your description -
so guessing a bit) looks something like:
```
!1 = DICompileUnit ... retainedTypes: !3
!2 = DICompositeType "x"
!3 = !{!2, !4}
!4 = DICompositeType "y"
```
Then you'd CHECK this something like:
```
; CHECK: DICompileUnit {{.*}} retainedTypes: [[RET_TY:![0-9]*]]
; CHECK: [[X:![0-9]*]] = DICompositeType "x"
; CHECK: [[RET_TY]] = !{[[X]], [[Y:![0-9]*]]}
; CHECK: [[Y]] = DICompositeType "y"
```
Yes, this does mean that if the types were emitted in a different order, the
test would fail - but that's a limitation we're living with for pretty much all
LLVM and Clang testing at the moment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80242/new/
https://reviews.llvm.org/D80242
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
