[PATCH] D80722: [ObjectYAML][DWARF] Make the `PubSection` optional.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/MachOEmitter.cpp:299
+                     Obj.DWARF.PubNames)
+            DWARFYAML::EmitPubSection(OS, *Obj.DWARF.PubNames,
                                       Obj.IsLittleEndian);
----------------
I think you should do this:

```
...
} else if (0 == strncmp(&Sec.sectname[0], "__debug_pubnames", 16) {
  if (Obj.DWARF.PubNames)
    DWARFYAML::EmitPubSection(OS, *Obj.DWARF.PubNames,  Obj.IsLittleEndian);
} else
....
```

Otherwise the code will continue doing `strncmp` for the following `else` branches, what is
probably not the desired flow.


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:149
+    Y.PubTypes.emplace(/*IsGNUStyle=*/false);
+    dumpPubSection(DCtx, *Y.PubTypes, PubTypes);
+  }
----------------
Thoughts (not for this patch):

Looking on this, I seems it would be better to change the `dumpPubSection` to
something like:

```
static DWARFYAML::PubSection dumpPubSection(DWARFContext &DCtx, DWARFSection Section, bool IsGNU);
```

It should simplify the code here  and also will allow to switch to return `Expected<DWARFYAML::PubSection>` later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80722