[PATCH] D73676: [Remarks] Extend the RemarkStreamer to support other emitters

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM with some nits inline.



================
Comment at: llvm/docs/Remarks.rst:630
+example, LLVM IR passes will emit ``llvm::DiagnosticInfoOptimization*`` that
+get converted to ``llvm::remarks::Remark`` objects to be serialized to disk,
+and clang could set up its own specialized remark streamer that takes
----------------
This sentence is pretty long and complex. Maybe add a full stop instead of the comma and start a new sentence. 


================
Comment at: llvm/docs/Remarks.rst:644
+
+With the only disadvantage being making the code more complex.
+
----------------
I think the trade-off is clear, but if you really want to be explicit maybe just say at the cost of an extra layer of abstraction. 


================
Comment at: llvm/include/llvm/IR/LLVMContext.h:225
 
-  /// Return the streamer used by the backend to save remark diagnostics. If it
-  /// does not exist, diagnostics are not saved in a file but only emitted via
-  /// the diagnostic handler.
-  RemarkStreamer *getRemarkStreamer();
-  const RemarkStreamer *getRemarkStreamer() const;
-
-  /// Set the diagnostics output used for optimization diagnostics.
-  /// This filename may be embedded in a section for tools to find the
-  /// diagnostics whenever they're needed.
+  /// The "Main remark streamer" used by all the specialized remark streamers.
+  /// This streamer keeps generic remark metadata in memory throughout the life
----------------
why is Main capitalized but not remark streamer? I think this should all be lowercase? That would be consistent with the rest of the patch. 


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

https://reviews.llvm.org/D73676