[Lldb-commits] [PATCH] D59913: Get Version SWIG wrapper should fill the list it makes

labath added a comment. The fix looks correct, but I am wondering if it wouldn't be possible to test this in a more platform-independent manner. Please see inline comment below. Comment at: packages/Python/lldbsuite/test/macosx/version_zero/TestGetVersionZeroVersion.py:1-49 +

Re: [Lldb-commits] [lldb] r357141 - Copy the breakpoint site owner's collection so we can drop

On 28/03/2019 02:54, Davide Italiano via lldb-commits wrote: On Wed, Mar 27, 2019 at 6:49 PM Jim Ingham via lldb-commits wrote: Author: jingham Date: Wed Mar 27 18:51:33 2019 New Revision: 357141 URL: http://llvm.org/viewvc/llvm-project?rev=357141&view=rev Log: Copy the breakpoint site owner'

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

teemperor updated this revision to Diff 192588. teemperor marked an inline comment as done. teemperor added a comment. - Addressed (most of) Aleksei's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59485/new/ https://reviews.llvm.org/D59485 Files: clang/include/clang/AST/AST

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

teemperor added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:590 + new RedirectingImporter(ToContext, ToFileManager, FromContext, + FromFileManager, MinimalImport, LookupTabl)); +}; a_sidorin w

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:590 + new RedirectingImporter(ToContext, ToFileManager, FromContext, + FromFileManager, MinimalImport, LookupTabl)); +}; teemperor wro

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

teemperor updated this revision to Diff 192594. teemperor added a comment. Ah, I get it now. Fixed! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59485/new/ https://reviews.llvm.org/D59485 Files: clang/include/clang/AST/ASTImporter.h clang/lib/AST/ASTImporter.cpp clang/unittests/

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

labath added a comment. Thank you for writing this up. I think there is a lot of confusion around this topic, and a document like this is a good step towards clearing it up. Not really related to this patch, but since we're talking about lldb_assert... The thing that has always bugged me about

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. Herald added a project: LLDB. Anyone? We still have this patch applied on our recently-rebased fork with no problems... Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53412/new/ https://reviews.llvm.org/D53412

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. Or better yet, create a structure that everyone must use and have the locking exist in the class itself: struct ProcessLocker { std::lock_guard guard; Process::StopLocker stop_locker; void ProcessLocker(Process &process) { ... } }; Repo

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. @clayborg, I'm not sure how that would work. There's many places that lock the process run lock without locking the target API mutex, and vice versa. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53412/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

This revision was automatically updated to reflect the committed changes. Closed by commit rL357188: Regression test to ensure that we handling importing of std::vector of enums… (authored by shafik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pr

[Lldb-commits] [lldb] r357188 - Regression test to ensure that we handling importing of std::vector of enums correctly

Author: shafik Date: Thu Mar 28 10:22:13 2019 New Revision: 357188 URL: http://llvm.org/viewvc/llvm-project?rev=357188&view=rev Log: Regression test to ensure that we handling importing of std::vector of enums correctly Summary: https://reviews.llvm.org/D59845 added a fix for the IsStructuralMat

[Lldb-commits] [PATCH] D59913: Get Version SWIG wrapper should fill the list it makes

jingham updated this revision to Diff 192677. jingham added a comment. Use yaml2obj to create the dylib with version 0.0.0 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59913/new/ https://reviews.llvm.org/D59913 Files: packages/Python/lldbsuite/test/macosx/

[Lldb-commits] [lldb] r357198 - [NFC] find_first_of/find_last_of -> find/rfind for single char.

Author: jdevlieghere Date: Thu Mar 28 11:10:14 2019 New Revision: 357198 URL: http://llvm.org/viewvc/llvm-project?rev=357198&view=rev Log: [NFC] find_first_of/find_last_of -> find/rfind for single char. For a single char argument, find_first_of is equal to find and find_last_of is equal to rfind.

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. In D53412#1446252 , @cameron314 wrote: > @clayborg, I'm not sure how that would work. There's many places that lock > the process run lock without locking the target API mutex, and vice versa. Add an argument to the ProcessLock

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

stella.stamenova added a comment. This is causing failures on the windows bot. Please fix it or revert it. http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/3066 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59847/new/ https://reviews.llvm.org/D5984

[Lldb-commits] [lldb] r357207 - Fix the swig typemap for "uint32_t *versions, uint32_t num_versions".

Author: jingham Date: Thu Mar 28 12:25:54 2019 New Revision: 357207 URL: http://llvm.org/viewvc/llvm-project?rev=357207&view=rev Log: Fix the swig typemap for "uint32_t *versions, uint32_t num_versions". It was making a list of a certain size but not always filling in that many elements, which wo

[Lldb-commits] [PATCH] D59913: Get Version SWIG wrapper should fill the list it makes

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB357207: Fix the swig typemap for "uint32_t *versions, uint32_t num_versions". (authored by jingham, committed by ). Re

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. There's dozens of places that take the API mutex without taking the process mutex. Take `Kill` for example: It needs to take the API mutex, but cannot take the run lock since it will be taken by the private state thread. Another example is `HandleCommand`, which take

Re: [Lldb-commits] [lldb] r357141 - Copy the breakpoint site owner's collection so we can drop

Does https://reviews.llvm.org/D59957 look right? I hadn't used this before but it seems to do the right thing. Jim > On Mar 28, 2019, at 2:01 AM, Pavel Labath wrote: > > On 28/03/2019 02:54, Davide Italiano via lldb-commits wrote: >> On Wed, Mar 27, 2019 at 6:49 PM Jim Ingham via lldb-com

[Lldb-commits] [PATCH] D59957: Convert = operators that take object mutexes to the multi-lock version of std::lock

jingham created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I made a copy-paste error copying the ModuleList trick for avoiding priority inversion in taking the lhs & rhs mutexes. Pavel pointed out you can use the std::lock(lock1, lock2) form to do this

[Lldb-commits] [PATCH] D59957: Convert = operators that take object mutexes to the multi-lock version of std::lock

davide added subscribers: labath, davide. davide added a comment. This looks correct to me, but I'm not extremely familiar either, so I'd wait for @labath to sign off. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59957/new/ https://reviews.llvm.org/D59957

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

shafik added a comment. In D59847#1446571 , @stella.stamenova wrote: > This is causing failures on the windows bot. Please fix it or revert it. > > http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/3066 I think I know why this is breaking,

[Lldb-commits] [lldb] r357210 - Fix for regression test, since we rely on the formatter for std::vector in the test we need a libc++ category.

Author: shafik Date: Thu Mar 28 13:25:57 2019 New Revision: 357210 URL: http://llvm.org/viewvc/llvm-project?rev=357210&view=rev Log: Fix for regression test, since we rely on the formatter for std::vector in the test we need a libc++ category. See differential https://reviews.llvm.org/D59847 for

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

shafik added a comment. @stella.stamenova I committed a fix, please let me know if this does not address the regression: http://llvm.org/viewvc/llvm-project?view=revision&revision=357210 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59847/new/ https://reviews.ll

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

stella.stamenova added a comment. In D59847#1446671 , @shafik wrote: > @stella.stamenova I committed a fix, please let me know if this does not > address the regression: > > http://llvm.org/viewvc/llvm-project?view=revision&revision=357210 @shafik Thank

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl updated this revision to Diff 192708. aprantl added a comment. Address review feedback! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 Files: lldb/docs/index.rst lldb/docs/resources/source.rst lldb/source/Utility/LLDBAssert.cpp

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. While writing this I came to the conclusion that `lldb_assert()` is really a lazy way of handling errors. If we can we should replace it with error handling and perhaps logging. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

shafik created this revision. shafik added reviewers: jingham, teemperor. Herald added a subscriber: jdoerfert. In an Objective-C context a local variable and namespace can cause an ambiguous name lookup when used in an expression. The solution involves mimicking the existing C++ solution which

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide added a comment. My (maybe unpopolar) opinion on the subject is that "soft assertions" are a way to cleanse your conscience of guilt, but they don't work really well in practice. When I started working on lldb, I was a fairly strong proponent of assertions everywhere. My view changed som

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

friss added inline comments. Comment at: source/Expression/ExpressionSourceCode.cpp:171 if (!var_name || var_name == ConstString("this") || +var_name == ConstString("self") || var_name == ConstString("_cmd") || var_name == ConstString(".block_descriptor")) -

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. Thanks for summarizing your thoughts, Davide. I think that what you wrote is a much better explanation of what I was trying to say with Use these sparingly and only if error handling is not otherwise feasible. I think that unless we can eliminate all uses of lldb_ass

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide added a comment. In D59911#1446787 , @aprantl wrote: > Thanks for summarizing your thoughts, Davide. > > I think that what you wrote is a much better explanation of what I was trying > to say with > > Use these sparingly and only if error handlin

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

xiaobai added a comment. Thank you for taking the time to write this up! I wish I could have read this when I started working on LLDB. :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 ___ lldb-com

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl updated this revision to Diff 192730. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 Files: lldb/docs/index.rst lldb/docs/resources/source.rst lldb/source/Utility/LLDBAssert.cpp lldb/www/source.html Index: lldb/www/source.html ==

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide accepted this revision. davide added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. In D59911#1446835 , @davide wrote: > > I do personally believe that your wording is reasonable. If it was me, I > would just be a little stronger and say that new code should never use > lldbassert, and if you're touching exi

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Works for me. In D59911#1445917 , @labath wrote: > In D59911#1445392 , @JDevlieghere > wrote: > >

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

jingham added a comment. In D59911#1446745 , @davide wrote: > My (maybe unpopolar) opinion on the subject is that "soft assertions" are a > way to cleanse your conscience of guilt, but they don't work really well in > practice. > When I started working

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

zturner added a comment. In D59911#1446942 , @jingham wrote: > In D59911#1446745 , @davide wrote: > > > My (maybe unpopolar) opinion on the subject is that "soft assertions" are a > > way to cleanse your conscience

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Hello Raphael, I think we should accept this change. I don't see an easy way to merge the LLDB stuff into ASTImporter; also, this patch provides a good extension point for ASTImporter si

[Lldb-commits] [PATCH] D59968: [Cmake] Unify python variables

JDevlieghere created this revision. JDevlieghere added reviewers: zturner, stella.stamenova, labath, sgraenitz. Herald added subscribers: lldb-commits, abidh, mgorny. Herald added a project: LLDB. Both FindPythonInterp and FindPythonLibs do two things, they'll set some variables (PYTHON_LIBRARIES

[Lldb-commits] [PATCH] D59968: [Cmake] Unify python variables

stella.stamenova accepted this revision. stella.stamenova added a comment. This revision is now accepted and ready to land. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59968/new/ https://reviews.llvm.org/D59968

[Lldb-commits] [PATCH] D59970: [CMake] Untangle linking of dependencies

JDevlieghere updated this revision to Diff 192745. JDevlieghere added a comment. And move python to the ScriptInterpreter CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59970/new/ https://reviews.llvm.org/D59970 Files: lldb/source/Core/CMakeLists.txt lldb/source/Plugins/ScriptInterp

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

jingham added a comment. In D59911#1446946 , @zturner wrote: > In D59911#1446942 , @jingham wrote: > > > In D59911#1446745 , @davide wrote: > > > > > My (maybe unpopolar) opi

[Lldb-commits] [PATCH] D59972: [Python] Remove readline module

JDevlieghere created this revision. JDevlieghere added a reviewer: labath. Herald added subscribers: jdoerfert, mgorny. Herald added a project: LLDB. Todd added this empty readline module to workaround an issue with an old version of Python on Ubuntu in 2014 (18841). In the meantime, Python seems

[Lldb-commits] [PATCH] D59970: [CMake] Untangle linking of dependencies

JDevlieghere created this revision. JDevlieghere added reviewers: zturner, stella.stamenova, labath, sgraenitz. Herald added a subscriber: mgorny. Herald added a project: LLDB. The utility library shouldn't depend on curses, libedit or python since it uses none of them. Move the first two to libC

[Lldb-commits] [PATCH] D59976: [Python] Remove Python include from ScriptInterpreterPython.h

JDevlieghere created this revision. JDevlieghere added reviewers: zturner, davide. Herald added a subscriber: abidh. Herald added a project: LLDB. This patch limits the scope of the python header to the python script interpreter plugin by removing the include from ScriptInterpreterPython.h. To m

[Lldb-commits] [PATCH] D59957: Convert = operators that take object mutexes to the multi-lock version of std::lock

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks great. Thank you. As of c++17 there will be an RAII way to do this https://en.cppreference.com/w/cpp/thread/scoped_lock. Theoretically we could jump ahead, and implement something

[Lldb-commits] [PATCH] D59968: [Cmake] Unify python variables

labath added inline comments. Comment at: lldb/cmake/modules/LLDBConfig.cmake:246-247 else() -find_package(PythonLibs REQUIRED) +include(FindPythonInterp) +include(FindPythonLibs) endif() Is there any difference between `include(FindPythonInterp