Page MenuHomePhabricator

[lldb-mi] Re-implement data-info-line command.
ClosedPublic

Authored by apolyakov on Jul 8 2018, 3:56 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

apolyakov created this revision.Jul 8 2018, 3:56 PM
aprantl added inline comments.Jul 9 2018, 9:05 AM
packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
376 ↗(On Diff #154534)

Why are you changing the regexp here? '.' matches any character, so to match the literal . you need to escape it with a backslash.

tools/lldb-mi/MICmdCmdData.cpp
28 ↗(On Diff #154534)

I don't think that comment is necessary.

1691 ↗(On Diff #154534)

@clayborg: Is there something better we could do instead of doing a linear search through all debug line entries?

apolyakov updated this revision to Diff 154623.Jul 9 2018, 9:26 AM

Returned accidentally removed backslashes in self.expect pattern, removed comment from:
#include <string> // For std::to_string.

apolyakov added inline comments.Jul 9 2018, 10:04 AM
tools/lldb-mi/MICmdCmdData.cpp
1691 ↗(On Diff #154534)

I think we can get rid of this by adding a new method to SBModule - ResolveSymbolContextsForFileSpec, which will use a method with the same from Module.

aprantl added inline comments.Jul 9 2018, 11:15 AM
packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
22 ↗(On Diff #154623)

It would be *awesome* if we could also convert this entire to a lit/FileCheck. Looks like all the tests in this file are basically skipped everywhere because it's so unreliable...

apolyakov added inline comments.Jul 9 2018, 11:26 AM
packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
22 ↗(On Diff #154623)

It's problematic to convert data-info-line test to a lit one since we don't know addresses of a source lines. It means that we can't do this:
-data-info-line *0xsome_address

aprantl added inline comments.Jul 11 2018, 11:12 PM
packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py
22 ↗(On Diff #154623)

I see. You are right, we can't change make one command input depend on previous output in a LIT-based test. But it looks like this is only needed for testing one form of the data-info-line command. We could extract everything else into a LIT-based test that runs on every platform and still greatly improve the test coverage.

It would be good to convert to LIT as more tests as we can. I'll do this in one of a next patches. Let's continue with this one?

aprantl accepted this revision.Jul 14 2018, 12:35 AM
This revision is now accepted and ready to land.Jul 14 2018, 12:35 AM
apolyakov updated this revision to Diff 156299.Jul 19 2018, 9:59 AM

Converted data-info-line python test to a lit one.

This revision was automatically updated to reflect the committed changes.