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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to