zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
I'm fine with this.
https://reviews.llvm.org/D51966
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
zturner accepted this revision.
zturner added inline comments.
This revision is now accepted and ready to land.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:274
- auto class_parent_id = raw.getClassParentId();
- if (auto class_parent = session.getSymbolById(clas
zturner added a comment.
I've been experimenting with DIA locally and after some investigation I'm not
sure this is going to be reliable. Let's say we have a class, we want the decl
context containing the class. For example, on line 366. So we call
`GetDeclContextContainingSymbol`. Despite
zturner accepted this revision.
zturner added a comment.
Maybe I can improve this in the native implementation of our PDB reader which
I'm currently working on, so that the results can be more consistent.
https://reviews.llvm.org/D51967
___
lldb-co
zturner accepted this revision.
zturner added inline comments.
Comment at: test/CMakeLists.txt:116-122
+ if (CMAKE_SYSTEM_NAME MATCHES "Windows")
+if (TARGET lld)
+ add_dependencies(check-lldb lld)
+else ()
+ message(WARNING "lld required to test LLDB on Window
zturner added inline comments.
Comment at: unittests/Symbol/TestClangASTContext.cpp:418-419
-arg = m_ast->GetTemplateArgument(t.GetOpaqueQualType(), 1, kind);
-EXPECT_EQ(kind, eTemplateArgumentKindIntegral);
-EXPECT_EQ(arg, int_type);
+EXPECT_EQ(m_ast->GetTempla
zturner added a comment.
I'd be open to having another organizational component that isn't Utility. But
as Jim said, there isn't a critical mass of stuff yet in Utility to figure out
where it makes sense to draw the line.
In all honesty, that second component might just end up being "Core" aga
zturner added a comment.
In https://reviews.llvm.org/D39896#922381, @probinson wrote:
> Drive by comment:
>
> In https://reviews.llvm.org/D39896#94, @jingham wrote:
>
> > You're right, the Triple stuff is in ADT! I was using it as an example of
> > something you clearly wouldn't do so I did
zturner added a comment.
On second thought I actually think the strategy used here is fine. But we have
`llvm::WritableBinaryStream` which could server as a single base interface for
a `mapped_file_region` based implementation as well as a malloc-based
implementation.
For the malloc based imp
zturner added inline comments.
Comment at: source/Utility/UUID.cpp:77-78
void UUID::Dump(Stream *s) const {
- const uint8_t *u = (const uint8_t *)GetBytes();
- s->Printf("%2.2X%2.2X%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X%"
-"2.2X%2.2X%2.2X%2.2X",
-
zturner added a comment.
You could use llvm's range adapters to make this slightly better.
auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
return llvm::find(Bytes, 0) != Bytes.end();
Another option would just be `return *this != UUID(m_num_uuid_bytes);`
https://reviews.llvm.org/D40537
zturner added a comment.
In https://reviews.llvm.org/D40537#937866, @sas wrote:
> In https://reviews.llvm.org/D40537#937196, @zturner wrote:
>
> > You could use llvm's range adapters to make this slightly better.
> >
> > auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
> > return llvm::fi
zturner added a comment.
It's too bad this has to be written as a unit test, because this would be the
perfect candidate for a FileCheck style test.
Probably a long shot, but have you tried the llvm-lit project? Last time I
tried it, it basically worked, but there were only a handful of tests
zturner added a comment.
For the same reason that the entire rest of the LLVM project and all other
subprojects do it, when it makes sense and the nature of the test lends itself
to it.
https://reviews.llvm.org/D40616
___
lldb-commits mailing list
zturner added a comment.
Note that there's no interactivity here. This is "feed some input, get some
output, make sure the output is correct". That's exactly what FileCheck is
designed for. This isn't even testing the public API, it's testing the private
API. We should prefer testing the ac
zturner created this revision.
Herald added subscribers: krytarowski, mgorny, srhines.
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, but this at least allows us to have something to build on.
zturner added inline comments.
Comment at: lit/Modules/compressed-sections.yaml:1
+# REQUIRES: zlib
+# RUN: yaml2obj %s > %t
labath wrote:
> It's right here. (I'm open to suggestions where to place it).
I see. I think part of the reason I didn't notice it is bec
zturner updated this revision to Diff 124960.
zturner added a comment.
Updated based on labath@'s suggestions. Also added a new class `LinePrinter`,
shamelessly ripped and stripped down from a copy used in one of LLVM's dumpers,
but that makes it possible to do some nice formatting easily.
ht
zturner added a comment.
In https://reviews.llvm.org/D40475#935615, @labath wrote:
> The thing to be aware of with this testing strategy is that you will probably
> be the only person who will be able to run these tests and verify dwz
> functionality. If you don't setup a buildbot testing this
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319504: Add lldb-test. (authored by zturner).
Changed prior to commit:
https://reviews.llvm.org/D40636?vs=124960&id=125052#toc
Repository:
rL LLVM
https://reviews.llvm.org/D40636
Files:
lldb/trunk
zturner added inline comments.
Comment at: lit/Modules/compressed-sections.yaml:20
+ - Name:.bogus
+# CHECK-NOT: .bogus
+Type:SHT_PROGBITS
labath wrote:
> zturner wrote:
> > You should probably put this as the very first check stateme
zturner added inline comments.
Comment at: lit/Modules/compressed-sections.yaml:1
+# REQUIRES: zlib
+# RUN: yaml2obj %s > %t
labath wrote:
> zturner wrote:
> > labath wrote:
> > > It's right here. (I'm open to suggestions where to place it).
> > I see. I think p
zturner added inline comments.
Comment at: lit/Modules/compressed-sections.yaml:20
+ - Name:.bogus
+# CHECK-NOT: .bogus
+Type:SHT_PROGBITS
labath wrote:
> zturner wrote:
> > labath wrote:
> > > zturner wrote:
> > > > You should probab
zturner created this revision.
Herald added a subscriber: emaste.
This is the bare minimum needed to dump `ClangASTContext`s via `lldb-test`.
Within the first 10 seconds of using this, I already found a bug. A `FIXME`
note and repro is included in the comments in this patch.
With this, it shou
zturner added a comment.
If I remove that assert, I get this output:
D:\src\llvmbuild\lldb>bin\lldb-test.exe clang-ast foo.o
error: foo.o {0x003b}: unhandled type tag 0x0005
(DW_TAG_formal_parameter), please file a bug and attach the file at the start
of this error message
error: foo.
zturner added a comment.
In https://reviews.llvm.org/D40745#942913, @jingham wrote:
> I'm sure this is just a "quick and dirty implementation" thing, but depending
> on the output of Dump functions doesn't seem like a great idea for long term
> stable testing.
>
> Those functions are meant to b
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319599: Add a symbols subcommand to lldb-test. (authored by
zturner).
Changed prior to commit:
https://reviews.llvm.org/D40745?vs=125197&id=125239#toc
Repository:
rL LLVM
https://reviews.llvm.org/D4
zturner added a comment.
This could be tested by using `lldb-test` to dump the section cache and running
`FileCheck` on it to make sure that all expected sections are cached.
https://reviews.llvm.org/D40869
___
lldb-commits mailing list
lldb-commit
zturner added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:392-399
+ bool is_regex = false;
+ if (name_str.find_first_of("[]?*.-+\\") != std::string::npos) {
+// Trying to compile an invalid regex could throw an exception.
+// Only search
zturner added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:108-125
+ if (auto module_sp = m_obj_file->GetModule()) {
+// See if a symbol file was specified through the `--symfile` option.
+FileSpec symfile = module_sp->GetSymbo
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
lgtm, if you have some time in the future for further improvements Greg's
suggestion would be a good way to make this better.
Repository:
rL LLVM
https://reviews.llvm.org/D41086
_
zturner added inline comments.
Comment at: include/lldb/Utility/Environment.h:70-72
+ std::pair insert(llvm::StringRef KeyEqValue) {
+return insert(KeyEqValue.split('='));
+ }
Why'd you decide to go with inserting an entire pre-formatted key value pair in
zturner added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:394
+ if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
+FindTypesByRegex(RegularExpression(name_str), max_matches, types);
else
clayborg wrote:
> You s
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
This looks better. Technically it's possible to break this with some weird
PDBs but I don't think it's possible to do better without using the native
reader.
Repository:
rL LLVM
https:
zturner added a comment.
Since it seems like you're going to be doing some work in here, it would be
great if you could update `lldb-test` to dump PDB symbol information. This
would allow us to easily test all sorts of things in here. For example, you
could find a PDB that returned an empty s
zturner added a comment.
This is another example where we could test it easily if `lldb-test` could dump
this. Are you willing to take a stab at this?
Repository:
rL LLVM
https://reviews.llvm.org/D41427
___
lldb-commits mailing list
lldb-commit
zturner added a comment.
This was originally written as a unit test because at the time we didn't have
`lldb-test`. To be honest I think it's time to remove these checked in
binaries and convert everything to FileCheck tests. There's a couple of
reasons this is more practical. For starters,
zturner added a comment.
Can you add a test with `REQUIRES: windows` that builds a simple program using
clang-cl, generates a PDB, and then uses `lldb-test` to check part of the
output against that executable? It can be a one line check, basically just
proving that it doesn't crash, and we can
zturner added a comment.
`GetDescription` might be read only, but the code that modifies the description
isn't, right?
https://reviews.llvm.org/D41909
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
zturner added a comment.
In https://reviews.llvm.org/D41902#972614, @clayborg wrote:
> As long as:
>
> % lldb /path/to/Foo.app
> (lldb) r
>
>
> Still works, then I am fine with this. The resolve executable should find the
> executable down inside the app bundle (like
> "/path/to/Foo.app/Con
zturner added a comment.
Seems like the high level problem is that the entity which acquires the mutex
is not the same as the entity which decides how data gets into the Module. For
example, we call `SymbolVendor::FindFunctions` and expect that someone is going
to somehow get some functions in
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
It's nice that this turned out so easy. Didn't even require modifying
`lldb-test`.
Comment at: lit/SymbolFile/PDB/compilands.test:9
+CHECK: {{^[0-9A-F]+}}: SymbolVendor (
zturner added a comment.
In a way it's kind of built into the semantics of `llvm::Error` that this is
the only way it could possibly work, since it's a move only type. If you do
this, for example:
Error E = doSomething();
LLDB_LOG_ERROR(E);
you'd get a compilation failure. The only way t
zturner added inline comments.
Comment at: packages/Python/lldbsuite/test/dotest.py:625
os.environ["LLDB_TEST"] = scriptPath
+os.environ["LLDB_BUILD"] = configuration.test_build_dir
Here this has the possibility of setting `os.environ["LLDB_BUILD"] = N
zturner accepted this revision.
zturner added a comment.
Sorry for forgetting about this! lgtm
Repository:
rL LLVM
https://reviews.llvm.org/D41427
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li
zturner added inline comments.
Comment at: include/lldb/Core/ThreadSafeDenseSet.h:49
void Clear() {
-stds::lock_guard<_MutexType> guard(m_mutex);
+std::lock_guard<_MutexType> guard(m_mutex);
m_set.clear();
aprantl wrote:
> Out of curiosity: Why/ho
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
Trivial fixes like this don't need to be reviewed, you can just commit them.
Thanks for your work btw!
https://reviews.llvm.org/D42345
___
ll
zturner added a comment.
Same as with the last one. For obviously correct bug fixes like this, just
commit them. As an aside, `make_unique` will make this a bit shorter so it
fits on one line. e.g. `auto reg_interface =
llvm::make_unique(arch);`
https://reviews.llvm.org/D42347
_
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
looks good. May as well fix the BCD typo while you're here though (either here
or in a followup patch)
Repository:
rL LLVM
https://reviews.llvm.org/D42434
___
zturner added a comment.
If we just need to test completion, write a lit-style test that uses lldb-test
that looks like this:
RUN: lldb-test complete --target=%T/foo --complete_str=MyPrefix | FileCheck %s
CHECK: Foo::Bar
CHECK: Foo::Baz
etc
Simple and not flaky
https://reviews.llvm
zturner added a comment.
Spinning up a process just to test that auto-completion works though seems a
little bit unnecessary, and introduces the possibility that flakiness and bugs
in the process spawn mechanism (if any exist) will manifest in the test
failure. It would be nice, if and when th
zturner added inline comments.
Comment at: lit/SymbolFile/PDB/Inputs/FuncSymbols.cpp:2
+// Static function
+static long StaticFunction(int a)
+{
Would it be worth trying one in an anonymous namespace?
Comment at: lit/SymbolFile/PDB/func-symbols
zturner added a comment.
Yea this seems like a good candidate for an lldb-test test. Something like
this.
RUN: yaml2obj %S/Inputs/stripped.yaml > %t.stripped.out
RUN: yaml2obj %S/Inputs/unstripped.yaml >
%t/.build-id/1b/8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9.debug
RUN: lldb-test symbols
zturner added a comment.
In the future when you upload diffs can you include context? (i.e. `git diff
-U99`). It's nice to be able to see the surrounding code when I'm looking
at a diff.
Is there ever a case where you would want to build a shared library without
`-fPIC`? I'm wondering i
zturner added a comment.
In the future, we could add options to the `autocomplete` subcommand that would
allow specification of a target, and things like cursor position to maximize
testability.
In general though, I like the approach. It's not hard to imagine 50+ tests
being written just for
zturner added a comment.
By the way, I'd suggest printing indices in front of each match and including
those in the FileCheck tests. Otherwise we could miss completions that sneak
in.
https://reviews.llvm.org/D43048
___
lldb-commits mailing list
zturner added a comment.
Number of matches might be sufficient, but it might be nice to know if the
ordering of matches changes for some reason. Unless there's a specific reason
we want to allow an unstable order, enforcing a stable order seems desirable
just on principle.
https://reviews.ll
zturner added a comment.
On the issue of keeping the other test, I think when an SB API method is
basically a pass-through to a private method, then having a test of the SB API
method that verifies "did the correct native method get called" is useful if
for no other reason than to verify the co
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
This lgtm. If this causes some tests that were previously skipped or xfailed
to start passing, you can unskip / unxfail them at the same time.
Comment at:
packages/Pytho
zturner accepted this revision.
zturner added a comment.
Looks good. Wish this function took a `StringRef` so you could just say
`return name.startswith("?") || name.startswith("_Z")`
https://reviews.llvm.org/D43059
___
lldb-commits mailing list
l
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
Sorry for the delay on this one, looks good.
Repository:
rL LLVM
https://reviews.llvm.org/D42443
___
lldb-commits mailing list
lldb-commits@
zturner added inline comments.
Comment at: lit/CMakeLists.txt:10-13
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER
${LLDB_TEST_C_COMPILER})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER
${LLDB_TEST_CXX_COMPILER})
+str
zturner added a comment.
It's probably possible to make it work, but as Jim said, there's no drop in
replacement currently. There's bits and pieces of stuff that, with a dedicated
effort, could be improved to the point of being sufficient, though.
https://reviews.llvm.org/D43099
__
zturner added a comment.
In https://reviews.llvm.org/D43048#1005513, @jasonmolenda wrote:
> No, the unwind unittests that exist today should stay written as unit tests.
> These are testing the conversion of native unwind formats -- for instance,
> eh_frame, compact unwind, or instruction analy
zturner added a reviewer: asmith.
zturner added a comment.
Aaron, do you remember why you added a check for `width == 0` here? Would it
ever not be 0?
https://reviews.llvm.org/D43215
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http:/
zturner added a reviewer: labath.
zturner added a comment.
What's the behavior on other platforms? When you set a breakpoint in a shared
library before you've run the program, shouldn't it still be unresolved, in
which case this test should have failed on those platforms too?
https://reviews.
zturner added a comment.
In https://reviews.llvm.org/D43419#1011045, @jasonmolenda wrote:
> On Darwin we load all the libraries that the binary links against
> pre-execution, if possible. So I see:
>
> % lldb a.out
> (lldb) ima li libfoo.dylib
> [ 0] 35C5FE62-5327-3335-BBCF-5369CB07D1D6 0x00
zturner added a comment.
I'm not sure how hard it would be, but it would be better if we could do the
same thing on Windows, for consistency of behavior. In principle it's
straightforward, just scan through the IAT and load all of the imported
modules. Probably the DynamicLoaderWindows plugin
zturner added a comment.
I guess one advantage to delaying it is that the debug info for the dynamic
library could be large and slow to parse, and you don't know if you're even
going to need it until you hit the breakpoint. So by delaying the resolution
even to File address, you postpone parsi
zturner added a comment.
I guess on the other hand, it's reasonable to assume that if you've set a
breakpoint somewhere, you're more likely than not to need it since you probably
had a reason for setting it. Is the debug info parsed when the executable is
loaded, or when the breakpoint is set?
zturner added a comment.
One issue I can see with this is that if you check the documentation of -b it
says this:
-b
--batch
Tells the debugger to run the commands from -s, -S, -o & -O, and
then quit. However if any run command stopped due to a signal or
crash, the debu
zturner added a comment.
If your commands are A, B, C, and D but A crashes and returns to the
interactive prompt, will it still try to execute B, C, and D? If so then I
guess that would work (I haven't tried). OTOH, there's a risk of people
forgetting to do this (but I'm not sure if the risk
zturner added a comment.
Actually I may have misunderstood the help text. It sounds like it may be
referring to a crash of the inferior, not a crash of LLDB itself. If the test
contains no run commands (as this one doesn't), then it doesn't seem like
there's any risk of this happening, and th
zturner added a comment.
Do we not need `call_foo2` anymore?
https://reviews.llvm.org/D43600
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner added a comment.
How is `call_foo2` any different than `call_foo1`, which was not removed from
this patch?
https://reviews.llvm.org/D43600
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
zturner added inline comments.
Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-175
+ class Guard final {
+std::recursive_mutex *m_mutex;
- typedef void (*BreakpointSiteSPMapFunc)(lldb::BreakpointSiteSP &bp,
- void *bat
zturner added a comment.
Personally I think it would be clearer to just use `std::unique_lock<>`.
Already it's locking the mutex twice, once with a `lock_guard` and once when
creating a `BreakpointSiteList::Guard`. Which works I guess because it's a
recursive mutex, but it's still confusing.
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
Ok, that's the piece that was missing since the code for `foo.cpp` and
`main.cpp` weren't part of the patch.
https://reviews.llvm.org/D43600
_
zturner added inline comments.
Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-161
+ class Guard : private std::unique_lock {
+using std::unique_lock::unique_lock;
+ };
Err, I meant to just deleted the `Guard` class entirely and return
`llvm::
zturner added a comment.
Couldn't we just add a creator helper so that you can say:
auto cleanup = makeCleanupRAII([&] {
process_sp->Destroy(/*force_kill=*/false);
debugger.StopEventHandlerThread();
});
https://reviews.llvm.org/D43662
___
zturner added a comment.
Curious if we need lldb-test for this. Could we get by with just using lldb in
batch mode? Obviously I'm not opposed to adding whatever we need to lldb-test,
just want to make sure it's something that can't already be done with just lldb.
Comment at
zturner added inline comments.
Comment at: include/lldb/Interpreter/Options.h:123-126
+ llvm::Expected Parse(const Args &args,
+ ExecutionContext *execution_context,
+ lldb::PlatformSP platform_sp,
+
zturner added a comment.
It would be nice if we could eventually get rid of the need to pass in a
platform and execution context here, but that's work for another day.
Comment at: include/lldb/Interpreter/Options.h:123-126
+ llvm::Expected Parse(const Args &args,
+
zturner added a comment.
In https://reviews.llvm.org/D43837#1022554, @jingham wrote:
> Okay, that sounds good then. Will you enforce the rule about the Utilities
> directory socially or by some mechanism?
If you mean the rule that Utility can't depend on anything else, I think it's
enforced
zturner added inline comments.
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:382-387
+ if (llvm::Error E = user_type_or_err.takeError()) {
+std::string Reason = llvm::toString(std::move(E));
if (log)
- log->Printf("Persistent variabl
zturner added a comment.
I'm also ok with not having the macro fwiw, just an idea to reduce boilerplate.
https://reviews.llvm.org/D43912
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-co
zturner added a comment.
Just compile something with /Z7 and you'll get a section called `.debug$S` in
the object file, which is exactly 8 characters. Then teach lldb-test to dump
an object file's sections.
https://reviews.llvm.org/D44042
___
lld
zturner accepted this revision.
zturner added inline comments.
This revision is now accepted and ready to land.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:499
auto type_list = GetTypeList();
-type_list->Insert(result);
+if (type_list)
+ type_lis
zturner added a subscriber: asmith.
zturner added a comment.
Lgtm
Repository:
rL LLVM
https://reviews.llvm.org/D44166
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner added a subscriber: JDevlieghere.
zturner added a comment.
Shouldn’t it be lldb-dotest? I’m confused about what this target does
Repository:
rL LLVM
https://reviews.llvm.org/D44473
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
zturner added a subscriber: labath.
zturner added a comment.
I think having all parsing functions in a single place will just move the
layering problem elsewhere, since a bunch of conversion functions for
objects from various libraries will be mushed together into one place.
https://reviews.llvm
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
Feel free to use your own judgement, and if you think it doesn't pass some
complexity threshold, you can just submit without review and we can do
post-commit review.
Repository:
rL LLVM
zturner added a subscriber: JDevlieghere.
zturner added a comment.
I haven’t had time to look at this in detail yet, but when I originally had
this idea I thought we would use lit’s discovery mechanism to find all .py
files, and then invoke them using dotest.py in single process mode with a
path t
zturner added a comment.
In https://reviews.llvm.org/D45215#1057311, @labath wrote:
> > Preferably lit would take care of as much as possible. I think Zachary’s
> > idea makes sense as an incremental step. If we think of one python file as
> > a google test executable, it makes sense to return
zturner added a comment.
It should be possible to define it outside the LLVM repo. Just in
`llvm/lldb/lit/lit.cfg` replace this line:
config.test_format = lit.formats.ShTest(execute_external)
with something like this:
import lldb_format
config.test_format = lldb_format.LLDBTestFormat()
zturner added a comment.
I don't think `sys.path` is set up correctly to be able to find the lldbtest
package from the `lldb/lit` folder.
These things kind of evolved separately, and the `lldb/lit` folder was created
as a place to start iterating on LLVM-style lit / FileCheck tests. These kind
zturner added a comment.
> This removes the last (direct) dependency from the Host module to
> Interpreter, so I remove the Interpreter module from Host's dependency list.
Big milestone! Kudos
https://reviews.llvm.org/D45480
___
lldb-commits mail
zturner added inline comments.
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:957-958
+
char path[PATH_MAX];
remote_file.GetPath(path, sizeof(path));
+
Can we use the version that returns a `std::string`? I consider this version
unsafe as i
zturner added a comment.
Does it print just the names of the .py files, or does it print the actual test
classes and methods in the Class.TestName format, evaluate XFAIL and SKIPs, etc?
I'm also not entirely clear why `set_up_environment` is even necessary.
Nothing below the call to `set_up_en
zturner added a comment.
In https://reviews.llvm.org/D46005#1077000, @labath wrote:
> In https://reviews.llvm.org/D46005#1076978, @zturner wrote:
>
> > Does it print just the names of the .py files, or does it print the actual
> > test classes and methods in the Class.TestName format, evaluate X
301 - 400 of 733 matches
Mail list logo
