[Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax

[Zachary Turner via lldb-commits]

Personally I'm not really a fan of source/line information at all.  There
is only a very small probability that anyone using a released LLDB (e.g.
through Xcode) is going to have their log lines match up with ToT, because
we're changing files all the time.  You change one thing on line 10, and
now every other log line that comes after it, potentially tens of thousands
of lines in the same file, are going to have their line information out of
sync.

On Thu, Dec 8, 2016 at 9:50 AM Greg Clayton via Phabricator <
reviews at reviews.llvm.org> wrote:

> clayborg added a comment.
>
> Seems weird that we are a C++ codebase and we fall back to C macros. We
> currently need the macro only for file + line and to also not have to worry
> about doing "if (log) log->". I am not a big fan of macros, but I am open
> to it if everyone else wants it.
>
>
>
> ================
> Comment at: include/lldb/Core/Log.h:218-219
> +          << llvm::formatv(
>       \
> +                 "{0,-60}: ",
>        \
> +                 (llvm::sys::path::filename(__FILE__) + "::" +
> __FUNCTION__)   \
> +                     .str())
>      \
> ----------------
> This is hard coding in the file + line all the time. I would like this to
> be an option. We also need to be able to optionally enable the pid/tid,
> thread name, stack backtrace and all the other options that are currently
> supported by "log enable". Seems like we need a log function that exists in
> Log.cpp to take a log mutex, add the extra log prefix stuff (file + line,
> pid/tid, thread name, timestamp, delta since last log time, stack etc) and
> then call the llvm::formatv() one or more times and then release the mutex.
>
>
> ================
> Comment at: include/lldb/Host/FileSpec.h:675-677
> +  void format(llvm::raw_ostream &Stream, llvm::StringRef Options) const {
> +    Stream << this->GetCString();
> +  }
> ----------------
> For a good example of what objects can do to help sell this proposal we
> should handle different values for the Options string. Maybe this to
> support the following:
>
> Options is empty -> print out full path
> Options == "filename" -> print out file name only with extension
> Options == "basename" -> print out file name only without extension
> Options == "directory" -> print out file directory only
> Options == "extension" -> print out file extension only
> Options == "volume" -> print out file volume only which will probably only
> work on windows paths currently
> Options == "fullname" -> print out volume + directory + filename, same as
> default empty string
>
> Then we can format like:
>
>
> ```
> FileSpec file("/tmp/foo.txt");
> llvm::formatv("{0; filename}", file);
> ```
>
>
> https://reviews.llvm.org/D27459
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161208/f38c5a69/attachment.html>