Page MenuHomePhabricator

Remove a couple of Stream flags
ClosedPublic

Authored by labath on Jan 12 2017, 10:24 AM.

Details

Summary

I came across this while trying to understand what Log::Debug does. It turns out
it does not do anything, as there is no instance of someone setting a debug flag
on a stream. The same is true for the Verbose and AddPrefix flags. Removing
these will enable some cleanups in the Logging class, and it brings us closer
towards the long term goal of standardizing on llvm stream classes.

I have removed these flags and all code the code which tested for their
presence -- there wasn't much of it, mostly in SymbolFileDWARF, which is
probably going away at some point anyway.

The eBinary flag still has some users, so I am letting it life for the time
being.

Diff Detail

Repository
rL LLVM

Event Timeline

labath updated this revision to Diff 84143.Jan 12 2017, 10:24 AM
labath retitled this revision from to Remove a couple of Stream flags.
labath updated this object.
labath added reviewers: clayborg, zturner.
labath added subscribers: lldb-commits, beanz.
clayborg accepted this revision.Jan 12 2017, 10:31 AM
clayborg edited edge metadata.

Most of the DWARF stuff is about to go away anyway in favor of using the LLVM DWARF parser as I am currently modifying it to support all we need in LLDB so we can get rid of the entire DWARF parsers, so many of these changes are fine.

Just fix the definition of eBinary to be (1<<0) and this is good to go.

include/lldb/Core/Stream.h
36 ↗(On Diff #84143)

No one serializes this flag or sets it manually, so make it equal to (1<<0).

This revision is now accepted and ready to land.Jan 12 2017, 10:31 AM
zturner accepted this revision.Jan 12 2017, 10:57 AM
zturner edited edge metadata.

Is this going to make passing LIBLLDB_LOG_OPTION_VERBOSE to GetLogIfAllCategoriesSet into a "you can't see these logs" operation?

I use this in a couple places where I have detailed information that I usually don't want to see, but in some odd cases I might need it. Loosing this capability would be a shame.

Is this going to make passing LIBLLDB_LOG_OPTION_VERBOSE to GetLogIfAllCategoriesSet into a "you can't see these logs" operation?

I use this in a couple places where I have detailed information that I usually don't want to see, but in some odd cases I might need it.  Loosing this capability would be a shame.

That is not going to be affected - I am not touching that flag. I am removing a *separate* flag on the Stream level, which was never set (although some of the (effectively dead) code did test for it's existence). See old and new implementations of Log::GetVerbose to see what I mean.

Yeah, I'm starting to remember this a bit. For some reason, we have individual log channels with verbose as a category (LIBLLDB_LOG_VERBOSE, POSIX_LOG_VERBOSE, KDP_LOG_VERBOSE) and we have LLDB_LOG_OPTION_VERBOSE which is set "GetVerbose" is testing for.

So you can do:

log enable -v lldb memory

or

log enable lldb memory verbose

and they will do different things depending on who checks what. That seems really confusing. We should remove the verbose category and have everybody check GetVerbose instead. But that's orthogonal to this patch.

This revision was automatically updated to reflect the committed changes.