jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
The InstrumentationRuntime implements a fairly specific model of something that
might provide history threads. There has to be a module in the program that
implements this, and a
Author: penryu
Date: Fri Mar 10 18:58:41 2017
New Revision: 297538
URL: http://llvm.org/viewvc/llvm-project?rev=297538&view=rev
Log:
fix xunit attribute parsing
Modified:
lldb/trunk/packages/Python/lldbsuite/test_event/formatter/xunit.py
Modified: lldb/trunk/packages/Python/lldbsuite/test_ev
labath is in Europe, so he will probably not see this until tonight.
On Fri, Mar 10, 2017 at 3:31 PM Jonathan Roelofs via Phabricator <
revi...@reviews.llvm.org> wrote:
> jroelofs added a comment.
>
> In https://reviews.llvm.org/D30844#698294, @zturner wrote:
>
> > The test suite is full of flaki
clayborg added a comment.
Looks ok to me. Jim, are you ok with this? If Jim is OK with this, then I am.
https://reviews.llvm.org/D30006
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com
jroelofs added a comment.
In https://reviews.llvm.org/D30844#698294, @zturner wrote:
> The test suite is full of flakiness, but as long as it's the same before and
> after your change, it doesn't seem like anything is broken.
Results are pretty much the same, with 1 or 2 twinkling tests (could
jroelofs added a subscriber: labath.
jroelofs added a comment.
@labath thoughts?
https://reviews.llvm.org/D30844
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner added a comment.
labath@ will need to comment on this. I wasn't aware that we were getting the
thread name anywhere else. Im not sure if this code is even important, but
hopefully it is not.
The test suite is full of flakiness, but as long as it's the same before and
after your chang
jroelofs updated this revision to Diff 91419.
jroelofs added a comment.
Herald added a subscriber: emaste.
Built successfully on linux (lots of tests fail, not sure if that's expected or
not?). I don't have a FreeBSD machine, but it looks "obviously" the same.
https://reviews.llvm.org/D30844
F
jroelofs added a comment.
erm, nevermind, I misread.
https://reviews.llvm.org/D30844
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner added a comment.
Oh wait, there's nothing else in those files Usually someone on the Apple
team picks up the slack when changes are required to the Xcode project. If you
use Xcode, you're welcome to remove them yourself.
(Goes without saying, but make sure the build still succeeds
zturner added a comment.
I don't think you need to delete the entire files. Just the `SetName` function.
https://reviews.llvm.org/D30844
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-c
jroelofs added a comment.
looks like lldb.xcodeproj refers to the files which I'll be deleting... do I
need to worry about that?
https://reviews.llvm.org/D30844
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin
jroelofs added a comment.
sure
https://reviews.llvm.org/D30844
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner added a comment.
It looks like I just forgot to remove those functions. All the callers have
been updated to use the llvm method. Can you just delete this method (and the
one in `HostThreadFreeBSD`?
https://reviews.llvm.org/D30844
___
ll
jroelofs added a comment.
Yeah, I'm on tip of trunk as of a few minutes ago.
`llvm::set_thread_name()` exists, but it doesn't take a thread parameter like
this one does (no idea if that's (still?) used or not in lldb).
https://reviews.llvm.org/D30844
zturner added a comment.
Hmm.. I thought this code was gone? Are you at tip of trunk? If so then I
must be crazy. I added `llvm::set_thread_name()` which already handles all
this correctly, and I thought I converted all of LLDB's clients over to using
the LLVM function.
https://reviews.ll
jroelofs created this revision.
Herald added a subscriber: srhines.
Alternatively, I could teach llvm:set_thread_name() how to take a thread
parameter, and use that here instead.
https://reviews.llvm.org/D30844
Files:
source/Host/linux/HostThreadLinux.cpp
Index: source/Host/linux/HostThrea
Author: jmolenda
Date: Fri Mar 10 13:31:54 2017
New Revision: 297496
URL: http://llvm.org/viewvc/llvm-project?rev=297496&view=rev
Log:
Simplify & correct the patch I wrote in r297441, after thinking
about this more I realized I could make the change isolated to
whether we decide an empty accelerat
jingham added a comment.
I missed that you had set this to target. I'd rather avoid doing that unless
there's good reason since it means we duplicate all the module iteration logic.
To my mind, within reason it's always better to be conservative and set too
many locations rather than too few.
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D30789
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lld
labath added a comment.
In https://reviews.llvm.org/D30817#697960, @jingham wrote:
> Can you say more about the problem you are trying to solve.
Yes.
Consider `foo.h` in the test case. Since the functions `foo1` and `foo2` are
inline, they will only show up in the module if they are used. Thi
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
Can you say more about the problem you are trying to solve. As far as
breakpoints are concerned, if you find a match in each of several modules it
seems to me you would always wan
zturner added inline comments.
Comment at: lldb/source/Commands/CommandCompletions.cpp:112
+ if (!Resolver)
+Resolver = &SR;
+
labath wrote:
> amccarth wrote:
> > This leaves the caller with no way to say the don't want a tilde resolver.
> > I didn't expec
zturner updated this revision to Diff 91361.
zturner added a comment.
Addressed suggestions from labath@
https://reviews.llvm.org/D30789
Files:
lldb/include/lldb/Interpreter/CommandCompletions.h
lldb/include/lldb/Utility/TildeExpressionResolver.h
lldb/source/Commands/CommandCompletions.cp
I can see both of them making sense, but I would actually prefer the
new behavior you inadvertently introduced. I've looked at the callers
and it seems no user sets find_directories to false, so I guess it's
fine to change the behavior, if we want to.
On 10 March 2017 at 14:08, Zachary Turner wro
You're right, I didn't notice that. But since you mention it, surely that
had to have been a bug in the original implementation right? That flag
isn't intended to be a synonym for "non recursive iteration ", because
that's what the Next enumeration value is for. The algorithm would
intentionally le
labath created this revision.
move-to-nearest-code needs special treatment to avoid creating
superfluous breakpoints in case multiple compilation units. It already
had code to handle the case when the compilation units are in the same
module, but it still did not properly handle the case when the
ravitheja added inline comments.
Comment at: scripts/interface/SBTrace.i:12
+
+class LLDB_API SBTrace {
+public:
clayborg wrote:
> Do we want something in here that explains what kind of trace buffer data you
> will be getting? I am thinking that even though we
labath added a comment.
Comment at: lldb/source/Host/common/FileSpec.cpp:786
+ continue;
+if (!find_directories && fs::is_directory(Status))
+ continue;
This looks like it changes behavior. Previously, if `find_directories` was
false this functi
labath added a comment.
Seems very reasonable. A couple of details I noticed are in the inline comments.
As for the name, I'd suggest just dropping the `2` and letting overload
resolution handle that. If we don't need the interpreter argument, then maybe
we should aim for removing that overloa
30 matches
Mail list logo
