[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14 +# CHECK-AFTER: ^running +# CHECK-AFTER: *stopped,reason="breakpoint-hit" + polyakov.alex wrote: > aprantl wrote: > > CHECK-AFTER is not recognized by FileCheck: > > > >

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14 +# CHECK-AFTER: ^running +# CHECK-AFTER: *stopped,reason="breakpoint-hit" + aprantl wrote: > CHECK-AFTER is not recognized by FileCheck: > > https://www.llvm.org/d

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14 +# CHECK-AFTER: ^running +# CHECK-AFTER: *stopped,reason="breakpoint-hit" + CHECK-AFTER is not recognized by FileCheck: https://www.llvm.org/docs/CommandGuide/FileCheck.

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + labath wrote: > polyakov.alex wrote: > > labath wrote: > > > polyakov.alex wrote: > > > > labath wrote: > > >

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. For #2 we should be able to come up with something much more trivial than pexpect (and thereby more reliable) because we really aren't expecting patterns. We are always sending a complete string reading a complete string back, then checking the string against some patt

[Lldb-commits] [PATCH] D46885: Remove append parameter to FindGlobalVariables

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. I couldn't find any additional uses of FindGlobalVariables in swift-lldb either. https://reviews.llvm.org/D46885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-c

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thank you. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Submitted the LLVM patch: https://reviews.llvm.org/D46887 https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. In https://reviews.llvm.org/D46588#1099566, @clayborg wrote: > In https://reviews.llvm.org/D46588#1099536, @aprantl wrote: > > > > If we're not able to come up with a command to make lldb-mi wait until > > > the target stops (maybe there is one already? I know very littl

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, JDevlieghere. Herald added subscribers: aprantl, mgorny. This places the `if(m_using_apple_tables)` branches inside the SymbolFileDWARF class behind an abstract DWARFIndex class. The class currently has two implementations: - AppleIn

[Lldb-commits] [PATCH] D46888: [DWARF] Have HashedNameToDIE store a DataExtractor by value

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, JDevlieghere. Herald added a subscriber: aprantl. The DataExtractors are cheap to copy so there is no reason to store them by reference. Also, in my upcoming indexing refactor I am planning to remove the apple tables data extractor me

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. If those are the only tests that fail, then I'd still go for it, as I still firmly believe this is the correct behavior for such a function. Such a low level test does not have to mean that someone really cares about this, it could be more of a case of documenting existi

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. So I made a fix to LLVM and there are tests that are testing for the empty string: [ RUN ] Support.RemoveDots /Users/gclayton/Documents/src/llvm/clean/llvm/unittests/Support/Path.cpp:1149: Failure Expected: "" To be equal to: remove_dots(".\\", false,

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. (Also, the function already does not remove leading `..` components, so there is precedent already for not removing stuff when it causes the path to become not equivalent.) https://reviews.llvm.org/D46783 ___ lldb-commits m

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D46783#1099561, @clayborg wrote: > So the function in llvm is called llvm::sys::path::remove_dots(...) and it is > removing the dots. Not sure it is correct to be changing a function that says > "remove_dots" to not remove dots and actually re

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In https://reviews.llvm.org/D46783#1097549, @labath wrote: > Preserving the dot if it is the only path component is perfectly reasonable > -- "" is not a valid path and we shouldn't convert a valid path into an > invalid one. However, I think this should be done on the

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In https://reviews.llvm.org/D46588#1099536, @aprantl wrote: > > If we're not able to come up with a command to make lldb-mi wait until the > > target stops (maybe there is one already? I know very little about > > lldb-mi), we may have to revisit the whole testing stra

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. So the function in llvm is called llvm::sys::path::remove_dots(...) and it is removing the dots. Not sure it is correct to be changing a function that says "remove_dots" to not remove dots and actually return something with a . in it... Seems like this should be taken

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. > If we're not able to come up with a command to make lldb-mi wait until the > target stops (maybe there is one already? I know very little about lldb-mi), > we may have to revisit the whole testing strategy... If one doesn't exist then I think it would be reasonable to

[Lldb-commits] [lldb] r332353 - Reapply "Remove Process references from the Host module"

2018-05-15 Thread Pavel Labath via lldb-commits
Author: labath Date: Tue May 15 06:42:26 2018 New Revision: 332353 URL: http://llvm.org/viewvc/llvm-project?rev=332353&view=rev Log: Reapply "Remove Process references from the Host module" This re-lands r332250/D46395, after fixing Mac build errors. Modified: lldb/trunk/include/lldb/Host/Ho

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + polyakov.alex wrote: > labath wrote: > > polyakov.alex wrote: > > > labath wrote: > > > > I'm not familiar with this

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + labath wrote: > polyakov.alex wrote: > > labath wrote: > > > I'm not familiar with this directive. Are you sur

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + polyakov.alex wrote: > labath wrote: > > I'm not familiar with this directive. Are you sure that this actually does

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + labath wrote: > I'm not familiar with this directive. Are you sure that this actually does > anything? I trie

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + I'm not familiar with this directive. Are you sure that this actually does anything? Repository: rL LLVM https:

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added a comment. Also, I combined FileCheck commands and lldb-mi input in a single file. Repository: rL LLVM https://reviews.llvm.org/D46588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 146777. polyakov.alex added a comment. Created separate directory for an lldb-mi tests. Repository: rL LLVM https://reviews.llvm.org/D46588 Files: lit/lit.cfg lit/tools/lldb-mi/breakpoint/break-insert.test lit/tools/lldb-mi/breakpoint/inputs/

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Adding a test for that in lldb is fine. If we're relying on some behavior we want to make sure it doesn't change without us noticing. However, I don't think we should make any special allowments for people using lldb with older versions of llvm. It sends the wrong messag

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Trying to be smart while being lazy and multithreaded is going to make the code complicated (possibility for bugs) and/or introduce a lot of locking overhead. A lot simpler solution is to let the caller decide if it want's the full CU or just the root DIE, and then make

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

2018-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lit/Breakpoint/Inputs/break-insert.input:1-3 +-break-insert breakpoint +-file-exec-and-symbols a.out +-exec-run Based on my experiments, lldb-mi seems to ignore lines starting with `#`. If that's true then we could put t