JDevlieghere added a comment. In D65128#1596964 <https://reviews.llvm.org/D65128#1596964>, @labath wrote:
> 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... Thanks for pointing this out. Somehow I convinced myself that formatv supported both, but on further inspection that is not true. I guess we could change the format strings too, but I'm afraid that is a more controversial change. > > > 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. That explains my confusion as to it being part of the macro. I never noticed it showing up in the log, and this explains why. 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
