[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())
----------------
aprantl wrote:
> JDevlieghere wrote:
> > Early return?
> I'm not convinced that this is much better. What do you think?
> 
> ```
> if (!V)
>   return Default;
> 
> const char *Str = V->getAsCString()
> if (!Str)
>   return Default;
> 
> return Str;
> ```
I would do the following, but doesn't really matter that much :-) 

```
if (!V)
  return Default;


if (const char *Str = V->getAsCString())
  return Str;

return Default;
```


================
Comment at: llvm/tools/dsymutil/DwarfLinker.h:498
 
+  /// .swiftinterface files referenced by the debug info.
+  std::map<std::string, std::string> ParseableSwiftInterfaces;
----------------
aprantl wrote:
> JDevlieghere wrote:
> > Can you explain what the first and second string is? Can this be a StringMap?
> I thought about his, too. This is a map from ModuleName->FilePath (I'll add that to the comment). I need the entries to be uniqued and sorted and there are only few entries expected per compile unit. The iteration order over a StringMap isn't well-defined and the RHS would need to be std::strings anyway, so I thought a std::map fits the bill.
Makes sense with the comment.


================
Comment at: llvm/tools/dsymutil/dsymutil.cpp:290
   if (OutputFileOpt == "-")
-    return OutputFileOpt;
+    return OutputLocation{OutputFileOpt, {}};
 
----------------
Since the resource dir is always empty, maybe make it a default argument in the ctor?


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

https://reviews.llvm.org/D60642