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&id=86693#toc
Repository:
rL LLVM
https://reviews.llvm.org/D29
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 terribly controversial. The "Long
>> term"
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 terribly controversial. The "Long
> term" part of the proposals might be, but that's not what you're talking
>
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 nee
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 discussing: all
> the res
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 may be value in both. I
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 wrote:
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 we
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 war
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
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 through
Thanks for the clarification, I wrote "try to convert" there because I
didn't look closely enough to see what problem it was solving. Since it
seems to be specific to ValueObject (which a grep confirms), we may be able
to just move the code for SharingPtr next to ValueObject. That said,
SharingPt
> 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 like this library already exist
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).
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__, thread_sp-
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/source/Target/ThreadList.cpp:3
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
http://lists.llvm.org/cgi-bin/mailm
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
18 matches
Mail list logo
