[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.


================
Comment at: lldb/source/Utility/Log.cpp:406
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+    os_log(g_os_log, "%{public}s", str.c_str());
+  } else {
----------------
JDevlieghere wrote:
> aprantl wrote:
> > Can you add a FIXME for future reference that this ought to be moved into a macro into the header, so we can benefit from os_log's special compiler support and the only reason why we don't do this is that it's unsafe to include <os/log.h> in the LLVM headers?
> I'd be happy to add the comment, but it wouldn't be entirely true though. The log handers are meant to sit underneath the more general logging infrastructure and as you pointed out, you need a constant string in order to benefit from the os_log optimization. Unlike for os_signposts, I think it's very unlikely that we'd ever want to bypass our whole logging system for that. 
s/the comment/a comment/ -> describing what I said here :-) 


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

https://reviews.llvm.org/D128321