[PATCH] D82435: [DWARFYAML][debug_gnu_*] Add the missing context `IsGNUStyle`. NFC.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/ObjectYAML/DWARFYAML.cpp:51
 void MappingTraits<DWARFYAML::Data>::mapping(IO &IO, DWARFYAML::Data &DWARF) {
+  auto *OldContext = IO.getContext();
+  DWARFYAML::DWARFContext DWARFCtx;
----------------
Higuoxing wrote:
> > @jhenderson : Let's be explicit about the type here. It's not obvious to me from looking at this what the type returned by getContext is.
> 
> `IO.getContext()` returns `void *`. It might be a `MachOYAML::Object&` or a `ELFYAML::Object&` here. I guess we cannot cast it to an explicit type here.
You could just replace `auto` with `void`, I believe. It helps make it clearer.


================
Comment at: llvm/unittests/ObjectYAML/DWARFYAMLTest.cpp:29-35
+  testing::internal::CaptureStderr();
+  auto SectionsOrErr = DWARFYAML::emitDebugSections(Yaml);
+  EXPECT_THAT_ERROR(SectionsOrErr.takeError(), Failed());
+  ASSERT_EQ(testing::internal::GetCapturedStderr(),
+            std::string("YAML:10:19: error: unknown key 'Descriptor'\n"
+                        "      Descriptor: 0x12\n"
+                        "                  ^~~~\n"));
----------------
Rather than capturing stderr, you could use: `EXPECT_THAT_EXPECTED(SectionsOrErr, FailedWithMessage("error message"));`


================
Comment at: llvm/unittests/ObjectYAML/DWARFYAMLTest.cpp:38
+
+TEST(DWARFPubSections, TestDWARFPubSections) {
+  StringRef Yaml = R"(
----------------
I'd put the passing cases for each section above the failing cases.


================
Comment at: llvm/unittests/ObjectYAML/DWARFYAMLTest.cpp:79-85
+  testing::internal::CaptureStderr();
+  auto SectionsOrErr = DWARFYAML::emitDebugSections(Yaml);
+  EXPECT_THAT_ERROR(SectionsOrErr.takeError(), Failed());
+  ASSERT_EQ(testing::internal::GetCapturedStderr(),
+            std::string("YAML:9:7: error: missing required key 'Descriptor'\n"
+                        "    - DieOffset: 0x1234\n"
+                        "      ^\n"));
----------------
Same as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82435