[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:19 using namespace lldb; +using namespace dwarf; shafik wrote: > JDevlieghere wrote: > > Wouldn't it be more consistent to use `lldb_private::dwarf`

Re: [Lldb-commits] [lldb] 2dc6e90 - [lldb/Host] Fix crash in FileSystem::IsLocal

FileSystem has a bunch of methods that unconditionally use `m_fs`. There's no reason `IsLocal` is special in that regard. If this crashes(which is what the radar is about) it's because the FileSystem is not properly initialized and we should figure out why instead. On Fri, Feb 25, 2022 at 6:41 PM

[Lldb-commits] [lldb] ef0de5d - [lldb] Update the CI docs

Author: Jonas Devlieghere Date: 2022-03-02T22:40:05-08:00 New Revision: ef0de5dce7ead61fc890476ee2666cb32f5125a5 URL: https://github.com/llvm/llvm-project/commit/ef0de5dce7ead61fc890476ee2666cb32f5125a5 DIFF:

[Lldb-commits] [lldb] 03dae31 - [lldb] Update TestBasicEntryValues.py for `image lookup` output

Author: Jonas Devlieghere Date: 2022-03-02T22:35:34-08:00 New Revision: 03dae31aca63cef0584dc25cc494005f1f241f99 URL: https://github.com/llvm/llvm-project/commit/03dae31aca63cef0584dc25cc494005f1f241f99 DIFF:

[Lldb-commits] [PATCH] D120792: [lldb] Fix python errors in gdbremote.py

This revision was automatically updated to reflect the committed changes. Closed by commit rGcd89f94aa904: [lldb] Fix python errors in gdbremote.py (authored by ddcc). Changed prior to commit: https://reviews.llvm.org/D120792?vs=412327=412594#toc Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [lldb] cd89f94 - [lldb] Fix python errors in gdbremote.py

Author: Dominic Chen Date: 2022-03-02T19:47:51-08:00 New Revision: cd89f94aa9048d59120d5d89ac84b5119bad45ab URL: https://github.com/llvm/llvm-project/commit/cd89f94aa9048d59120d5d89ac84b5119bad45ab DIFF: https://github.com/llvm/llvm-project/commit/cd89f94aa9048d59120d5d89ac84b5119bad45ab.diff

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:46 using namespace lldb_private; +using namespace llvm::dwarf; JDevlieghere wrote: > Why not `lldb_private::dwarf`? I think I started out with this file and must have

[Lldb-commits] [PATCH] D120792: [lldb] Fix python errors in gdbremote.py

kastiglione accepted this revision. kastiglione added a comment. This revision is now accepted and ready to land. other than the comment about the json, looks all good to me! Comment at: lldb/examples/python/gdbremote.py:1224-1225 +print(json.dumps(json_tree, indent=4,

[Lldb-commits] [lldb] 42db8bf - [lldb] Skip check for the lldb prompt in TestIOHandlerProcessSTDIO

Author: Jonas Devlieghere Date: 2022-03-02T16:44:14-08:00 New Revision: 42db8bfa20d956f06f4c19ddfa6f1688bd29f8d4 URL: https://github.com/llvm/llvm-project/commit/42db8bfa20d956f06f4c19ddfa6f1688bd29f8d4 DIFF:

[Lldb-commits] [PATCH] D120792: [lldb] Fix python errors in gdbremote.py

ddcc added inline comments. Comment at: lldb/examples/python/gdbremote.py:1224-1225 +print(json.dumps(json_tree, indent=4, separators=(',', ': '))) +except json.JSONDecodeError: +return kastiglione wrote: > I don't know this tool, but

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:35 +using namespace dwarf; using namespace std; extern int g_verbose; [[ https://llvm.org/docs/CodingStandards.html#do-not-use-using-namespace-std |

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

JDevlieghere added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:46 using namespace lldb_private; +using namespace llvm::dwarf; Why not `lldb_private::dwarf`? Comment at:

[Lldb-commits] [lldb] 77bfdeb - [lldb] Update error messages in TestMemoryHistory.py

Author: Jonas Devlieghere Date: 2022-03-02T15:56:45-08:00 New Revision: 77bfdeb092d186179efd7e032ba1c11fc0a6a444 URL: https://github.com/llvm/llvm-project/commit/77bfdeb092d186179efd7e032ba1c11fc0a6a444 DIFF:

[Lldb-commits] [PATCH] D120766: [lldb] Devirtualize IOHandler::{IsActive, SetIsDone, GetIsDone} (NFC)

This revision was automatically updated to reflect the committed changes. Closed by commit rG37eb15ad7ab2: [lldb] Devirtualize IOHandler::{IsActive,SetIsDone,GetIsDone} (NFC) (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

This revision was automatically updated to reflect the committed changes. Closed by commit rG10222764a9a3: [lldb] Avoid data race in IOHandlerProcessSTDIO (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D120762?vs=412479=412567#toc

[Lldb-commits] [lldb] 1022276 - [lldb] Avoid data race in IOHandlerProcessSTDIO

Author: Jonas Devlieghere Date: 2022-03-02T15:55:49-08:00 New Revision: 10222764a9a3b09678013d3d13b66790ea3298d9 URL: https://github.com/llvm/llvm-project/commit/10222764a9a3b09678013d3d13b66790ea3298d9 DIFF:

[Lldb-commits] [lldb] 37eb15a - [lldb] Devirtualize IOHandler::{IsActive, SetIsDone, GetIsDone} (NFC)

Author: Jonas Devlieghere Date: 2022-03-02T15:55:49-08:00 New Revision: 37eb15ad7ab25e0135b759f3ecdd6ada5ce82330 URL: https://github.com/llvm/llvm-project/commit/37eb15ad7ab25e0135b759f3ecdd6ada5ce82330 DIFF:

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

jingham added a comment. Excellent, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119963/new/ https://reviews.llvm.org/D119963 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik updated this revision to Diff 412548. shafik added a comment. Updating diff based on comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120836/new/ https://reviews.llvm.org/D120836 Files: lldb/include/lldb/Core/dwarf.h lldb/include/lldb/Symbol/DWARFCallFrameInfo.h

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

zequanwu added a comment. In D119963#3355563 , @jingham wrote: > In D119963#3355501 , @zequanwu > wrote: > >> Address comments > > This sort of thing is hard to check in the option groups w/o blowing out the >

[Lldb-commits] [lldb] 302d717 - [LLDB] Add error message when using --show-variable-ranges without -v

Author: Zequan Wu Date: 2022-03-02T14:20:14-08:00 New Revision: 302d7179e101ebbf48c386a632828a1150a18769 URL: https://github.com/llvm/llvm-project/commit/302d7179e101ebbf48c386a632828a1150a18769 DIFF: https://github.com/llvm/llvm-project/commit/302d7179e101ebbf48c386a632828a1150a18769.diff

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

JDevlieghere added a comment. I think you might be looking at a combination of the old and the new patch. The new mutex protects the whole `Cancel` and `SetIsRunning`. I don't think this needs to be a recursive mutex because these functions are not calling each other. They both indirectly

[Lldb-commits] [PATCH] D120755: Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only wait

clayborg updated this revision to Diff 412539. clayborg added a comment. Fixes: - use a "configuration_done_sent" boolean to track when the configurationDone packet has already been sent and stop all process events from being sent before this happens. This allows "launchCommands" and

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

jingham added a comment. In D119963#3355501 , @zequanwu wrote: > Address comments This sort of thing is hard to check in the option groups w/o blowing out the number of groups you need to have. The way to check after all the option parsing is done

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

This revision was automatically updated to reflect the committed changes. Closed by commit rG15983c28aa81: [LLDB] Dump valid ranges of variables (authored by zequanwu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119963/new/

[Lldb-commits] [lldb] 15983c2 - [LLDB] Dump valid ranges of variables

Author: Zequan Wu Date: 2022-03-02T13:44:19-08:00 New Revision: 15983c28aa819031e08a2b3fe49d02c41839b22c URL: https://github.com/llvm/llvm-project/commit/15983c28aa819031e08a2b3fe49d02c41839b22c DIFF: https://github.com/llvm/llvm-project/commit/15983c28aa819031e08a2b3fe49d02c41839b22c.diff

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

zequanwu added inline comments. Comment at: lldb/source/Commands/Options.td:962 +"\\x01">, GroupRange<1, 6>, Desc<"Dump valid ranges of variables (must be " +"used in conjunction with --verbose">; def target_modules_lookup_verbose : Option<"verbose", "v">,

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

zequanwu updated this revision to Diff 412532. zequanwu marked 3 inline comments as done. zequanwu added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119963/new/ https://reviews.llvm.org/D119963 Files:

[Lldb-commits] [lldb] 4977da2 - [lldb] Explicitly declare the default constructor in PlatformAndroidRemoteGDBServer

Author: Pavel Labath Date: 2022-03-02T22:35:54+01:00 New Revision: 4977da2c555b0a8d49be68bad4c54b816d7cc908 URL: https://github.com/llvm/llvm-project/commit/4977da2c555b0a8d49be68bad4c54b816d7cc908 DIFF: https://github.com/llvm/llvm-project/commit/4977da2c555b0a8d49be68bad4c54b816d7cc908.diff

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

clayborg added a comment. How is using m_mutex better than just using the std::atomic? Just protecting the modification of the value with a mutex doesn't seem like it would do much more than the atomic already did unless we are using the lock the protect the value for longer that just the

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

labath added a comment. In D120836#3355245 , @shafik wrote: > In D120836#3355167 , @labath wrote: > >> I think it's reasonable to be able to refer to the dwarf constants from >> within the dwarf plugin via their

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik added a comment. In D120836#3355167 , @labath wrote: > I think it's reasonable to be able to refer to the dwarf constants from > within the dwarf plugin via their base names alone. The implementation -- a > top-level `using namespace

[Lldb-commits] [PATCH] D119146: [lldb/Platform] Prepare decouple instance and plugin names

stella.stamenova added a comment. It looks like this broke the windows lldb bot: https://lab.llvm.org/buildbot/#/builders/83/builds/15999 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119146/new/ https://reviews.llvm.org/D119146

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

tschuett added a comment. I found this: https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120836/new/ https://reviews.llvm.org/D120836

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

mgorny added inline comments. Herald added a project: All. Comment at: lldb/packages/Python/lldbsuite/test/lldbpexpect.py:27 +def launch(self, executable=None, extra_args=None, timeout=60, +dimensions=None, run_under=None, post_spawn=None): logfile =

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. Thanks for the test. Comment at: lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py:16 +self.build() +

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

labath added a comment. I think it's reasonable to be able to refer to the dwarf constants from within the dwarf plugin via their base names alone. The implementation -- a top-level `using namespace llvm::dwarf` -- is not reasonable, but that's because the plugin is very old, and completely

[Lldb-commits] [PATCH] D120594: Improve error messages for command that need a stopped process

This revision was automatically updated to reflect the committed changes. Closed by commit rGdaba82362228: Refine error msgs from CommandObject Disassemble (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120594/new/

[Lldb-commits] [lldb] daba823 - Refine error msgs from CommandObject & Disassemble

Author: Jason Molenda Date: 2022-03-02T11:17:48-08:00 New Revision: daba82362228b4aa460c26079c028ebf832066fd URL: https://github.com/llvm/llvm-project/commit/daba82362228b4aa460c26079c028ebf832066fd DIFF: https://github.com/llvm/llvm-project/commit/daba82362228b4aa460c26079c028ebf832066fd.diff

[Lldb-commits] [lldb] 7cdda6b - Revert "[cmake] Prefix gtest and gtest_main with "llvm_"."

Author: Stella Laurenzo Date: 2022-03-02T11:13:46-08:00 New Revision: 7cdda6b8ce49ae3c90c068cff4dc355bba5d77f2 URL: https://github.com/llvm/llvm-project/commit/7cdda6b8ce49ae3c90c068cff4dc355bba5d77f2 DIFF:

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

kastiglione added a comment. sorry Shafik I see that you've asked that very question. I'll give a +1 to making use of `using namespace` within .cpp files, especially where a namespace is used pervasively. When a file uses it only a namespace infrequently, I think explicit is reasonable.

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

kastiglione added a comment. For the .cpp files with hundreds of lines modified, what do you think about adding `using namespace llvm::dwarf;` to those? For starters `DWARFExpression.cpp`, `DWARFAbbreviationDeclaration.cpp`, and `SymbolFileDWARF.cpp`. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

shafik added a comment. I used `using namespace` in a few functions but I am not committed to this approach. So I am happy to hear feedback on whether we want to just use fully qualified names everywhere instead or nail down a criteria as to when it is acceptable. We also have `using

[Lldb-commits] [lldb] 9f37775 - [cmake] Prefix gtest and gtest_main with "llvm_".

Author: Stella Laurenzo Date: 2022-03-02T10:53:32-08:00 New Revision: 9f37775472b45986b0ecce5243bd6ce119e5bd69 URL: https://github.com/llvm/llvm-project/commit/9f37775472b45986b0ecce5243bd6ce119e5bd69 DIFF:

[Lldb-commits] [PATCH] D120594: Improve error messages for command that need a stopped process

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120594/new/ https://reviews.llvm.org/D120594

[Lldb-commits] [PATCH] D120718: Fix DataExtractor symbol conflict

JDevlieghere added a comment. Thanks for cleaning up the implementation file. LGMT module removing the added `lldb_private::` which I believe we no longer need. Comment at: lldb/include/lldb/Core/DataFileCache.h:164 /// True if the signature was successfully decoded,

[Lldb-commits] [PATCH] D120810: [lldb] Remove the global platform list

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. It's always great to see changes that enable more testing. I left one inline comment/nit but besides that this LGTM. Comment at:

[Lldb-commits] [PATCH] D120755: Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only wait

yinghuitan added inline comments. Comment at: lldb/tools/lldb-vscode/VSCode.cpp:532-533 +lldb::SBError VSCode::WaitForProcessToStop(uint32_t seconds) { + // Wait for the process hit a stopped state. When running a launch (with or + // without "launchCommands") or attach (with

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

JDevlieghere updated this revision to Diff 412479. JDevlieghere added a comment. Unsurprisingly no tests failed with the typo. Added a test case to cover reading from stdin through the `IOHandlerProcessSTDIO`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120762/new/

[Lldb-commits] [PATCH] D120792: [lldb] Fix python errors in gdbremote.py

kastiglione added inline comments. Comment at: lldb/examples/python/gdbremote.py:1224-1225 +print(json.dumps(json_tree, indent=4, separators=(',', ': '))) +except json.JSONDecodeError: +return I don't know this tool, but should it print the

[Lldb-commits] [PATCH] D120803: [lldb] Don't print *trailing* nuls in char arrays

kastiglione accepted this revision. kastiglione added a comment. This revision is now accepted and ready to land. thanks for doing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120803/new/ https://reviews.llvm.org/D120803

[Lldb-commits] [PATCH] D120810: [lldb] Remove the global platform list

labath created this revision. labath added reviewers: JDevlieghere, clayborg, jingham. Herald added subscribers: mgorny, emaste. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. This patch moves the platform creation and selection logic into the

[Lldb-commits] [PATCH] D119146: [lldb/Platform] Prepare decouple instance and plugin names

This revision was automatically updated to reflect the committed changes. Closed by commit rGd2edca6276d1: [lldb/Platform] Prepare decouple instance and plugin names (authored by labath). Herald added a project: All. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [lldb] d2edca6 - [lldb/Platform] Prepare decouple instance and plugin names

Author: Pavel Labath Date: 2022-03-02T14:57:01+01:00 New Revision: d2edca6276d1715a02d1144eae577b3d79673d67 URL: https://github.com/llvm/llvm-project/commit/d2edca6276d1715a02d1144eae577b3d79673d67 DIFF: https://github.com/llvm/llvm-project/commit/d2edca6276d1715a02d1144eae577b3d79673d67.diff

[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

emrekultursay added a comment. In D120718#3352450 , @JDevlieghere wrote: > I think the better solution here is to get rid of the `using namespace llvm;` > in the implementation file instead. Done. PTAL. Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

emrekultursay updated this revision to Diff 412361. emrekultursay added a comment. Removed "using namespace llvm" from the implementation file and qualified all associated references with "llvm::". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D120803: [lldb] Don't print *trailing* nuls in char arrays

labath created this revision. labath added reviewers: kastiglione, teemperor, jingham. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. Embedded nul characters are still printed, and they don't terminate the string. See also D111634

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

labath added a comment. In D120762#3353686 , @JDevlieghere wrote: > In D120762#3353655 , @labath wrote: > >> Are you sure that we're still sending input to the process (I'm not sure how >> much test coverage

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

JDevlieghere added a comment. In D120762#3353655 , @labath wrote: > Are you sure that we're still sending input to the process (I'm not sure how > much test coverage for this do we have)? I'll rerun the tests tomorrow with my typo and see if anything

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

JDevlieghere updated this revision to Diff 412337. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. Fix typo in `SetIsRunning`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120762/new/ https://reviews.llvm.org/D120762 Files:

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/Target/Process.cpp:4343 +std::lock_guard guard(m_mutex); +if (GetIsDone()) + break; labath wrote: > I'm confused. How come this

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

labath added a comment. This seems like a better approach, but I'm not sure how does it actually work (see inline comment). Are you sure that we're still sending input to the process (I'm not sure how much test coverage for this do we have)? Comment at:

[Lldb-commits] [PATCH] D120755: Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only wait

labath added a comment. Herald added a project: All. I'm not exactly exactly thrilled by the sleep-based implementation, but otherwise, the patch seems fine. Comment at: lldb/tools/lldb-vscode/VSCode.cpp:551 + auto timeout_time = +