[Lldb-commits] [PATCH] D143690: [lldb] Add the ability to provide a message to a progress event update

bulbazord accepted this revision. bulbazord added a comment. Thanks for including the motivation in your summary, that made following this change a lot easier. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143690/new/ https://reviews.llvm.org/D143690

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord updated this revision to Diff 496299. bulbazord added a comment. Stop exposing unsupported SBReproducer functions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142926/new/ https://reviews.llvm.org/D142926 Files: lldb/bindings/CMakeList

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143623/new/ https://reviews.llvm.org/D143623 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

bulbazord requested changes to this revision. bulbazord added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Commands/CommandObjectLog.cpp:180-184 +if ((m_options.handler != eLogHandlerCircular && + m_options.handler != eLogHan

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. Not sure how useful it would be but I recorded the full list of methods get added with this change. Take a look and let me know if there are any that you think shouldn't be added. SBAttachInfo::~SBAttachInfo() SBBroadcaster::operator<(const lldb::SBBroadcaster &)

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. In D142926#4113600 , @clayborg wrote: > In D142926#4111717 , @bulbazord > wrote: > >> Certainly interesting: >> >> - SBListener::StopListeningForEventClass return type conflicts (ABI br

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord updated this revision to Diff 495978. bulbazord added a comment. - Removed a bunch of header guards effectively adding methods to the python bindings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142926/new/ https://reviews.llvm.org/D14

[Lldb-commits] [PATCH] D143548: [lldb] Add the ability to remove diagnostic callbacks

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143548/new/ https://reviews.llvm.org/D143548 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. In addition to what I wrote above, I also fixed several documentation bugs (putting docstrings on the wrong method). I manually audited the generated code before and after this change. Here are my notes: Not too interesting: - Some parameter names changed - Fixed lo

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord updated this revision to Diff 495695. bulbazord added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: Michael137, sstefan1. First pass at a complete implementation. Actions taken: - Added swig flags to ignore swig warnings indicating that swig is ignoring certa

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord added a comment. In D142926#4102287 , @labath wrote: > I like this. > > I'm not sure what it would take, but I think it'd be nice if the > de-swig-ification was not specific to the framework build. Ideally, I'd make > it controlled by a separa

[Lldb-commits] [PATCH] D143282: [lldb] Accept negative indexes in __getitem__

bulbazord added a comment. A simple test would be what you did in your commit summary, trying to get `thread.frame[-1]` and verifying that it's the correct frame. LGTM with a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143282/new/ https://

[Lldb-commits] [PATCH] D143236: [lldb] Add a way to get a scripted process implementation from the SBAPI

bulbazord added a comment. This is an interesting idea. I suppose I find it a little strange that something from the SB API is passing a void * back to the caller instead of something more well defined (like another SB class or some primitive) but I suppose that's not the worst thing in the wo

[Lldb-commits] [PATCH] D143232: Return an error when the CFA resolves to no known register, instead of segfaulting

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM. I wonder if there's a good way to exercise this with a test? Like maybe we can create some bogus unwind information and see if LLDB falls over when consuming it? Repository: rG

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord added inline comments. Comment at: lldb/include/lldb/API/SBAddress.h:14-16 +#ifdef SWIG +%include "interface/SBAddressDocstrings.i" +#endif clayborg wrote: > bulbazord wrote: > > clayborg wrote: > > > Do we want two different .i files? Right now we have

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Makes sense to me. It does seem a little weird that you can specify a name or a pid for attaching to a scripted process but I suppose that's just a part of the interface. Repository:

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. I like this. I have no opinion on having `-i/-b` versus `--mode/-m` to indicate which mode crashlog.py should use, though I think that using an enum makes it easier to add new modes down

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord added inline comments. Comment at: lldb/cmake/modules/LLDBFramework.cmake:91 DEPENDS ${header} OUTPUT ${staged_header} -COMMAND ${CMAKE_COMMAND} -E copy ${header} ${staged_header} -COMMENT "LLDB.framework: collect framework header") +COMMAND unifdef -US

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, clayborg, jingham, labath, mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. This is a proof-of-concept change that shows what it would take to use the API headers a

[Lldb-commits] [PATCH] D142059: [lldb/Plugins] Add ScriptedProcess::GetCapabilities affordance (NFC)

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142059/new/ https://reviews.llvm.org/D142059 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord requested changes to this revision. bulbazord added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:421 Target &target = GetTarget(); + bool force_lookup = m_scripted_metadata.GetClassName().contains("CrashLog"); -

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord added a comment. Seems okay to me. @JDevlieghere How does this look to you? Comment at: lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37 +for section in image.section_infos: +if section.start

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord added a comment. Additionally, it looks like you're doing maybe 2-3 things in this patch (Progress reporting, adding a guard rail, changing the format of `loaded_images` in the scripted process example). You could probably break this up into multiple patches. Repository: rG LLVM G

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord added a comment. I'm somewhat unfamiliar with this code so I'll ask some clarifying questions. I think I understand the idea in general though. Comment at: lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37 +for section

[Lldb-commits] [PATCH] D140253: [debugserver] Clear memory allocations after exec

This revision was automatically updated to reflect the committed changes. Closed by commit rG58def623ca46: [debugserver] Clear memory allocations after exec (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140253/new/ https://r

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

This revision was automatically updated to reflect the committed changes. Closed by commit rGf2f3b1a87ad2: [lldb] Do not deallocate memory after exec (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140249/new/ https://reviews.

[Lldb-commits] [PATCH] D141021: [lldb] Remove tools copied into LLDB.framework before install

bulbazord added a comment. In D141021#4043411 , @labath wrote: > Does this mean that the in-tree lldb will cease to be functional once someone > "installs" it? (at least until the next rebuild) I believe so, but only in the case where you are building t

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

bulbazord updated this revision to Diff 488035. bulbazord added a comment. Address @jingham's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140249/new/ https://reviews.llvm.org/D140249 Files: lldb/include/lldb/Target/Memory.h lldb/sou

[Lldb-commits] [PATCH] D141021: [lldb] Remove tools copied into LLDB.framework before install

bulbazord added inline comments. Comment at: lldb/source/API/CMakeLists.txt:215-216 + + add_dependencies(install-liblldb +lldb-framework-cleanup) endif() I think you need to add it as a dependency of `install-liblldb-stripped` as well? CHANGES SINCE LAST

[Lldb-commits] [PATCH] D141021: [lldb] Add lldb-framework-cleanup target

bulbazord added a comment. I think this idea will work but I have a few comments and questions: Based on my understanding of this change, we're supposed to manually run the build system with the target `lldb-framework-cleanup` before we perform an install. Is this the case? This seems most usef

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

bulbazord updated this revision to Diff 483692. bulbazord added a comment. Add a comment explaining why we do not dealloc after exec Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140249/new/ https://reviews.llvm.org/D140249 Files: lldb/include/l

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

bulbazord added inline comments. Comment at: lldb/source/Target/Memory.cpp:337 std::lock_guard guard(m_mutex); - if (m_process.IsAlive()) { + if (m_process.IsAlive() && !did_exec) { PermissionsToBlockMap::iterator pos, end = m_memory_map.end(); jasonmol

[Lldb-commits] [PATCH] D140253: [debugserver] Clear memory allocations after exec

bulbazord created this revision. bulbazord added a reviewer: jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. After an exec, the inferior is a new process and none of these memory regions are still allocated. Clear them out. Re

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

bulbazord created this revision. bulbazord added reviewers: jasonmolenda, jingham, JDevlieghere. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. After an exec has occured, resources used to manage the state of a Process are cleaned up. One su

[Lldb-commits] [PATCH] D140113: [lldb] Force override when adding crashlog command

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Do we want to do this for all the modules in `examples/python` that do something to this effect? e.g. `memory.py` as well. It also might be worth changing some of those messages from err

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

bulbazord added inline comments. Comment at: lldb/examples/python/scripted_process/scripted_platform.py:33 + +processes = { +420: { JDevlieghere wrote: > Why is this method implemented and not a `pass` like the others? This method is not imple

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM, small nit Comment at: lldb/test/API/functionalities/scripted_process/main.cpp:21 + // returns + lock.lock(); +} nit: no need t

[Lldb-commits] [PATCH] D139251: [lldb/Interpreter] Introduce ScriptedPlatform{, Python}Interface

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.cpp:97 + +#endif nit: This `#endif` should have a ` // L

[Lldb-commits] [PATCH] D139249: [lldb] Add Debugger & ScriptedMetadata reference to Platform::CreateInstance

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139249/new/ https://reviews.llvm.org/D139249 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

bulbazord added inline comments. Comment at: lldb/test/API/functionalities/scripted_process/main.cpp:18 + std::unique_lock lock(mutex); + cv.notify_one(); + n = foo(n); Why do you need this initial notification? Comment at: lldb/test/API/fun

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

bulbazord requested changes to this revision. bulbazord added a comment. This revision now requires changes to proceed. I think that the idea you've laid out in the summary makes sense but the implementation seems off. My understanding of `std::condition_variable` is that combined with `std::mut

[Lldb-commits] [PATCH] D139252: [lldb/Plugins] Introduce Scripted Platform Plugin

bulbazord added a comment. There's some overlap in implementation between `ScriptedPlatform::GetProcessInfo` and `ScriptedPlatform::FindProcesses`. If you don't anticipate these diverging dramatically, it might make sense to extract out common functionality into something like `ScriptedPlatfor

[Lldb-commits] [PATCH] D139251: [lldb/Interpreter] Introduce ScriptedPlatform{, Python}Interface

bulbazord added inline comments. Comment at: lldb/include/lldb/Interpreter/ScriptedPlatformInterface.h:14 +#include "lldb/Interpreter/ScriptedInterface.h" +#include "lldb/Target/MemoryRegionInfo.h" + What is `MemoryRegionInfo.h` for? Comment at

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

bulbazord added inline comments. Comment at: lldb/bindings/python/python-wrapper.swig:353-354 + } else { +error_string.assign("wrong number of arguments in __init__, should be 2 " +"(not including self)"); + } I think the error strin

[Lldb-commits] [PATCH] D139249: [lldb] Add Debugger & ScriptedMetadata reference to Platform::CreateInstance

bulbazord added a comment. The idea of the patch seems fine to me. One thing that I thought about is creating a `PlatformSpec` struct that can contain all the needed configuration for `CreateInstance`/`GetOrCreate`. What do you think of this? Comment at: lldb/source/API/SBDeb

[Lldb-commits] [PATCH] D139247: [lldb/Interpreter] Make ScriptedProcessInfo more generic

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

[Lldb-commits] [PATCH] D139247: [lldb/Interpreter] Make ScriptedProcessInfo more generic

bulbazord added inline comments. Comment at: lldb/include/lldb/Interpreter/ScriptedMetadata.h:9-10 + +#ifndef LLDB_SOURCE_PLUGINS_SCRIPTED_METADATA_H +#define LLDB_SOURCE_PLUGINS_SCRIPTED_METADATA_H + This should be `LLDB_INTERPRETER_SCRIPTEDMETADATA_H` to be con

[Lldb-commits] [PATCH] D138237: [lldb] Restore default setting of LLDB_INCLUDE_TESTS in standalone builds

This revision was automatically updated to reflect the committed changes. Closed by commit rG58d38b3ff8c3: [lldb] Restore default setting of LLDB_INCLUDE_TESTS in standalone builds (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D138237: [lldb] Restore default setting of LLDB_INCLUDE_TESTS in standalone builds

bulbazord created this revision. bulbazord added reviewers: labath, mgorny. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In 52f39853abd46495a6d636c4b035e1b92cf4b833

[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

This revision was automatically updated to reflect the committed changes. Closed by commit rG13cd39017de0: [lldb] Add information on type systems to statistics dump command (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137191

[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

bulbazord updated this revision to Diff 472672. bulbazord added a comment. Remove `const &` from argument to `Module::ForEachTypeSystem` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137191/new/ https://reviews.llvm.org/D137191 Files: lldb/include/lldb/Core/Module.h lldb/include/ll

[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

bulbazord updated this revision to Diff 472468. bulbazord added a comment. Address feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137191/new/ https://reviews.llvm.org/D137191 Files: lldb/include/lldb/Core/Module.h lldb/include/lldb/Symbol/TypeSystem.h lldb/include/lldb/Ta

[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

bulbazord added a comment. I have a pull request to apple's fork to show how this is used for swift: https://github.com/apple/llvm-project/pull/5540 I primarily wanted this portion to be upstream so that the patch is easier to maintain downstream, but it could be useful for clang-produced TypeS

[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

bulbazord created this revision. bulbazord added reviewers: clayborg, JDevlieghere, aprantl. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Context: I plan on using this change primarily downstream in

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

This revision was automatically updated to reflect the committed changes. Closed by commit rG0cfa50154a47: [LLDB] Only run lldb-server Shell tests if it gets built (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135825/new/ ht

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

bulbazord updated this revision to Diff 467587. bulbazord added a comment. Implement David's suggestion CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135825/new/ https://reviews.llvm.org/D135825 Files: lldb/test/CMakeLists.txt lldb/test/Shell/lit.cfg.py lldb/test/Shell/lit.site.c

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

bulbazord added a comment. In D135825#3855539 , @DavidSpickett wrote: > Would a `lit.local.cfg` work in this folder (`lldb/test/Shell/lldb-server/` > that is)? That would automatically apply it to all existing and future tests. > > `llvm/test/MC/AArch64

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

bulbazord created this revision. bulbazord added reviewers: labath, JDevlieghere. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. It's easy enough to disable the lldb-server build. The lldb-server unit

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

This revision was automatically updated to reflect the committed changes. Closed by commit rGbefa77e59a77: [lldb] Filter DIEs based on qualified name where possible (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129682/new/ h

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord updated this revision to Diff 449350. bulbazord edited the summary of this revision. bulbazord added a comment. Simplify `Module::LookupInfo::NameMatchesLookupInfo`. Perform an initial check to see if the name matches, and if not, do the more expensive demangling operation. CHANGES S

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord marked an inline comment as done. bulbazord added inline comments. Comment at: lldb/source/Core/Module.cpp:740 + bool user_provided_name_is_mangled = + Mangled::GetManglingScheme(m_name.GetStringRef()) != + Mangled::eManglingSchemeNone; labat

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord marked 3 inline comments as done. bulbazord added inline comments. Comment at: lldb/source/Core/Module.cpp:730 +bool Module::LookupInfo::NameMatchesLookupInfo( +ConstString function_name, LanguageType language_type) const { clayborg wrote: > So th

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord updated this revision to Diff 448472. bulbazord added a comment. Herald added a reviewer: shafik. - Expanded on NameMatchesLookupInfo to handle different scenarios when names we are comparing are mangled/demangled. - Extracted functionality from DWARFASTParserClang to construct a demang

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord marked an inline comment as done. bulbazord added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:37 +Mangled mangled_name(die.GetMangledName()); +if (!lookup_info.NameMatchesLookupInfo(mangled_name.GetDemangledName(), +

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord added a comment. Friendly ping :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129682/new/ https://reviews.llvm.org/D129682 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/l

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord updated this revision to Diff 445629. bulbazord edited the summary of this revision. bulbazord added a comment. I have factored out part of `Module::LookupInfo::Prune` into its own method `Module::LookupInfo::NameMatchesLookupInfo` which I re-use in `ProcessFunctionDIE` instead of copy

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord added a comment. In D129682#3651175 , @labath wrote: > Well.. first of let me say that the performance gain is impressive. I > wouldn't have expected to gain that much with a relatively simple change. Thank you! I am also very surprised, it tu

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

bulbazord created this revision. bulbazord added reviewers: clayborg, jingham, JDevlieghere, labath. Herald added a subscriber: arphaman. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Context: When setting a breakpoint by name, we invoke `M

[Lldb-commits] [PATCH] D128366: [lldb] Make Module::LookupInfo::Prune language-agnostic

bulbazord added a comment. In D128366#3608051 , @labath wrote: > In D128366#3605484 , @bulbazord > wrote: > >> In D128366#3603943 , @labath wrote: >> >>> Could we move th

[Lldb-commits] [PATCH] D128366: [lldb] Make Module::LookupInfo::Prune language-agnostic

bulbazord added a comment. In D128366#3603943 , @labath wrote: > How hot is this code? Parsing a demangled name isn't entirely cheap, and the > fact that the objc++ class calls back into the c++ version means that the c++ > name will be parsed twice. N

[Lldb-commits] [PATCH] D128366: [lldb] Make Module::LookupInfo::Prune language-agnostic

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, clayborg, jingham, labath. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Module::LookupInfo::Prune tries to prune results from lookups that don't match the user-provi

[Lldb-commits] [PATCH] D114668: [lldb] Move generic DWARFASTParser code out of Clang-specific code

bulbazord added a comment. I think breaking it out of the Clang-specific class makes sense if we want LLDB to be more language-agnostic. Do you have an idea of what bits of `DWARFASTParserClang` can be moved out other than `ParseChildArrayInfo` and `GetAccessTypeFromDWARF` (from the patch on to

[Lldb-commits] [PATCH] D114719: [lldb] Move non-clang specific method to the generic DWARF Parser

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. This makes sense to me. Out of curiosity, do you have plans to use this for non-clang-based languages? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114719/new/ https://reviews

[Lldb-commits] [PATCH] D113789: Add the ability to cache information for a module between debugger instances.

bulbazord added a comment. This patch effectively introduces a file format to cache lldb internal state. While the tests and the code do give some information for what it looks like, some documentation about this format would be nice. I like this idea a lot! My comments were kind of surface lev

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

bulbazord closed this revision. bulbazord added a comment. 385b2189cc44 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110115/new/ https://reviews.llvm.org/D110115 _

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

bulbazord added a comment. friendly ping! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110115/new/ https://reviews.llvm.org/D110115 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://l

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

bulbazord updated this revision to Diff 374639. bulbazord added a comment. Rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110115/new/ https://reviews.llvm.org/D110115 Files: lldb/include/lldb/Target/Language.h lldb/source/Expression/CM

[Lldb-commits] [PATCH] D109928: [lldb] Remove IRExecutionUnit::CollectFallbackNames

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG43552651319e: [lldb] Remove IRExecutionUnit::CollectFallbackNames (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D109928: [lldb] Remove IRExecutionUnit::CollectFallbackNames

bulbazord added a comment. In D109928#3013627 , @shafik wrote: > I think this looks fine, just verify that this in a C++ case an expression > with an `extern "C"` function call works fine e.g. > > extern "C" { > int g() { return 10;} > } > >

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

bulbazord added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:524-530 + if (param_and_qual_matches.size()) +return param_and_qual_matches[0]; // It is assumed that there will be only + // one! +

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

bulbazord created this revision. bulbazord added reviewers: clayborg, jingham, teemperor, JDevlieghere. Herald added a subscriber: mgorny. bulbazord requested review of this revision. Herald added a project: LLDB. This change accomplishes the following: - Moves `IRExecutionUnit::FindBestAlternate

[Lldb-commits] [PATCH] D109928: [lldb] Remove IRExecutionUnit::CollectFallbackNames

bulbazord added a comment. In D109928#3005085 , @clayborg wrote: > Looks fine to me. I am assuming the full test suite passes with this change > in place? It didn't introduce any new test failures on any of my machines. Comment at: l

[Lldb-commits] [PATCH] D109928: [lldb] Remove IRExecutionUnit::CollectFallbackNames

bulbazord created this revision. bulbazord added a reviewer: clayborg. bulbazord requested review of this revision. Herald added a project: LLDB. The work that IRExecutionUnit::CollectFallbackNames is basically the work that `CPlusPlusLanguage::GetDemangledFunctionNameWithoutArguments` does alread

[Lldb-commits] [PATCH] D109785: [lldb] Refactor and rename CPlusPlusLanguage::FindAlternateFunctionManglings

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa65f6aafe2d3: [lldb] Refactor and rename CPlusPlusLanguage::FindAlternateFunctionManglings (authored by bulbazord). Changed prior to commit: https

[Lldb-commits] [PATCH] D109785: [lldb] Refactor and rename CPlusPlusLanguage::FindAlternateFunctionManglings

bulbazord added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h:130 - // Given a mangled function name, calculates some alternative manglings since - // the compiler mangling may not line up with the symbol we are expecting - static ui

[Lldb-commits] [PATCH] D109785: [lldb] Refactor and rename CPlusPlusLanguage::FindAlternateFunctionManglings

bulbazord created this revision. bulbazord added reviewers: clayborg, teemperor, jingham, JDevlieghere, labath. bulbazord requested review of this revision. Herald added a project: LLDB. I have 2 goals with this change: 1. Disambiguate between CPlusPlus::FindAlternateFunctionManglings and IRExec

[Lldb-commits] [PATCH] D109779: [LLDB] [Minidump] Fix format string warnings on Windows

bulbazord added a reviewer: MaskRay. bulbazord added a comment. Adding MaskRay who fixed something similar to this yesterday. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109779/new/ https://reviews.llvm.org/D109779 __

[Lldb-commits] [PATCH] D109600: [lldb] Remove PluginInterface::GetPluginVersion

bulbazord accepted this revision. bulbazord added a comment. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109600/new/ https://reviews.llvm.org/D109600 ___ lldb-commits mailing list lldb-commi

[Lldb-commits] [PATCH] D109384: [lldb] Delete IRExecutionUnit::SearchSpec

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG303b27f21b98: [lldb] Delete IRExecutionUnit::SearchSpec (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D109384: [lldb] Delete IRExecutionUnit::SearchSpec

bulbazord created this revision. bulbazord added reviewers: clayborg, jingham, teemperor, JDevlieghere. bulbazord requested review of this revision. Herald added a project: LLDB. IRExecutionUnit::SearchSpec is a struct that encapsulates information needed to look for a symbol. Specifically, it is

[Lldb-commits] [PATCH] D109013: [lldb] Tighten lock in Language::ForEach

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG862a311301f5: [lldb] Tighten lock in Language::ForEach (authored by bulbazord). Changed prior to commit: https://reviews.llvm.org/D109013?vs=36975

[Lldb-commits] [PATCH] D109013: [lldb] Tighten lock in Language::ForEach

bulbazord added a comment. For further context, I encountered this when debugging the test failures from D108229 . `BreakpointResolverName::AddNameLookup` constructs a `Module::LookupInfo` in a callback that it passes to `Language::ForEach`. Repository: rG

[Lldb-commits] [PATCH] D109013: [lldb] Tighten lock in Language::ForEach

bulbazord created this revision. bulbazord added reviewers: jingham, teemperor, JDevlieghere, clayborg, aprantl. bulbazord requested review of this revision. Herald added a project: LLDB. It is easy to accidentally introduce a deadlock by having the callback passed to Language::ForEach also attemp

[Lldb-commits] [PATCH] D108395: [lldb] Delete IRExecutionUnit::CollectCandidateCPlusPlusNames

bulbazord added a comment. Embarassingly I found out my test environment was not working correctly, this does indeed fail some tests. I will upload a patch to actually refactor it :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108395/new/ https:

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

bulbazord added a comment. In D108229#2966405 , @aprantl wrote: > FYI (you may have have already noticed) this causes some merge conflicts in > swift-lldb. It would be great if we could work together to figure out how to > best resolve them. Yes, I fig

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcd2134e42aa7: [lldb] Refactor Module::LookupInfo constructor (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

bulbazord added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:73 + llvm::StringRef basename = method.GetBasename(); + if (basename.empty()) { +if (CPlusPlusLanguage::ExtractContextAndIdentifier( jingham wrote: > T

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

bulbazord added a comment. ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108229/new/ https://reviews.llvm.org/D108229 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.or

[Lldb-commits] [PATCH] D108395: [lldb] Delete IRExecutionUnit::CollectCandidateCPlusPlusNames

bulbazord added a comment. In D108395#2957194 , @teemperor wrote: > I'm very supportive of the idea that we delete everything that isn't tested > :) (and I'm only partly joking here). I can sympathize, I'm only partly serious with this change! :D > How

[Lldb-commits] [PATCH] D108395: [lldb] Delete IRExecutionUnit::CollectCandidateCPlusPlusNames

bulbazord created this revision. bulbazord added reviewers: clayborg, jingham, teemperor, JDevlieghere. Herald added a subscriber: mgorny. bulbazord requested review of this revision. Herald added a project: LLDB. My goal is to remove lldbExpression's dependence on lldbPluginCPlusPlusLanguage. I

<    3   4   5   6   7   8   9   10   11   12   >