This is an archive of the discontinued LLVM Phabricator instance.

Start breaking some dependencies in lldbUtility
ClosedPublic

Authored by zturner on Jan 31 2017, 3:59 PM.

Details

Summary

The goal here is to make lldbUtility a library which depends on no other libraries. It seems like this library already exists in spirit as a place to house very low level code which is commonly used by all parts of LLDB, so it makes sense to designate this as the one top-level library. We can change the name in the future to something like Support if we choose to (implying that it is analogous to LLVMSupport), but for now I just want to break the dependencies.

So, this patch deletes a bunch of dead code and moves some code that is actually only used in 1-2 places up to where it's actually used.

This is not enough to get Utility dependency free. Further tasks that I've identified, but which are too large to fit into this patch, include:

  1. Move RegularExpression from Core -> Utility (Long term: Delete and use LLVM)
  2. Move Stream from Core -> Utility (Long term: Delete and use llvm::raw_ostream)
  3. Move ConstString from Core -> Utility
  4. Move ProcessStructReader from Utility -> Process
  5. Move RegisterNumber from Utility -> Target
  6. Move Error from Core -> Utility
  7. Try to convert ValueObject::GetSP() from SharingPtr to std::shared_ptr, then delete SharingPtr.
  8. Move ModuleCache from Utility -> Target

Finally, once all of those things are done, we can begin breaking up the lldb-private.h, and lldb-enumerations.h, etc header files and moving the enumerations to more appropriate locations, which will finally break this up.

Diff Detail

Event Timeline

zturner created this revision.Jan 31 2017, 3:59 PM

Move Error from Core -> Utility

Also, I almost forgot.

Long term: Delete and use llvm::Error

labath edited edge metadata.Jan 31 2017, 4:20 PM

Looks reasonable. To make sure things stay this way, we should make sure that the Utility unit test has only this module specified as a dependency (I guess after @beanz is done with digging through that).

lldb/source/Target/ThreadList.cpp
387

Any reason for not using the log macro here? (replace if(log) log->Format) with LLDB_LOG(log, ...)).

I did not anticipate using this function directly as the __FILE__ , __FUNCTION__ thingies make it very obnoxious.

zturner added inline comments.Jan 31 2017, 4:22 PM
lldb/source/Target/ThreadList.cpp
387

Ahh, this is the first time I've written a logging statement since your changes, I think I'm just not accustomed to the new way yet. I can update this though.

Looks reasonable. To make sure things stay this way, we should make sure that the Utility unit test has only this module specified as a dependency (I guess after @beanz is done with digging through that).

Yea, this was the plan. After we're all done, drop all the dependencies from the CMakeLists.txt and the unit test exe and then everything else should happen automatically.

This revision was automatically updated to reflect the committed changes.
lldb/source/Utility/ARM_DWARF_Registers.h