labath added a comment.

I don't think you can make a change like this mechanically, and the reason is 
not the file-function prepending, but the fact that the two styles use a 
different format specifier syntax (printf vs. the llvm formatv thingy). If you 
change the function without changing the format strings, you just create a 
bunch of static log messages...

In D65128#1596813 <https://reviews.llvm.org/D65128#1596813>, @JDevlieghere 
wrote:

> In D65128#1596809 <https://reviews.llvm.org/D65128#1596809>, @jingham wrote:
>
> > Actually, I don't want this change as is.  Some logs - like the expression 
> > and step logs - are laid out for readability, and LLDB_LOG automatically 
> > adds the file & function which will make them much harder to read.
> >
> > We need a variant that doesn't inject this information, and then this 
> > change should use that variant.  Or we can remove the file & function from 
> > LLDB_LOG and decide to let the people add those as they wish.  I don't have 
> > a strong opinion one way or the other, but I certainly don't want this 
> > extra noise in the step or expression logs.
>
>
> I agree with you. I always use LLDB_LOG, but not because it includes the file 
> and function, but because it's consistent, it gives me the ability to use the 
> LLVM formatter, and I don't have to explicitly check whether the log is NULL 
> or not. I can only speak for myself of course.
>
> @labath Since you added the macro, what's your take on this? Do you care a 
> lot about the file and function, and should we have two macros, one that 
> includes this and one that doesn't? Or should we have just one macro, and 
> have users decide whether they need this information or not?


The reason I added the file-function prepending stuff was because a lot of our 
logging statements do things like:

  if(log)
    log->Printf("MyUsuallyVeryLongClassName::%s real stuff...", args..., 
__FUNCTION__);

This means that the Printf call usually does not fit a single line, and so each 
logging statement ends up taking at least three lines (together with the 
if(log) stuff). By making sure the origin of the log statement is automatically 
added to the message, I was hoping that we'd only log the actual interesting 
stuff, and so we'd have succinct log statements which don't interfere with the 
flow of the rest of the code (`LLDB_LOG(log, "real stuff...", args...)`).

So, that was the direction I wanted to take things in. However, I wouldn't say 
that has been entirely successful, so if you want to take a different direction 
now, that's fine with me. I just want to point out two things:

- The file-function prepending stuff is optional. In fact it's not even enabled 
by default -- you have to pass a `--file-function` flag to the "log enable" 
command to get this behavior
- The prepending code goes through a lot of trouble to make sure the 
file-function field is always of the same width. This means that if you have a 
log file which contains these things, you can just scroll to the right to avoid 
seeing them, or (assuming your editor supports block operations) easily cut 
them out.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65128



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

Reply via email to