[PATCH] D86860: [DWARFYAML] Make the debug_str section optional.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:89
 Error DWARFYAML::emitDebugStr(raw_ostream &OS, const DWARFYAML::Data &DI) {
-  for (auto Str : DI.DebugStrings) {
+  assert(DI.DebugStrings && "unexpected emitDebugStr() call");
+  for (auto Str : *DI.DebugStrings) {
----------------
 I think you don't really need this assert, because `Optional<>` implementation is based on
`OptionalStorage` which has it already:

```
  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
    assert(hasVal);
    return value;
  }
```


================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:90
+  assert(DI.DebugStrings && "unexpected emitDebugStr() call");
+  for (auto Str : *DI.DebugStrings) {
     OS.write(Str.data(), Str.size());
----------------
I'd avoid using `auto`.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:35
 # RUN: yaml2obj --docnum=2 %s -o %t2.o
-# RUN: llvm-readelf --string-dump=.debug_str %t2.o | FileCheck %s --check-prefix=DWARF-DEFAULT
+# RUN: llvm-readelf --string-dump=.debug_str %t2.o | FileCheck -DSIZE=000006 %s --check-prefix=DWARF-DEFAULT
 # RUN: llvm-readelf -S %t2.o | FileCheck %s --check-prefix=SHDRS
----------------
Seems `-DSIZE` is unused? (you are not using `SHDRS-DEFAULT` here).


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:51
   StringRef RemainingTable = DCtx.getDWARFObj().getStrSection();
+  std::vector<StringRef> DebugStr;
   while (RemainingTable.size() > 0) {
----------------
You can do something like:

```
Y.DebugStrings.emplace();
```


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:55
     RemainingTable = SymbolPair.second;
-    Y.DebugStrings.push_back(SymbolPair.first);
+    DebugStr.push_back(SymbolPair.first);
   }
----------------
And then just

```
Y.DebugStrings->push_back(SymbolPair.first);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86860