[PATCH] D69141: [Remarks] Add support for linking remarks

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: llvm/include/llvm/Remarks/Remark.h:119
+  if (!LHS && RHS)
+    return true;
+  if (LHS && !RHS)
----------------
Why does this not return false?


================
Comment at: llvm/include/llvm/Remarks/RemarkLinker.h:33
+                    const std::unique_ptr<Remark> &RHS) const {
+      return *LHS < *RHS;
+    };
----------------
Should we have an assert here that says that both pointers should be valid?


================
Comment at: llvm/include/llvm/Remarks/RemarkLinker.h:39
+  /// dangling references.
+  StringTable StrTab;
+  /// A set holding unique remarks.
----------------
A newline between members (and method below) would really improve readability.


================
Comment at: llvm/lib/Remarks/RemarkLinker.cpp:108
+      return link(*Section, RemarkFormat);
+    else
+      return Error::success();
----------------
No else after return


================
Comment at: llvm/lib/Remarks/RemarkLinker.cpp:110
+      return Error::success();
+  } else {
+    return SectionOrErr.takeError();
----------------
No else after return


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

https://reviews.llvm.org/D69141