[PATCH] D60642: [dsymutil] Collect parseable Swift interfaces in the .dSYM bundle.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h:169
+                             StringRef Default = {}) {
+  if (V)
+    if (auto S = V->getAsCString())
----------------
Early return?


================
Comment at: llvm/tools/dsymutil/CompileUnit.h:322
+  /// The DW_AT_language of this unit.
+  uint16_t Language = 0;
+
----------------
dwarf::SourceLanguage? 


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:273
+      DWARFDie CUDie = CU.getOrigUnit().getUnitDIE();
+      auto ResolvedPath = resolveObjectPath(CUDie, Path);
+      sys::path::append(ResolvedPath, Path);
----------------
Since it's passed already, can the helper append the path? 


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:295
+    std::map<std::string, std::string> &ParseableSwiftInterfaces,
+    std::function<void(const Twine &, const DWARFDie &)> ReportWarning,
+    bool InImportedModule = false) {
----------------
Maybe we should just make this a member function instead of passing ReportWarning as a callback. 


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:2196
+
+  auto PCMpath = resolveObjectPath(CUDie, PCMfile, Options.PrependPath);
   if (Error E = loadClangModule(PCMfile, PCMpath, Name, DwoId, ModuleMap, DMO,
----------------
Aha, this answers my previous question: no because the helper is used here too.


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:2513
+    StringRef InterfaceFile = I.second;
+    SmallString<0> InputPath;
+    if (!Options.PrependPath.empty()) {
----------------
`SmallString<0>`? Also, how about moving this out of the loop and clearing it?


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:2521
+    if (Options.Verbose)
+      outs() << "cp " << InterfaceFile << ' ' << Path.str() << '\n';
+
----------------
nit: maybe be a little more verbose here


================
Comment at: llvm/tools/dsymutil/DwarfLinker.h:498
 
+  /// .swiftinterface files referenced by the debug info.
+  std::map<std::string, std::string> ParseableSwiftInterfaces;
----------------
Can you explain what the first and second string is? Can this be a StringMap?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60642