[PATCH] D73676: [Remarks] Extend the RemarkStreamer to support other emitters
Jonas Devlieghere via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 4 16:04:37 PST 2020
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
More information about the cfe-commits
mailing list