[PATCH] D87179: Fix debug_abbrev emitter to only assign table id once

[James Henderson via Phabricator via llvm-commits]

jhenderson added a subscriber: grimar.
jhenderson added a comment.

Thanks @aadsm for the fix. On the note of writing Mach-O inputs, I completely agree that it's a pain, especially when compared with the ELF equivalents. The problem is that the Mach-O side of yaml2obj/obj2yaml has not yet been heavily worked on in a similar manner to much of @grimar's recent work on the ELF side. The obvious change would be to remove the need for lots of properties that could otherwise be derived automatically. If you or anybody else wants looking at that, I'm sure there would be lots of grateful Mach-O developers (I personally don't develop for Mach-O, so am not going to be investing time myself, but am happy to review).



================
Comment at: llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml:683
 
-# RUN: yaml2obj --docnum=3 %s | obj2yaml | FileCheck %s --check-prefix=MULTI-TABLES
+# RUN: yaml2obj --docnum=3 %s | obj2yaml | FileCheck %s --check-prefix=MULTI-TABLES --dump-input=always
 
----------------
Remove `--dump-input` from the FileCheck command.

I actually have rarely used the option myself - you can achieve the goal of getting FileCheck to dump the input by having no check prefixes that match the prefix being looked for, which is often a good starting point.


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:27-28
     for (auto AbbrvDeclSet : *AbbrevSetPtr) {
+      if (AbbrvDeclSet.second.begin() == AbbrvDeclSet.second.end())
+        continue;
       Y.DebugAbbrev.emplace_back();
----------------
Higuoxing wrote:
> Do we really need this check here? I think it will prevent us from dumping an empty abbrev table.
> 
> ```
> debug_abbrev:
>     - ID: 0
>       Table:
>         - Code:           1
>           Tag:            DW_TAG_compile_unit
>           Children:       DW_CHILDREN_yes
>           Attributes:
>             - Attribute:  DW_AT_producer
>               Form:       DW_FORM_strp
>     - ID: 1   ## This table will not be dumped.
> ```
Could we get a test case for this situation, please? Either separately or as part of this patch, I don't mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87179