[PATCH] D48241: [DebugInfo] Emit ObjC methods as part of interface.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D48241#1134240, @dblaikie wrote:

> Not sure I should get too involved in this discussion, knowing so little about Objective C - but if it seems sensible, could you provide some examples (just in comments/description in this code review) of what the DWARF used to look like and what it looks like after this change?


After this change we emit a forward declaration as a child of the DW_TAG_structure_type. The definition (below) remains unchanged.

  0x00000027:   DW_TAG_structure_type
                  DW_AT_APPLE_objc_complete_type  (true)
                  DW_AT_name      ()
                  DW_AT_byte_size (0x04)
                  DW_AT_decl_file ("cat.m")
                  DW_AT_decl_line (1)
                  DW_AT_APPLE_runtime_class       (0x10)
  
  0x0000002d:     DW_TAG_member
                    DW_AT_name    ()
                    DW_AT_type    (0x0000007b "base ")
                    DW_AT_decl_file       ("cat.m")
                    DW_AT_decl_line       (2)
                    DW_AT_data_member_location    (0x00)
                    DW_AT_accessibility   (DW_ACCESS_protected)
  
  0x00000037:     DW_TAG_subprogram
                    DW_AT_name    ()
                    DW_AT_decl_file       ("cat.m")
                    DW_AT_decl_line       (10)
                    DW_AT_prototyped      (true)
                    DW_AT_type    (0x0000007b "base ")
                    DW_AT_declaration     (true)
  
  0x0000003f:       DW_TAG_formal_parameter
                      DW_AT_type  (0x0000007f "structure *")
                      DW_AT_artificial    (true)
  
  0x00000044:       DW_TAG_formal_parameter
                      DW_AT_type  (0x00000084 "structure *")
                      DW_AT_artificial    (true)
  
  0x00000049:       NULL
  ...
  0x0000007a:     NULL
  ...
  0x000000b5:   DW_TAG_subprogram
                  DW_AT_low_pc    (0x0000000000000000)
                  DW_AT_high_pc   (0x000000000000001c)
                  DW_AT_frame_base        (DW_OP_reg6 RBP)
                  DW_AT_object_pointer    (0x000000cf)
                  DW_AT_name      ()
                  DW_AT_decl_file ("cat.m")
                  DW_AT_decl_line (10)
                  DW_AT_prototyped        (true)
                  DW_AT_type      (0x0000007b "base ")
  
  0x000000cf:     DW_TAG_formal_parameter
                    DW_AT_location        (DW_OP_fbreg -8)
                    DW_AT_name    ()
                    DW_AT_type    (0x000000b0 "structure *")
                    DW_AT_artificial      (true)
  
  0x000000d8:     DW_TAG_formal_parameter
                    DW_AT_location        (DW_OP_fbreg -16)
                    DW_AT_name    ()
                    DW_AT_type    (0x00000195 "structure *")
                    DW_AT_artificial      (true)
  
  0x000000e1:     NULL



> Does this address the discoverability that's being discussed in the llvm-dev thread? There were concerns there about having to search through all the instances of a type to find all of its functions - I imagine, since Objective C classes aren't closed (if I understand correctly) that would still be a concern here? If it is, what is the benefit/tradeoff of this change (if it doesn't address the discoverability/still requires the Objective C accelerator table)?

Following this approach we have the exact same problem (and I believe we could use the same solution). The motivation for this change is that there's no clean way to implement the .apple_objc accelerator table using the debug_names, because it's conceptually different from the other tables. It maps an interface name to the DIEs of its methods, as opposed to all the other tables that map a name to their corresponding DIE. The only way this could work is as a separate table, because otherwise you'd have to ensure, for every entry, if it's a "regular" one, which obviously violates the standard. Putting it in a separate column is also a bad idea, because that means the column is present for all the entries, including the ones that don't need it.



================
Comment at: clang/lib/CodeGen/CGDebugInfo.h:111
+    // methods.
+    llvm::DICompositeType *DIInterfaceDecl;
+    std::vector<MethodData> Methods;
----------------
aprantl wrote:
> Isn't the interface already the key in the DenseMap?
That's the `ObjCInterfaceDecl` while this is the `DICompositeType`. 


https://reviews.llvm.org/D48241