On Fri, Jun 8, 2018 at 10:10 AM Leonard Mosescu wrote:
> I agree, checked in binaries are not always pretty. But some coverage
> depends checked in binaries (or at very least is dramatically harder to get
> the same thing from source)
>
> Are we saying that sacrificing coverage to keep tests smal
On Fri, Jun 8, 2018 at 11:52 AM Pavel Labath wrote:
> On Fri, 8 Jun 2018 at 18:48, Leonard Mosescu wrote:
> >
> > To me, a linker order file (of any linker) sounds like a good
> abstraction level for generating that kind of input (on linux, I might
> have preferred a .s file with hardcoded .loc
Ahh sorry, I jumped in kinda late and the thread was already quite long so
I didn’t read everything. It would probably be some overhead to learn how
to write the asm files but you can probably have clang-cl generate one for
you and just move the directives around
On Fri, Jun 8, 2018 at 12:18 PM Pav
Author: zturner
Date: Tue Jun 12 10:43:52 2018
New Revision: 334518
URL: http://llvm.org/viewvc/llvm-project?rev=334518&view=rev
Log:
Refactor ExecuteAndWait to take StringRefs.
This simplifies some code which had StringRefs to begin with, and
makes other code more complicated which had const cha
The internal api has no guarantees as to its stability.
On Fri, Jun 15, 2018 at 7:48 AM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:
> clayborg added inline comments.
>
>
>
> Comment at: source/API/SBHostOS.cpp:48
> + case ePathTypePythonDir:
> +fspec = Scr
It can assert when we pass the python or clang directory enumeration. We
could also remove the values from the internal enumeration but keep them in
the public enumeration. However, we can’t be forced into providing a stable
internal api just because we must provide a stable public api.
On Fri, Jun
Related question: Is the laziness done to save memory, startup time, or
both?
On Thu, Jun 21, 2018 at 7:36 AM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:
> clayborg added a comment.
>
> In https://reviews.llvm.org/D48393#1138989, @labath wrote:
>
> > I am not sure this will act
Performance i get. Gdb is almost unusable for large programs because of how
long it takes to load debug info.
Do you have specific numbers on memory usage? How much memory (absolute and
%) is saved by loading debug info lazily on a relatively large project?
On Thu, Jun 21, 2018 at 7:54 AM Greg Cla
Fwiw I’ve seen cases where tests have passed even though they shouldn’t
have — the functionality being tested was broken. The one that comes to
mind was where we were doing a backtrace and then checking that it matched
the regex “main\(argc=3” to make sure the local variable argc had the
correct va
I think the problem is in lld, we don’t emit S_UDT symbols because it
caused weird problems in WinDbg. There’s a comment explaining it in
PDB.cpp. But after some recent fixes this may just work. So we should
probably try again to emit the S_UDT in lld, I think that should fix it
On Fri, Jul 20,
I had previously thought of making a top level project called Dump that
depends on everything. Also makes it very obvious where all the dumpers
are. It can have overloaded functions called lldb_private::dump(T&) for
every value of T, then no matter what type you have, all you have to do is
call dum
We only use dumping from lldb-test, from inside the debugger, or from the
top level command interpreter. All of those things necessarily depend on
everything anyway.
On Fri, Jul 20, 2018 at 8:02 AM Pavel Labath wrote:
> Well, if it depends on everything, then it can only used from places
> that a
When you have a DIA interface for struct S, can you just call
findChildren()? Will that enumerate tge unnamed struct?
The fact that pdbutil doesn’t is only an indication of how the printing
code behaves, you shouldn’t interpret anything about what information is
available from it
On Wed, Jul 25, 20
Wouldn’t the location of the unnamed struct be the location of its first
member? We already print the offsets of the individual members, so that
part is solvable (although that code is horrendously complex). The second
part is figuring out how to correlate the member back to the unnamed struct
it c
In the meantime, perhaps you could request commit access :)
On Thu, Jul 26, 2018 at 9:30 PM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:
> aleksandr.urakov added a comment.
>
> Thanks!
>
> I have created the corresponding patch for Clang (
> https://reviews.llvm.org/D49871,
Please remember to test with the cmake build when you add or remove files,
as that is the build that all of the buildbots use. I almost reverted this
since it broke every LLDB buildbot, but I noticed that it's just forgetting
to remove the files from the CMakeLists.txt so I'll fix it.
On Thu, Aug
Author: zturner
Date: Thu Aug 2 10:44:41 2018
New Revision: 338746
URL: http://llvm.org/viewvc/llvm-project?rev=338746&view=rev
Log:
Fix CMake build.
Some new files were committed to the repository but not added
to the CMakeLists.txt, so this patch fixes the build.
Modified:
lldb/trunk/sour
No objections here
On Fri, Aug 3, 2018 at 3:24 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath added a comment.
>
> Does anyone have thoughts on this? (I have one more move like this lined
> up (for the Listener/Broadcaster classes), and then I should be done with
> the
Several months ago when this came up, i hypothesized that Utility would
eventually contain a mix of random stuff some of which may not make sense
in the long term. But that in the meantime, it’s useful to have some sort
of layering abstraction for “has no other dependencies”. Eventually when
this r
I think we have llvm::contains() which would allow you to make this
slightly better. Other than that, good find!
On Fri, Aug 3, 2018 at 5:49 PM Leonard Mosescu via Phabricator <
revi...@reviews.llvm.org> wrote:
> lemo created this revision.
> lemo added reviewers: zturner, labath, teemperor.
> l
Looks like i was wrong, nevermind!
On Fri, Aug 3, 2018 at 6:23 PM Leonard Mosescu via Phabricator via
lldb-commits wrote:
> lemo added subscribers: bgianfo, labath, penryu, teemperor, zturner.
> lemo added a comment.
>
> Thanks Zach. I can't find llvm::contains() though, any pointers to it?
>
>
>
I am OOO this week and only have access to mobile, so hopefully someone
else can review it
On Mon, Aug 6, 2018 at 11:05 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:
> aleksandr.urakov added a comment.
>
> Ping!
>
> Can you review this, please?
>
>
> https://reviews.llvm.o
Did you see my comments on the first round about how the CMake build didn’t
work? Because I don’t see any changes to CMakeLists.txt here, which means
it still won’t work.
The easiest way to make sure you get all the fixes that may have gone in
after your initial commit is to revert the revert and
How does this differ from VS Code's builtin LLDB MI support?
On Wed, Aug 8, 2018 at 8:15 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath added a comment.
>
> To end things on a positive note: I think the test coverage is pretty good
> (I'm sure somebody will suggest sw
To elaborate, if you install the C/C++ plugin, and you go to Debug ->
Configurations, and you go down to the MICmdMode property, it is by default
set to "gdb". But you can change this to "lldb" and it works out of the
box.
On Wed, Aug 8, 2018 at 8:30 AM Zachary Turner wrote:
> How does this dif
Ok sounds good then, mostly just wanted to make sure you weren't
re-inventing something that already existed :) Do you plan to publish this
on the VSCode marketplace?
On Wed, Aug 8, 2018 at 8:38 AM Greg Clayton wrote:
> It is a different protocol. The LLDB MI plugin didn't work very well as
> w
Ok I’ll take a look later today then when i get in
On Mon, Aug 13, 2018 at 2:13 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:
> aleksandr.urakov added a comment.
>
> Unfortunately, there was no people yet, who can review this :)
>
> Ping! Can anyone review this, please?
>
On Wed, Aug 15, 2018 at 4:48 PM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:
> clayborg marked 23 inline comments as done.
> clayborg added inline comments.
>
>
>
> Comment at: tools/lldb-vscode/SourceBreakpoint.h:24
> + // Set this breakpoint in LLDB as a new
On Wed, Aug 15, 2018 at 4:59 PM Zachary Turner wrote:
>
>
> On Wed, Aug 15, 2018 at 4:48 PM Greg Clayton via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> clayborg marked 23 inline comments as done.
>> clayborg added inline comments.
>>
>>
>>
>> Comment at: tools/lldb-vsc
That's a pretty good idea. clang-tidy could probably catch something like
this.
On Thu, Aug 16, 2018 at 1:13 PM Greg Clayton via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> It would be interesting to have some sort of warning or static analyzer to
> avoid platform specific name conflict
(I mean it can't right now, but we could probably make it)
On Thu, Aug 16, 2018 at 1:35 PM Zachary Turner wrote:
> That's a pretty good idea. clang-tidy could probably catch something like
> this.
>
> On Thu, Aug 16, 2018 at 1:13 PM Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org>
On Wed, Aug 22, 2018 at 12:34 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:
> aleksandr.urakov added a subscriber: labath.
> aleksandr.urakov added a comment.
>
> It seems that the cause of the failure is
> https://reviews.llvm.org/rL340206, but I'm not sure if the adding
For PE/COFF files, the Age is also in the executable and Guid+Age actually
constitute a 20-byte UUID. Is this not the case on Apple? What object file
format are you dealing with?
On Wed, Aug 29, 2018 at 10:11 AM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:
> clayborg created thi
We probably don't. Windows debug info in LLDB is currently based off of
DIA SDK, which handles everything for us (but obviously only works on
Windows). That's one of the things that would need to be fixed to debug
Windows minidumps on Linux (I assume).
On Wed, Aug 29, 2018 at 10:47 AM Leonard Mo
I don’t think I’m really a good person to look at AST stuff. I can look for
general style comments, obvious flaws, and test coverage. but you may be
the best person regarding on the content of the patch. Does that sound ok?
On Fri, Aug 31, 2018 at 7:20 AM Aleksandr Urakov via Phabricator <
revi...@
Lgtm, thanks!
On Wed, Sep 5, 2018 at 6:43 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:
> aleksandr.urakov updated this revision to Diff 164027.
> aleksandr.urakov added a comment.
>
> Code cleanup
>
>
> https://reviews.llvm.org/D51162
>
> Files:
> include/lldb/Symbol/Cl
That’s fine
On Tue, Sep 11, 2018 at 7:11 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:
> aleksandr.urakov added a comment.
>
> @stella.stamenova @ted Fixed with https://reviews.llvm.org/rL341942,
> thanks again!
> @zturner I'll rewrite `GetClassOrFunctionParent` and will c
On Tue, Sep 11, 2018 at 4:28 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:
> aleksandr.urakov added inline comments.
>
>
>
> Comment at: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:289
> + return GetClassOrFunctionParent(*lexical_parent);
>
A visitor would work, but unless we need this frequently it might be
overkill. If we do need it frequently then it would be very helpful though.
For now since we just have this one use case I think a switch statement on
the tag is the simplest. You can group all same cases together and use the
raw
This is definitely required, but only on windows. I’d put it behind a check
for Windows, and I’d also add a check to print a warning/error if (TARGET
lld) returns false on windows
On Mon, Nov 6, 2017 at 9:22 AM Davide Italiano via Phabricator <
revi...@reviews.llvm.org> wrote:
> davide added a sub
It’s been a long time since I looked at this but I remember it just being a
problem in the Makefiles, not in LLDB. I could be remembering wrong
On Mon, Nov 6, 2017 at 1:44 PM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:
> clayborg added a comment.
>
> Are we planning on getting
Can’t you just disable linker gc in the makefile?
On Tue, Nov 7, 2017 at 4:18 AM Pavel Labath via Phabricator via
lldb-commits wrote:
> labath created this revision.
> Herald added a subscriber: srhines.
>
> This test was failing in various configurations on linux in a fairly
> unpredictible way.
I just wonder if we should be disabling linker gc across the board for all
tests unless you explicitly opt in.
Seems like something we would want only rarely, if ever
On Tue, Nov 7, 2017 at 4:36 AM Pavel Labath wrote:
> I could, but I thought this would be more portable. For the Makefile
> solut
If it works, is easy, and doesn’t regress anything I’d honestly rather just
disable linker gc.
But I’m not familiar with the method you mentioned. I thought linker gc
happens when you have -function-sections, -fdata-sections, and —gc-sections.
Wouldn’t it work to just not have those? Or is someth
Fair enough, originally I thought this was the “traditional” kind of GC,
which would have made this approach simpler
On Tue, Nov 7, 2017 at 6:23 AM Pavel Labath wrote:
> On 7 November 2017 at 13:51, Pavel Labath wrote:
> > On 7 November 2017 at 13:29, Zachary Turner wrote:
> >> If it works, is
Super awesome. When you do move it to Utility, can you run the deps python
script to see if any cmake dependencies can be updated?
On Fri, Nov 10, 2017 at 4:02 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath created this revision.
>
> In https://reviews.llvm.org/D39387,
Can we just extend llvm's mapped_file_region to support a boolean Writable
flag?
On Wed, Nov 15, 2017 at 8:03 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath created this revision.
> Herald added subscribers: krytarowski, arichardson, aprantl, emaste.
>
> We sometimes
On Wed, Nov 15, 2017 at 9:51 AM Pavel Labath wrote:
> On 15 November 2017 at 17:42, Zachary Turner wrote:
> > Can we just extend llvm's mapped_file_region to support a boolean
> Writable
> > flag?
> >
> mapped_file_region already can be writable. The feature it is missing
> is the ability to *no
It sounds to me like what you *really* need is for the VReg type itself to
be aligned 16. How about changing this:
struct VReg {
uint8_t bytes[16];
};
to this:
struct VReg {
llvm::AlignedCharArray<16, 16> bytes;
};
Then the FPU struct can remain unchanged.
MSVC does have a 128 bit t
I don't really feel strongly about how you fix it. I'm sure there was a
good reason for making it do that, but at this point I don't remember what
it is and I don't think undoing it will cause a problem.
That said, part of the difficulty of having an API such as this is that
Hyrum's Law [http://w
Yea, if there is a valid string there, it should definitely be returning
"", hard to argue with that.
On Wed, Nov 15, 2017 at 7:06 PM Jason Molenda wrote:
> The general point you're making is reasonable, and something like
> Thread::GetStopDescription is not clear which the correct behavior shou
Probably obvious, can you add some tests to verify the new behavior?
On Wed, Nov 15, 2017 at 7:47 PM Zachary Turner wrote:
> Yea, if there is a valid string there, it should definitely be returning
> "", hard to argue with that.
>
> On Wed, Nov 15, 2017 at 7:06 PM Jason Molenda wrote:
>
>> The
Also, it would be nice (but again this isn't immediately affecting me so I
can't really put too much pressure here) if it continued to return None in
case of error. Any code that would be affected by changing the error
return value from None to "" was very likely broken already and this just
expos
It would be great if we could eventually just use llvm-tblgen to generate
all these register definitions.
On Fri, Nov 17, 2017 at 9:54 AM Greg Clayton via Phabricator via
lldb-commits wrote:
> clayborg added inline comments.
>
>
>
> Comment at: source/Plugins/Process/elf-core/el
On Sun, Nov 19, 2017 at 6:35 AM Jan Kratochvil via Phabricator via
lldb-commits wrote:
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL318626: Add comments to DWARFCompileUnit length
> fields/methods (authored by jankratochvil).
>
> Changed prior t
Right but isn’t there a DWARF64_HEADER and DEARF32_HEADER struct somewhere?
This way you could just say
return m_isdwarf64 ? sizeof(DWARF64_HEADER) : sizeof(DWARF32_HEADER);
On Mon, Nov 20, 2017 at 7:50 AM Greg Clayton wrote:
>
> On Nov 19, 2017, at 4:56 PM, Zachary Turner wrote:
>
>
>
> On Sun
You can make structs that are host and byte-order independent, LLVM is
filled with stuff like this. And while you might end up processing the
information off in a way that it can be stored in a single compile unit
without such a struct, it still can be useful when you're actually *doing*
the parsi
As an aside, I don't really like this class. For example, You can
currently assign a UUID[16] to a UUID[20]. That doesn't make a lot of
sense to me.
As a future cleanup, I think this class should probably be a template such
as UUID, and then internally it can store a std::array. And
we can stat
yaml2core would be an excellent idea for a tool.
On Mon, Nov 27, 2017 at 9:48 PM Davide Italiano via Phabricator via
lldb-commits wrote:
> davide added subscribers: vsk, aprantl, davide.
> davide added a comment.
>
> I thought about this, and it's the only patch from your patchset that I
> don't
On Tue, Nov 28, 2017 at 2:48 AM Pavel Labath via Phabricator via
lldb-commits wrote:
> labath added a comment.
>
> On 28 November 2017 at 06:12, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
> > yaml2core would be an excellent idea for a t
On Tue, Nov 28, 2017 at 10:18 AM Greg Clayton wrote:
> On Nov 27, 2017, at 10:11 PM, Zachary Turner wrote:
>
> As an aside, I don't really like this class. For example, You can
> currently assign a UUID[16] to a UUID[20]. That doesn't make a lot of
> sense to me.
>
>
> What about an invalid UU
On Tue, Nov 28, 2017 at 10:24 AM Zachary Turner wrote:
> On Tue, Nov 28, 2017 at 10:18 AM Greg Clayton wrote:
>
>> On Nov 27, 2017, at 10:11 PM, Zachary Turner wrote:
>>
>> As an aside, I don't really like this class. For example, You can
>> currently assign a UUID[16] to a UUID[20]. That doe
On Tue, Nov 28, 2017 at 10:18 AM Stephane Sezer via Phabricator <
revi...@reviews.llvm.org> wrote:
>
>
> The other alternative seems a bit less explicit to me but I don't mind it
> too much. What's the issue with using `std::any_of` exactly?
>
>
In the sense of correctness, nothing is wrong with u
Eh, that actually just makes me think the compiler *can* check it. For
example, right now you can have mach-o files with 20 byte UUIDs. But just
in the code, not in practice. You could have a bug in your code that
accidentally wrote the wrong number of bytes from a dynamic buffer.
You could enf
Also worth pointing out that when you write things this way, this UUID
class can be part of a larger structure that matches the record layout of a
header or section in a binary file, and then you can just memcpy over the
class and your'e good to go. For example you could have
```
struct MachOHead
the size makes sense in the given context.
On Tue, Nov 28, 2017 at 11:48 AM Jim Ingham wrote:
> How would the ObjectFile API's that take or return UUID's work in this
> case?
>
> Jim
>
>
> > On Nov 28, 2017, at 11:44 AM, Zachary Turner via lldb-commits <
We’ve had this discussion several times, but at the end of the day nothing
has really changed with the larger llvm project having adopted this model
and having spent significant effort in moving forward with it.
Normally, the reason we don't use this model for LLDB tests is because they
require in
On Wed, Nov 29, 2017 at 1:59 PM Jim Ingham wrote:
> I'm mostly basing this concern on the bad effect this had on gdb all of
> whose testing was expect based command scraping. gdb is a tool that's much
> closer to lldb than any of the compiler tools so that experience seems
> relevant. It's been
, Nov 29, 2017 at 3:23 PM Jason Molenda wrote:
>
>
> > On Nov 29, 2017, at 2:51 PM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> >
> >
> > On Wed, Nov 29, 2017 at 1:59 PM Jim Ingham wrote:
> > I'm mostly basi
going to require writing a custom command
> in lldb-test to poke those API's. How would this be any easier than
> writing a unit test?
>
> Jim
>
>
> >
> > On Wed, Nov 29, 2017 at 3:23 PM Jason Molenda
> wrote:
> >
> >
> > > On Nov 29,
You’re right that it’s basically reimplementing readobj but as you said, we
have our own object file readers. More importantly though, even if we
delegated to llvm, something could still in theory go wrong in the Module
class.
Plus, the important thing part of this patch is not this one module
com
Did you forget to add the new test to the changeset?
On Thu, Nov 30, 2017 at 6:19 AM Pavel Labath wrote:
> I have to say I quite like how this test turned out to look like. In
> terms of the last night's SBAPI vs. lldb-mi vs. whateever discussion,
> i'd can say that there is nothing preventing th
Author: zturner
Date: Thu Nov 30 16:52:51 2017
New Revision: 319504
URL: http://llvm.org/viewvc/llvm-project?rev=319504&view=rev
Log:
Add lldb-test.
This is basically a proof-of-concept and starting point for having a
testing-centric tool in LLDB. I'm sure this leaves a lot of room to be
desired
As you said a smaller repro is needed, but I'm imagining a case where we
can write a file containing some C++ code that uses a template that LLDB
doesn't understand, compile it with clang to generate some DWARF, and then
run `lldb-test clang-ast a.out`, where the clang-ast subcommand loads the
file
I don't recall, is there a hard restriction on debugserver not being
allowed to use llvm code? Because YAML is a superset of JSON
On Fri, Dec 1, 2017 at 3:36 PM Davide Italiano via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> Also, FWIW, this is blatantly not the correct way of using ass
ngham wrote:
> >
> > Yes, we don't use llvm code in debugserver. It doesn't actually use any
> lldb classes either, it's its own standalone thing.
> >
> > Jim
> >
> >
> >> On Dec 1, 2017, at 4:01 PM, Zachary Turner via lldb-commits <
Author: zturner
Date: Fri Dec 1 16:15:29 2017
New Revision: 319599
URL: http://llvm.org/viewvc/llvm-project?rev=319599&view=rev
Log:
Add a symbols subcommand to lldb-test.
Differential Revision: https://reviews.llvm.org/D40745
Modified:
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF
On Fri, Dec 1, 2017 at 3:50 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:
> jingham added a comment.
>
> Cool. We do need to make sure people don't start writing tests against it
> yet, however. That would be wasted effort.
>
I don't think it follows that it would be a wasted
I'm curious why it's not working as it's supposed to work on these
platforms. When it does work, it's quite helpful
On Sat, Dec 2, 2017 at 12:24 PM Davide Italiano
wrote:
> Maybe we should remove this feature altogether?
>
> On Fri, Dec 1, 2017 at 4:11 PM, Jim Ingham via lldb-commits
> wrote:
That said, it does seem to make more sense as something you would do in the
main lldb executable, and not in library code.
On Sat, Dec 2, 2017 at 12:26 PM Zachary Turner wrote:
> I'm curious why it's not working as it's supposed to work on these
> platforms. When it does work, it's quite helpfu
It almost looks to me like this should be using aggregation instead of
inheritance. That would add a 3rd option (mark it mutable). That said,
nothing wrong with using this approach either
On Mon, Dec 4, 2017 at 5:43 PM Vedant Kumar via Phabricator via
lldb-commits wrote:
> vsk created this revisi
Admittedly I was on mobile when I wrote that so I didn't have code to look
at. I assumed from the pattern and the few function signatures I saw that
GPR etc were inheriting from thread_state_t, and that was how the cast
worked at all. A quick look at the code suggests this is not the case
though.
Can you / did you try this on Windows? I don't see any reason why it
wouldn't work, but I remember having difficulty with all this CMake some
time ago.
On Tue, Dec 5, 2017 at 1:50 PM Shoaib Meenai via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> Author: smeenai
> Date: Tue Dec 5 13:49:5
Oh come now, we have cross-compilation ;-)
That said, I'd like to try this out locally before you submit, but it might
be tomorrow.
On Tue, Dec 5, 2017 at 2:19 PM Shoaib Meenai wrote:
> I didn't, because I didn't have easy access to a Windows LLVM setup (I
> need to get that working again). I d
Ok, cool!
On Tue, Dec 5, 2017 at 2:24 PM Shoaib Meenai wrote:
> Oh, I thought you meant specifically on a Windows build machine, not just
> targeting Windows. I've already submitted it, but I can do some more
> testing locally as well (and I can always revert if anything seems broken).
> With tha
What about adding GetMemorySize?
On Mon, Dec 11, 2017 at 10:23 AM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:
> clayborg added a comment.
>
> I think GetFileSize() should remain the number of bytes of the section on
> disk and we should add new API if we need to figure out the
SymbolFile does not have a method for searching by regex. There is a
FindFunctions and FindGlobalVariables that allows searching by regex, but
not one for types. That said, one could be added to the base class, yes.
That does seem like a better long term solution.
On Mon, Dec 11, 2017 at 1:37 PM
Long term it would be nice if we could get all these register definitions
automatically generated with llvm-tblgen. That's a big undertaking, though.
On Mon, Dec 11, 2017 at 7:27 PM Vedant Kumar via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> Author: vedantk
> Date: Mon Dec 11 19:27:13
Can we try using lldb_private::RegularExpression for everything? (Long
term, adding a new base class method seems like a better approach, but at
least this quick fix is an immediate fix and should be strictly better than
crashing)
On Wed, Dec 13, 2017 at 10:27 AM Aaron Smith via Phabricator <
rev
I agree that's better long term, but this code was already there, and the
patch makes things strictly better than before (literally just fixes a
crash in existing code). So I think we can agree that this should be
fixed, but as it's a bigger change I don't think it should hold up fixing a
crash.
That seems reasonable. Seems Aaron ran into this not because he was trying
to do a regex search, but because he *wasnt* trying to do a regex search.
So if he doesn’t have immediate need for a regex search, and if it’s not
tested anyway, removing it seems fine
On Wed, Dec 20, 2017 at 10:49 AM Greg C
lldb-test doesn’t actually exercise any of this. It’s a pretty new addition
and doesn’t even support pdb yet as far as I know. So when you say it fails
without the other changes, but passes with this, I think you must be
talking about some test that runs via check-lldb, or some unittest.
To be cle
Sorry I missed this, lgtm
On Fri, Dec 22, 2017 at 7:55 PM Aaron Smith via Phabricator <
revi...@reviews.llvm.org> wrote:
> asmith created this revision.
> asmith added reviewers: zturner, lldb-commits, labath, clayborg.
>
> https://reviews.llvm.org/D41086 fixed an exception in
> FindTypes()/FindTy
Something under lldb\lit. Can make a new directory, like DebugInfo\PDB or
something.
On Tue, Jan 2, 2018 at 9:10 PM Aaron Smith via Phabricator <
revi...@reviews.llvm.org> wrote:
> asmith added a comment.
>
> Do you prefer to drop the test changes and corresponding binaries?
> For an llvm-lit te
Write a unit test and make a class that inherits from it so it can access
the protected member
On Thu, Jan 4, 2018 at 6:54 PM Davide Italiano via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> @Pavel, is there an easy way of testing this? m_core is a protected
> member of ArchSpec so we can'
On Wed, Jan 10, 2018 at 2:09 PM Jim Ingham wrote:
> The only hard part of writing any kind of test for this is actually
> getting a legitimate .app into the testsuite. Doesn't seem fair to ask
> Pavel to do that, since he doesn't work on macOS...
>
> Jim
>
What exactly *is* a .app file on disk?
In that case I think Platform should implement all the logic it needs in
itself, instead of relying Host to callback into it.
I think the point of this patch is to separate dependencies, so Platform
should be able to depend on Host, but not vice versa.
On Thu, Jan 11, 2018 at 8:05 AM Greg Clayton
Looks fine to me too
On Wed, Jan 17, 2018 at 8:56 AM Davide Italiano via Phabricator <
revi...@reviews.llvm.org> wrote:
> davide added a comment.
>
> I think this is fine, but I'll defer to Zachary.
>
>
> https://reviews.llvm.org/D42182
>
>
>
>
___
lldb-
The comments were automatically added a long time ago when I tried to run
IWYU on LLDB source code. I don't think you need to maintain them.
On Sat, Jan 20, 2018 at 2:25 PM Raphael Isemann via Phabricator via
lldb-commits wrote:
> teemperor created this revision.
> teemperor added a reviewer: a
dotest is also a potentially fallible layer on top of the SB Api call, but
one that involves *more* behind-the-scenes code between the test and code
being tested.
An lldb-test test would consist, in its entirety, of about 10 lines of
text. I don’t see how it’s possible to beat that from a simplici
201 - 300 of 2276 matches
Mail list logo
