This is an archive of the discontinued LLVM Phabricator instance.

Replace WINLOG_*** macros with LLDB_LOG
ClosedPublic

Authored by labath on Feb 20 2017, 10:15 AM.

Details

Summary

The main difference here is that in the WINLOG macros you can specify
log categories per call, whereas here you have to go the usual lldb
route of getting a Log* variable first. While this means you have to
write at least two statements, it usually means that each statement will
fit on a single line, whereas fitting the WINLOG invocation on a single
line was almost impossible. So the total size of code does not increase
even in functions with a single log statement, and functions with more
logging get shorter.

The downside here is reduced flexibility in specifying the log
categories, which a couple of functions used quite heavily (e.g.
RefreshStateAfterStop). For these I chose a single category used most
prominently and put everything into that, although a solution with
multiple log variables is definitely possible.

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Feb 20 2017, 10:15 AM
amccarth accepted this revision.Feb 21 2017, 2:08 PM

LGTM.

Maybe we have too many categories.

source/Plugins/Process/Windows/Common/DebuggerThread.cpp
207 ↗(On Diff #89130)

This looks like the only possibly controversial change in this file. This log message will no longer happen for those logging just exception-related events. I guess that's acceptable as it's unlikely.

source/Plugins/Process/Windows/Common/ProcessWindows.cpp
873 ↗(On Diff #89130)

This one and the next one seem unfortunate, as the messages are indeed something you'd expect to see when logging breakpoints.

It makes me wonder if all these categories are worth the effort.

This revision is now accepted and ready to land.Feb 21 2017, 2:08 PM

Maybe we have too many categories.

Thanks. I was thinking about that as well. Currently there is about 110 log statements in the windows log and 8 log categories. Things wouldn't get too noisy even if we showed everything into the same category.

This revision was automatically updated to reflect the committed changes.