[PATCH] D69142: [dsymutil] Add support for linking remarks

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:2554
+  SmallString<128> Path;
+  // Contents/Resources/Remarks
+  sys::path::append(Path, *Options.ResourceDir, "Remarks");
----------------
I'm not sure if this comment is more helpful than saying "create the `Remarks` directory in the `Resources` directory", and below saying "append the file name, possibly followed by a dash and its architecture in case we're dealing with a fat file", or something like that :-) 


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:2563
+  sys::path::append(Path, sys::path::filename(BinaryPath));
+  if (Options.NumDebugMaps > 1) {
+    // Contents/Resources/Remarks/a.out-x86_64
----------------
Maybe say that this is for fat binaries. 


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:2565
+    // Contents/Resources/Remarks/a.out-x86_64
+    // Contents/Resources/Remarks/a.out-x86_64h
+    Path += '-';
----------------
This looks like you're iterating over something. If you go this route I think one is fine. 


================
Comment at: llvm/tools/dsymutil/LinkUtils.h:72
 
+  /// Number of debug maps processed in total.
+  unsigned NumDebugMaps = 0;
----------------
Doxygen supports "grouping" with `@{` and `@}`. Can we create a group and specify that this is used for the remarks?


================
Comment at: llvm/tools/dsymutil/Options.td:148
+
+def remarks_prepend_path: Separate<["--", "-"], "remarks-prepend-path">,
+  MetaVarName<"<path>">,
----------------
Please update `cmdline.test` with these new options.


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

https://reviews.llvm.org/D69142