dblaikie accepted this revision.
dblaikie added a comment.

I think this is good - with some check after the template list to constrain the 
dag/not checks to only be within the intended tag, not accidentally skip past 
into some other tag.



================
Comment at: clang/test/CodeGenCXX/debug-info-template-parameter.cpp:27
+// DWARF-DUMP:       DW_TAG_class_type
+// DWARF-DUMPABEL:     DW_AT_name      ("foo<char, 3, true, 1>")
+// DWARF-DUMP:         DW_TAG_template_type_parameter
----------------
typo? (`DUMPABEL`)


================
Comment at: clang/test/CodeGenCXX/debug-info-template-parameter.cpp:42
+// DWARFv4-DAG:          DW_AT_default_value   (true)
+// STRICT-NOT:           DW_AT_default_value
+
----------------
Probably good to end this list of DAG/NOT with a `DWARF-DUMP: {{DW_TAG|NULL}}` 
to ensure these attribute matches don't happen to end up matching some other 
tag after the template parameters?


================
Comment at: clang/test/CodeGenCXX/debug-info-template-parameter.cpp:37
+// STRICT-NOT:         DW_AT_default_value   (true)
+// DWARF-DUMP:       DW_TAG_template_value_parameter
+// DWARF-DUMP-NEXT:    DW_AT_type    ({{.*}} "bool")
----------------
Michael137 wrote:
> Michael137 wrote:
> > dblaikie wrote:
> > > Could consider using `DWARF-DUMP-LABEL` for the TAGs here (& maybe an 
> > > extra `DWARF-DUMP-LABEL: DW_TAG` at the end) so that the attributes are 
> > > constrained to be within the same tag? (could use `DWARF-DUMP-DAG` to 
> > > test attributes in a way that's not order-specific, which would be nice - 
> > > but I don't know that we do that much in the dwarfdump-driven tests, so 
> > > not a huge deal)
> > +1 for the `DAG` directive
> > 
> > Could you elaborate on the `-LABEL` though? IIUC the labels have to match a 
> > line in the file uniquely, which the `DW_TAG`s wouldn't
> Could label the `DW_AT_name` perhaps? Unless I'm misunderstanding
Oh, yeah, unless the tests small enough to have unique/simple DW_TAGs you could 
-LABEL, this is probably all the reason why we don't DAG in dwarfdump tests 
usually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139953

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

Reply via email to