[PATCH] D158980: [DebugInfo][NFC] Move ObjC Selector name handling to lib DebugInfo

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

Couple of minor nitpicks, otherwise this LGTM.



================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:752
 
+struct ObjCSelectorNames {
+  StringRef Selector;
----------------
Why plural?


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:755
+  StringRef ClassName;
+  std::optional<StringRef> ClassNameNoCategory = std::nullopt;
+  std::optional<std::string> MethodNameNoCategory = std::nullopt;
----------------
Is the `=` part necessary or will it be default constructed as empty like `llvm::Optional`?


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:757
+  std::optional<std::string> MethodNameNoCategory = std::nullopt;
+};
+
----------------
If there are any comments to explain the latter two members, that would be nice.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp:992
+  ObjCSelectorNames Ans;
+  Ans.ClassName = ClassNameStart.take_front(FirstSpace);
+  Ans.Selector = SelectorStart.drop_back(); // drop ']';
----------------
maybe that's for a later patch, but this seems to be a complicated way of saying
consumeFront("-[")
split(" ")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158980