[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

This revision was automatically updated to reflect the committed changes. Closed by commit rL293806: Break some dependencies in lldbUtility. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D29359?vs=86516=86693#toc Repository: rL LLVM

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

lgtm On 1 February 2017 at 11:01, Zachary Turner wrote: > Correct, I'm not doing any of the "long term" stuff now, I was just thinking > out loud with that. > > On Wed, Feb 1, 2017 at 10:49 AM Jim Ingham wrote: >> >> I don't think any of this should be

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

I don't think any of this should be terribly controversial. The "Long term" part of the proposals might be, but that's not what you're talking about here, right? These changes will presumably require adjustments to the Xcode project. If you can do that yourself, then that's great. If you

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

Are you interested in seeing the followup patches for review (moving classes from Core -> Utility etc), or does it sound reasonable just based on my description? On Tue, Jan 31, 2017 at 6:05 PM Jim Ingham wrote: > BTW, not to exclude the positive because it doesn't need

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

BTW, not to exclude the positive because it doesn't need discussing: all the rest of the changes you were proposing seem appropriate and good to me. Thanks for taking the trouble to clean this up. Jim > On Jan 31, 2017, at 5:45 PM, Zachary Turner wrote: > > Yea, there

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

Yea, there may be value in both. If i ever tried to do this I'd probably take the approach of converting all the obvious cases first that should enforce checking and then see what's left. Then we could use llvm:Error and lldb::Status, or something On Tue, Jan 31, 2017 at 5:39 PM Jim Ingham

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

Yeah, I have no idea how you'd make sure that the SBError returned to Python in a Python SBValue was checked before it went out of scope, and I'm not sure it's our business to be enforcing that... So we're going to have to do something special for those errors. We also use the pattern where

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

llvm::Error only enforces checking at the point that it goes out of scope. So the example you mention should be supported, as long as you propagate the value all the way up and don't destroy it. There's also ways to explicitly ignore an Error (similar to casting to void to make the compiler not

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

I think we discussed this before, but we need an informational error object. IIRC, the llvm::Error has to be checked. But for instance if you ask a value object to evaluate itself, but you've moved to a section of code where the variable has no location, then evaluating that value will result

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

My vote is to err on the side of leaving stuff in Utility that is general (like SharingPtr) but only used by one client, rather than moving it next to its current only client. After all, you are likely to go dumpster diving in Utility when you need a tool, but you're less likely to look

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

> On Jan 31, 2017, at 3:59 PM, Zachary Turner via Phabricator via lldb-commits > wrote: > > zturner created this revision. > Herald added a subscriber: mgorny. > > The goal here is to make `lldbUtility` a library which depends on no other > libraries. It seems

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

zturner added a comment. In https://reviews.llvm.org/D29359#662417, @labath wrote: > 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-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

zturner added inline comments. Comment at: lldb/source/Target/ThreadList.cpp:387 if (log) - log->Printf("ThreadList::%s thread 0x%4.4" PRIx64 - ": voted %s, but lost out because result was %s", - __FUNCTION__,

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

labath added a comment. 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). Comment at:

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

zturner added a comment. > Move Error from Core -> Utility Also, I almost forgot. Long term: Delete and use `llvm::Error` https://reviews.llvm.org/D29359 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

zturner created this revision. Herald added a subscriber: mgorny. 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