[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In https://reviews.llvm.org/D45215#1056043, @labath wrote:

> I don't think this is going in a good direction TBH.
>
> You are building another layer on top of everything, whereas I think we 
> should be cutting layers out. Besides the issues already pointed out (not 
> being able to differentiate PASS/XFAIL/SKIP, not all .py files being test 
> files), I see one more problem: a single file does not contain a single test 
> -- some of our test files have dozens of tests, and this would bunch them 
> together.


I completely agree and removing the driver logic from dotest would contribute 
to that goal, no?

> I think the solution here is to invent a new lit test format, instead of 
> trying to fit our very square tests into the round ShTest boxes. Of the 
> existing test formats, I think that actually the googletest format is closest 
> to what we need here, and that's because it supports the notion of a "test" 
> being different from a "file" -- gtest executables typically contain dozens 
> if not hundreds of test, and yet googletest format is able to recognize each 
> one individually. The only difference is that instead of running something 
> like "my_google_test_exec --gtest_list_all_tests" you would use some python 
> introspection to figure out the list of tests within a file.

Great, I wasn't aware that there was a dedicated googletest format. If it's a 
better fit then we should definitely consider using something like that.

> Besides this, having our own test format would allow us to resolve the other 
> problems of this approach as well:
> 
> - since it's the test format who determines the result of the test, it would 
> be trivial to come up with some sort of a protocol (or reusing an existing 
> one) to notify lit of the full range of test results (pass, fail, xfail, 
> unsupported)
> - the test format could know that a "test file" is everything ending in ".py" 
> **and** starting with Test (which is exactly the rules that we follow now), 
> so no special or new conventions would be needed.
> - it would give us full isolation between individual test methods in a file, 
> while still having the convenience of being able to factor out common code 
> into utility functions

If we come up with out own test format, would we be able to reuse the current 
dotest.py output?

> I know this is a bit more up-front work, but I think this will result in a 
> much nicer final product, and will allow us to remove a lot more code more 
> easily (maybe even all of unittest2 eventually).

That's totally warranted if it helps in the long term.

> (I apologise for the rashness of my response, I can go into this in more 
> detail tomorrow).

No worries, I didn't have that impression at all. I appreciate the constructive 
feedback!


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D45215#1056607, @JDevlieghere wrote:

> In https://reviews.llvm.org/D45215#1056043, @labath wrote:
>
> > I don't think this is going in a good direction TBH.
> >
> > You are building another layer on top of everything, whereas I think we 
> > should be cutting layers out. Besides the issues already pointed out (not 
> > being able to differentiate PASS/XFAIL/SKIP, not all .py files being test 
> > files), I see one more problem: a single file does not contain a single 
> > test -- some of our test files have dozens of tests, and this would bunch 
> > them together.
>
>
> I completely agree and removing the driver logic from dotest would contribute 
> to that goal, no?


Maybe we could achieve that this way, but this seems like a strange way of 
achieving that. Maybe I just don't know what are the next steps you have in 
plan for this.

> 
> 
>> I think the solution here is to invent a new lit test format, instead of 
>> trying to fit our very square tests into the round ShTest boxes. Of the 
>> existing test formats, I think that actually the googletest format is 
>> closest to what we need here, and that's because it supports the notion of a 
>> "test" being different from a "file" -- gtest executables typically contain 
>> dozens if not hundreds of test, and yet googletest format is able to 
>> recognize each one individually. The only difference is that instead of 
>> running something like "my_google_test_exec --gtest_list_all_tests" you 
>> would use some python introspection to figure out the list of tests within a 
>> file.
> 
> Great, I wasn't aware that there was a dedicated googletest format. If it's a 
> better fit then we should definitely consider using something like that.

Just to be clear. I doubt we will be able to reuse any of the existing 
googletest code, but I don't think that matters as the entirety of googletest 
support code in lit (`llvm/utils/lit/lit/formats/googletest.py`) is 150 lines 
of code, and I don't expect ours to be much longer.

> 
> 
>> Besides this, having our own test format would allow us to resolve the other 
>> problems of this approach as well:
>> 
>> - since it's the test format who determines the result of the test, it would 
>> be trivial to come up with some sort of a protocol (or reusing an existing 
>> one) to notify lit of the full range of test results (pass, fail, xfail, 
>> unsupported)
>> - the test format could know that a "test file" is everything ending in 
>> ".py" **and** starting with Test (which is exactly the rules that we follow 
>> now), so no special or new conventions would be needed.
>> - it would give us full isolation between individual test methods in a file, 
>> while still having the convenience of being able to factor out common code 
>> into utility functions
> 
> If we come up with out own test format, would we be able to reuse the current 
> dotest.py output?

I'm not sure what you mean by this, but I'm pretty sure the answer is yes. :)

If you're talking about the textual output, then we could do the exact same 
thing as googletest is doing. googletest does something like this (`execute()` 
in googletest.py):

  out, err, exitCode = lit.util.executeCommand(
  [testPath, '--gtest_filter=' + testName],
  env=test.config.environment,
  timeout=litConfig.maxIndividualTestTime)
  if exitCode:
  return lit.Test.FAIL, out + err

The only thing we'd need to change is command we execute.


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: zturner.
labath added a comment.

Adding Zachary as he's familiar with lit internals.

I'll try to elaborate more on the approach I had in mind. I would split my 
approach into a couple of tests.

1. Write a tool which will dump out the list of all tests in the test suite. 
This could either be a separate python file, or a special flag to dotest.py, 
whichever is easier. It's implementation would basically be like:

  for each test subdir:
for each file in subdir:
  if file matches Test*.py:
test = __import__(file)
for each class in test:
  for each method in class:
if method matches "test*":
  print("%s.%s.%s"%(file. class.__name__, method.__name__))

The only tricky part here is the `__import__` line. I think we'll need to do 
some fiddling with `sys.path` or whatever to get the test files to import 
correctly. We'll probably need to look at how unittest2 imports them to get it 
right.

This fiddling is also the reason I think this should be a separate utility. 
Since the "test discovery" phase happens in the context of the main lit 
process, which is shared with all other test suites, we should try to avoid 
messing with it's state too much.

2. have the new test format invoke this utility in its discovery function 
(getTestsInDirectory)

3. Invoke something like: `['dotest.py'] + common_args + ['-p', TestFile, '-f', 
testclass + '.' + testmethod`]` in the `execute()` function

The thing to resolve here is to figure out how to translate the result of that 
command into the test state. Whether we wan't to use just the exit code (this 
would give us PASS/FAIL states only), do we want to scrape the command output 
to get the rest of the states (XFAIL, SKIP, ...), or do something else..

I am deliberately bypassing lldb-dotest here, as I think it becomes unnecessary 
in this setup (so we already remove one layer) -- the common args can be 
hardcoded by cmake into `lit.site.cfg` which lit will read for us. And since 
now lit is the one executing the test, you don't have to worry about 
remembering all the arguments because you can just pass the test name to lit, 
and it will figure them out for you (we can still print the full dotest line we 
execute to help debugging).

4. Start ripping out all the multiprocessing/driver logic from dotest.


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Alright, I'm convinced this is the way to go.

- For (1) I'll see if I can get some inspiration from the visit logic in 
dotest.py. I guess the functionality is similar. I agree on doing this in a 
separate tool, especially if we want to remove functionality from dotest in the 
long run.
- For the lit stuff I'll wait on input from Zachary, but it doesn't seem to be 
too hard either.
- No objections to skipping lldb-dotest, it was only meant to support the whole 
lit thing. If we don't need it the better, it has already caused me some minor 
headaches :-)

I'll leave this diff open for the central discussion until I have diffs for the 
sub-problems.


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D45215#1056731, @JDevlieghere wrote:

> Alright, I'm convinced this is the way to go.
>
> - For (1) I'll see if I can get some inspiration from the visit logic in 
> dotest.py. I guess the functionality is similar. I agree on doing this in a 
> separate tool, especially if we want to remove functionality from dotest in 
> the long run.


I'm not aware of any code which would do the full visit. We have some code in 
dosep.py (`find_test_files_in_dir_tree`), but this one only does file 
traversal. It does not look inside the the files for classes, test methods and 
such.

The real visiting happens inside unittest2 (loader.py). The code there seems 
very complicated, but I think that's because it tries to support various things 
we don't use, so /I hope/ that our visiting logic can be simpler.

OTOH, looking at loader.py, it seems to me it may actually be possible to let 
it do the enumeration for us, if we stroke it the right way.


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Zachary Turner via Phabricator via lldb-commits
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 to a specific file.

Why do we need run lines?


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Zachary Turner via lldb-commits
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 to a specific file.

Why do we need run lines?
On Wed, Apr 4, 2018 at 5:04 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:

> labath added a comment.
>
> In https://reviews.llvm.org/D45215#1056731, @JDevlieghere wrote:
>
> > Alright, I'm convinced this is the way to go.
> >
> > - For (1) I'll see if I can get some inspiration from the visit logic in
> dotest.py. I guess the functionality is similar. I agree on doing this in a
> separate tool, especially if we want to remove functionality from dotest in
> the long run.
>
>
> I'm not aware of any code which would do the full visit. We have some code
> in dosep.py (`find_test_files_in_dir_tree`), but this one only does file
> traversal. It does not look inside the the files for classes, test methods
> and such.
>
> The real visiting happens inside unittest2 (loader.py). The code there
> seems very complicated, but I think that's because it tries to support
> various things we don't use, so /I hope/ that our visiting logic can be
> simpler.
>
> OTOH, looking at loader.py, it seems to me it may actually be possible to
> let it do the enumeration for us, if we stroke it the right way.
>
>
> https://reviews.llvm.org/D45215
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In https://reviews.llvm.org/D45215#1056917, @zturner wrote:

> 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 to a specific file.


Assuming we can work around the problem of not every `.py` file being a test 
(by filtering the `Test` prefix), would there be a way to differentiate the 
different test within a single file?

> Why do we need run lines?

With a custom test format these would no longer be needed.


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Zachary Turner via lldb-commits
On Wed, Apr 4, 2018 at 8:11 AM Jonas Devlieghere via Phabricator <
revi...@reviews.llvm.org> wrote:

> JDevlieghere added a comment.
>
> In https://reviews.llvm.org/D45215#1056917, @zturner wrote:
>
> > 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 to a specific file.
>
>
> Assuming we can work around the problem of not every `.py` file being a
> test (by filtering the `Test` prefix), would there be a way to
> differentiate the different test within a single file?
>
Would we need to?  dotest will just run all the tests in a single file.

I can see how it might be desirable as an end state, but not necessarily as
an incremental step.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Pavel Labath via lldb-commits
On Wed, 4 Apr 2018 at 16:47, Zachary Turner  wrote:

>
> On Wed, Apr 4, 2018 at 8:11 AM Jonas Devlieghere via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> JDevlieghere added a comment.
>>
>> In https://reviews.llvm.org/D45215#1056917, @zturner wrote:
>>
>> > 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 to a specific file.
>>
>>
>> Assuming we can work around the problem of not every `.py` file being a
>> test (by filtering the `Test` prefix), would there be a way to
>> differentiate the different test within a single file?
>>
> Would we need to?  dotest will just run all the tests in a single file.
>
> I can see how it might be desirable as an end state, but not necessarily
> as an incremental step.
>

I think I would be fine with not having test-function level resolution in
v1 of the feature *if* there is a reasonable path forward to make that
happen in the future. The RUN lines proposal seemed to make that hard if
not impossible, but with a custom test format it seems plausible to reach
that state incrementally.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Adrian Prantl via lldb-commits


> On Apr 4, 2018, at 8:53 AM, Pavel Labath  wrote:
> 
> 
> 
> On Wed, 4 Apr 2018 at 16:47, Zachary Turner  > wrote:
> 
> On Wed, Apr 4, 2018 at 8:11 AM Jonas Devlieghere via Phabricator 
> mailto:revi...@reviews.llvm.org>> wrote:
> JDevlieghere added a comment.
> 
> In https://reviews.llvm.org/D45215#1056917 
> , @zturner wrote:
> 
> > 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 to a specific file.
> 
> 
> Assuming we can work around the problem of not every `.py` file being a test 
> (by filtering the `Test` prefix), would there be a way to differentiate the 
> different test within a single file?
> Would we need to?  dotest will just run all the tests in a single file.
> 
> I can see how it might be desirable as an end state, but not necessarily as 
> an incremental step.
> 
> I think I would be fine with not having test-function level resolution in v1 
> of the feature *if* there is a reasonable path forward to make that happen in 
> the future.

I forgot about the multiple tests-per-py-file scenario. I think it is important 
to completely support this in the LIT driver. The whole point of this exercise 
is to get dotest out of the business of scheduling tests so the end-goal should 
be to have LIT recognize the individual functions as tests.

Side-question to Jonas: was your idea to run lit for each variant 
(dwarf,dwo,dsym,...) or to have dotest spawn multiple versions of each test?

> The RUN lines proposal seemed to make that hard if not impossible, but with a 
> custom test format it seems plausible to reach that state incrementally.

As an intermediate step, we could also have one RUN line per test function, but 
I see that that is a lot of work to maintain.

-- adrian___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Zachary Turner via lldb-commits
Sure, but getting lit to run one file at a time is a nice incremental step
towards that and can make both patches easier to review.
On Wed, Apr 4, 2018 at 9:02 AM Adrian Prantl  wrote:

>
> On Apr 4, 2018, at 8:53 AM, Pavel Labath  wrote:
>
>
>
> On Wed, 4 Apr 2018 at 16:47, Zachary Turner  wrote:
>
>>
>> On Wed, Apr 4, 2018 at 8:11 AM Jonas Devlieghere via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>> JDevlieghere added a comment.
>>>
>>> In https://reviews.llvm.org/D45215#1056917, @zturner wrote:
>>>
>>> > 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 to a specific file.
>>>
>>>
>>> Assuming we can work around the problem of not every `.py` file being a
>>> test (by filtering the `Test` prefix), would there be a way to
>>> differentiate the different test within a single file?
>>>
>> Would we need to?  dotest will just run all the tests in a single file.
>>
>> I can see how it might be desirable as an end state, but not necessarily
>> as an incremental step.
>>
>
> I think I would be fine with not having test-function level resolution in
> v1 of the feature *if* there is a reasonable path forward to make that
> happen in the future.
>
>
> I forgot about the multiple tests-per-py-file scenario. I think it is
> important to completely support this in the LIT driver. The whole point of
> this exercise is to get dotest out of the business of scheduling tests so
> the end-goal should be to have LIT recognize the individual functions as
> tests.
>
> Side-question to Jonas: was your idea to run lit for each variant
> (dwarf,dwo,dsym,...) or to have dotest spawn multiple versions of each test?
>
> The RUN lines proposal seemed to make that hard if not impossible, but
> with a custom test format it seems plausible to reach that state
> incrementally.
>
>
> As an intermediate step, we could also have one RUN line per test
> function, but I see that that is a lot of work to maintain.
>
> -- adrian
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Adrian Prantl via lldb-commits


> On Apr 4, 2018, at 9:07 AM, Zachary Turner  wrote:
> 
> Sure, but getting lit to run one file at a time is a nice incremental step 
> towards that and can make both patches easier to review.

Agreed, I just want to make sure that we are all on the same page as to which 
direction we want to evolve the code to.

-- adrian
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-04 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

ping


https://reviews.llvm.org/D45170



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-04 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

I have only one general question and otherwise I'm fine with this: Does this 
bring LLDB's DWARF parser closer to the structure of LLVM's? We still want to 
long-term replace LLDB's DWARF parser with LLVM's so every refactoring should 
aim at making their interfaces more similar.


https://reviews.llvm.org/D45170



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Jonas Devlieghere via lldb-commits

> On Apr 4, 2018, at 5:12 PM, Adrian Prantl  wrote:
> 
> 
> 
>> On Apr 4, 2018, at 9:07 AM, Zachary Turner  wrote:
>> 
>> Sure, but getting lit to run one file at a time is a nice incremental step 
>> towards that and can make both patches easier to review.
> 
> Agreed, I just want to make sure that we are all on the same page as to which 
> direction we want to evolve the code to.

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 a list of test for every python file 
for “v2”. I think running the different variants as separate tests is going to 
be “v3” and will require quite a bit more work. 

> 
> -- adrian

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Zachary Turner via lldb-commits
I have some ideas for how to run the different variants as separate tests,
but I'll save it for the next RFC :)

On Wed, Apr 4, 2018 at 10:24 AM Jonas Devlieghere 
wrote:

>
> > On Apr 4, 2018, at 5:12 PM, Adrian Prantl  wrote:
> >
> >
> >
> >> On Apr 4, 2018, at 9:07 AM, Zachary Turner  wrote:
> >>
> >> Sure, but getting lit to run one file at a time is a nice incremental
> step towards that and can make both patches easier to review.
> >
> > Agreed, I just want to make sure that we are all on the same page as to
> which direction we want to evolve the code to.
>
> 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 a list of test for every
> python file for “v2”. I think running the different variants as separate
> tests is going to be “v3” and will require quite a bit more work.
>
> >
> > -- adrian
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

> 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 a list of test for every
>  python file for “v2”. I think running the different variants as separate
>  tests is going to be “v3” and will require quite a bit more work.

Actually, this (v3) should happen pretty much automatically, as the test 
variant is encoded in the test method name. we have a python metaclass which 
does this replication for us, and as far as the rest of the world is concerned, 
they are just separate tests.

So if you just normally enumerate all methods in a test class, you will 
naturally get every variant in it's own test.


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

2018-04-04 Thread Zachary Turner via Phabricator via lldb-commits
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 a list of test for every
> >  python file for “v2”. I think running the different variants as separate
> >  tests is going to be “v3” and will require quite a bit more work.
>
> Actually, this (v3) should happen pretty much automatically, as the test 
> variant is encoded in the test method name. we have a python metaclass which 
> does this replication for us, and as far as the rest of the world is 
> concerned, they are just separate tests.
>
> So if you just normally enumerate all methods in a test class, you will 
> naturally get every variant in it's own test.


Yea but there are some issues with that, such as wanting to have a bit of 
common set up for each directory so that each variant doesn't have to do the 
same expensive thing over and over.

Anyway, we should discuss this when it's time to actually implement that since 
I think there are some interesting cases to consider.


https://reviews.llvm.org/D45215



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D45298: [debugserver] Fix LC_BUILD_VERSION load command handling.

2018-04-04 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision.
friss added reviewers: jasonmolenda, labath.

In one of the 2 places the LC_BUILD_VERSION load command is handled, there
is a bug preventing us from actually handling them (the address where to
read the load command was not updated). This patch factors reading the
deployment target load commands into a helper and adds testing for the 2
code paths calling the helper.

The testing is a llittle bit complicated because the only times those load
commands matter is when debugging a simulator process. I added a new
decorator to check that a specific SDK is available. The actual testing was
fairly easy once I knew how to run a simulated process.

Adding Pavel as a reviewer given I slightly modified the lldb-server test
harness.


https://reviews.llvm.org/D45298

Files:
  packages/Python/lldbsuite/test/decorators.py
  packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py
  packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
  tools/debugserver/source/DNB.cpp
  tools/debugserver/source/DNB.h
  tools/debugserver/source/MacOSX/MachProcess.h
  tools/debugserver/source/MacOSX/MachProcess.mm
  tools/debugserver/source/RNBRemote.cpp

Index: tools/debugserver/source/RNBRemote.cpp
===
--- tools/debugserver/source/RNBRemote.cpp
+++ tools/debugserver/source/RNBRemote.cpp
@@ -6091,100 +6091,18 @@
   for (uint32_t i = 0; i < mh.ncmds && !os_handled; ++i) {
 const nub_size_t bytes_read =
 DNBProcessMemoryRead(pid, load_command_addr, sizeof(lc), &lc);
-uint32_t raw_cmd = lc.cmd & ~LC_REQ_DYLD;
-if (bytes_read != sizeof(lc))
-  break;
-switch (raw_cmd) {
-case LC_VERSION_MIN_IPHONEOS:
-  os_handled = true;
-  rep << "ostype:ios;";
-  DNBLogThreadedIf(LOG_RNB_PROC,
-   "LC_VERSION_MIN_IPHONEOS -> 'ostype:ios;'");
-  break;
-
-case LC_VERSION_MIN_MACOSX:
-  os_handled = true;
-  rep << "ostype:macosx;";
-  DNBLogThreadedIf(LOG_RNB_PROC,
-   "LC_VERSION_MIN_MACOSX -> 'ostype:macosx;'");
-  break;
 
-#if defined(LC_VERSION_MIN_TVOS)
-case LC_VERSION_MIN_TVOS:
+uint32_t major_version, minor_version, patch_version;
+auto *platform = DNBGetDeploymentInfo(pid, lc, load_command_addr,
+  major_version, minor_version,
+  patch_version);
+if (platform) {
   os_handled = true;
-  rep << "ostype:tvos;";
-  DNBLogThreadedIf(LOG_RNB_PROC,
-   "LC_VERSION_MIN_TVOS -> 'ostype:tvos;'");
-  break;
-#endif
-
-#if defined(LC_VERSION_MIN_WATCHOS)
-case LC_VERSION_MIN_WATCHOS:
-  os_handled = true;
-  rep << "ostype:watchos;";
-  DNBLogThreadedIf(LOG_RNB_PROC,
-   "LC_VERSION_MIN_WATCHOS -> 'ostype:watchos;'");
-  break;
-#endif
-
-default:
+  rep << "ostype:" << platform << ";";
   break;
 }
 load_command_addr = load_command_addr + lc.cmdsize;
   }
-
-// Test that the PLATFORM_* defines are available from mach-o/loader.h
-#if defined (PLATFORM_MACOS)
-  for (uint32_t i = 0; i < mh.ncmds && !os_handled; ++i) 
-  {
-nub_size_t bytes_read =
-DNBProcessMemoryRead(pid, load_command_addr, sizeof(lc), &lc);
-uint32_t raw_cmd = lc.cmd & ~LC_REQ_DYLD;
-if (bytes_read != sizeof(lc))
-  break;
-
-if (raw_cmd == LC_BUILD_VERSION)
-{
-  uint32_t platform; // first field of 'struct build_version_command'
-  bytes_read = DNBProcessMemoryRead(pid, load_command_addr + 8, sizeof(platform), &platform);
-  if (bytes_read != sizeof (platform))
-  break;
-  switch (platform)
-  {
-  case PLATFORM_MACOS:
-  os_handled = true;
-  rep << "ostype:macosx;";
-  DNBLogThreadedIf(LOG_RNB_PROC,
-   "LC_BUILD_VERSION PLATFORM_MACOS -> 'ostype:macosx;'");
-  break;
-  case PLATFORM_IOS:
-  os_handled = true;
-  rep << "ostype:ios;";
-  DNBLogThreadedIf(LOG_RNB_PROC,
-   "LC_BUILD_VERSION PLATFORM_IOS -> 'ostype:ios;'");
-  break;
-  case PLATFORM_TVOS:
-  os_handled = true;
-  rep << "ostype:tvos;";
-  DNBLogThreadedIf(LOG_RNB_PROC,
-   "LC_BUILD_VERSION PLATFORM_TVOS -> 'ostype:tvos;'");
-  break;
-  case PLATFORM_WATCHOS:
-  os_handled = true;
-  rep << "ostype:watchos;";
-  DNBLogThreadedIf(LOG_RNB_PROC,
-   "

[Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

2018-04-04 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In https://reviews.llvm.org/D45170#1057067, @aprantl wrote:

> I have only one general question and otherwise I'm fine with this: Does this 
> bring LLDB's DWARF parser closer to the structure of LLVM's? We still want to 
> long-term replace LLDB's DWARF parser with LLVM's so every refactoring should 
> aim at making their interfaces more similar.


It does indeed. Very close.


https://reviews.llvm.org/D45170



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits