[Lldb-commits] [PATCH] D149663: [lldb] Remove FileSpec::GetLastPathComponent

bulbazord added a comment. In D149663#4313112 , @mib wrote: > @bulbazord What if the FileSpec is pointing to a directory instead of a file > ? What would `GetFilename` return in that case compared to > `GetLastPathComponent` ? FileSpec chops

[Lldb-commits] [PATCH] D149671: [lldb] Minor cleanups at callsites of FileSpec::GetFileNameExtension

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. FileSpec::GetFileNameExtension returns a

[Lldb-commits] [PATCH] D149625: [lldb] Refactor SBFileSpec::GetDirectory

bulbazord added a comment. In D149625#4312838 , @jingham wrote: > The old code had the side-effect of NOT resolving the path of the SBFileSpec > in order to get its directory. I am not sure whether that was on purpose or > not, however. To be more

[Lldb-commits] [PATCH] D149663: [lldb] Remove FileSpec::GetLastPathComponent

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a subscriber: emaste. Herald added a project: All. bulbazord requested review of this revision. Herald added subscribers: lldb-commits, MaskRay. Herald added a project: LLDB. As far

[Lldb-commits] [PATCH] D149625: [lldb] Refactor SBFileSpec::GetDirectory

This revision was automatically updated to reflect the committed changes. Closed by commit rG2bea2d7b070d: [lldb] Refactor SBFileSpec::GetDirectory (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149625/new/

[Lldb-commits] [PATCH] D149625: [lldb] Refactor SBFileSpec::GetDirectory

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. There's no reason to create an entire new filespec to mutate and

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

This revision was automatically updated to reflect the committed changes. Closed by commit rGe53e1de57ecc: [lldb] Change ObjectValueDictionary to use a StringMap (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149482/new/

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

bulbazord updated this revision to Diff 518569. bulbazord added a comment. Remove unneeded StringRef Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149482/new/ https://reviews.llvm.org/D149482 Files:

[Lldb-commits] [PATCH] D149565: [lldb] Add debugger.external-editor setting

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/D149565/new/ https://reviews.llvm.org/D149565 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Thanks for adding the test. The test itself doesn't seem portable and likely will fail on several platforms though I'm not sure which off the top of my head. After landing this, please

[Lldb-commits] [PATCH] D149565: [lldb] Add debugger.external-editor setting

bulbazord requested changes to this revision. bulbazord added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:395-396 - static std::optional g_app_fsref; - static std::string g_app_error; - static

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

bulbazord updated this revision to Diff 518542. bulbazord added a comment. Sort output for dumping to be deterministic (based on alphabetical order like previous implementation) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149482/new/

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord added a comment. In D149262#4306884 , @kuilpd wrote: > In D149262#4306820 , @bulbazord > wrote: > >> Sorry I should have brought this up earlier but I've noticed you don't have >> any tests with this

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

bulbazord added inline comments. Comment at: lldb/source/Core/Disassembler.cpp:846 +// Note: The reason we are making the data_type a uint64_t when value is +// uint32_t is that there is no eTypeUInt32 enum value. +if (llvm::StringRef(value) ==

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord added a comment. Sorry I should have brought this up earlier but I've noticed you don't have any tests with this change. Is it possible you could add something there? Something to make sure that these settings work as expected. Sorry for the churn btw, I really appreciate your

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord added inline comments. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:340-348 +Process *process_sp; +ABISP abi_sp; +if ((process_sp = exe_ctx.GetProcessPtr()) && +(abi_sp = process_sp->GetABI())) { +

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. llvm has a structure for maps where the key's type

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord requested changes to this revision. bulbazord added a comment. This revision now requires changes to proceed. Mostly looks good to me, just a small thing about an implementation detail. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:38 +using namespace

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

bulbazord accepted this revision. bulbazord added a comment. LGTM! Thanks for cleaning this up. Comment at: lldb/source/Target/Thread.cpp:1762-1763 + frame_sc.line_entry.file, frame_sc.line_entry.line)) { +LLDB_LOG_ERROR(GetLog(LLDBLog::Host),

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

bulbazord added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337 + + LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no); + JDevlieghere wrote: > bulbazord wrote: > > nit: Move log line below checking if the file_path

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

bulbazord added a comment. LGTM overall. A few nits but overall an excellent cleanup! :) Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337 + + LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no); + nit: Move log line below checking if

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

bulbazord updated this revision to Diff 517725. bulbazord added a comment. Address @mib's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149096/new/ https://reviews.llvm.org/D149096 Files: lldb/source/Symbol/LocateSymbolFile.cpp Index:

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

bulbazord added inline comments. Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:89-91 + dsym_file.append(".dSYM"); + llvm::sys::path::append(dsym_file, path_style, "Contents", "Resources", + "DWARF", filename); mib wrote: > Why do

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord added inline comments. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:339 + if (stack_frame_size == 0) +stack_frame_size = arch == llvm::Triple::msp430 ? 512 : 512 * 1024; kuilpd wrote: > bulbazord wrote: > > jingham wrote: >

[Lldb-commits] [PATCH] D149300: [lldb] Change return type of FileSpec::GetFileNameExtension

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6fcdfc378c2f: [lldb] Change return type of FileSpec::GetFileNameExtension (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D149214: [lldb] Speed up DebugAbbrev parsing

bulbazord added a comment. In D149214#4300491 , @aprantl wrote: > Did you also measure the extra memory consumption? I would be surprised if > this mattered, but we do parse a lot of DWARF DIEs... > > Generally this seems fine. I compared the memory

[Lldb-commits] [PATCH] D149300: [lldb] Change return type of FileSpec::GetFileNameExtension

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a subscriber: emaste. Herald added a project: All. bulbazord requested review of this revision. Herald added subscribers: lldb-commits, MaskRay. Herald added a project: LLDB. These

[Lldb-commits] [PATCH] D149284: [lldb] Remove finding .Bundle directories in PlatformDarwinKernel

This revision was automatically updated to reflect the committed changes. Closed by commit rG33d6bd1c6674: [lldb] Remove finding .Bundle directories in PlatformDarwinKernel (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D149284: [lldb] Fix finding .Bundle directories in PlatformDarwinKernel

bulbazord updated this revision to Diff 517309. bulbazord added a comment. Remove code instead of attempting to fix things Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149284/new/ https://reviews.llvm.org/D149284 Files:

[Lldb-commits] [PATCH] D149284: [lldb] Fix finding .Bundle directories in PlatformDarwinKernel

bulbazord added a comment. In D149284#4299933 , @jasonmolenda wrote: > I added this in 2016 via 243bd763ecad466fac748b565ea52c661107684d and I have > no idea what it is for. I know of no such prefix related to kernel > debugging. Let's remove it

[Lldb-commits] [PATCH] D149284: [lldb] Fix finding .Bundle directories in PlatformDarwinKernel

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. Herald added a subscriber: lldb-commits. FileSpec::GetFileNameExtension returns a string where the first character is

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

bulbazord added a comment. In D149111#4299871 , @bulbazord wrote: > Looks like this test does not pass on windows: > https://lab.llvm.org/buildbot/#/builders/219/builds/2389 > > I'm aware and working on this. I marked the test as unsupported on windows

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

bulbazord added a comment. Looks like this test does not pass on windows: https://lab.llvm.org/buildbot/#/builders/219/builds/2389 I'm aware and working on this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149111/new/

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord requested changes to this revision. bulbazord added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:339 + if (stack_frame_size == 0) +stack_frame_size = arch == llvm::Triple::msp430

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

This revision was automatically updated to reflect the committed changes. Closed by commit rGc997acb97a9b: [lldb] Add support for specifying language when setting watchpoint by expression (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D149218: [lldb] Fix another GCC build failure in ScriptedPythonInterface.h

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

[Lldb-commits] [PATCH] D149214: [lldb] Speed up DebugAbbrev parsing

bulbazord created this revision. bulbazord added reviewers: aprantl, JDevlieghere. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. While measuring some performance in LLDB I noticed that we were

[Lldb-commits] [PATCH] D149179: [lldb/test] Consolidate interactive scripted process debugging test

bulbazord added a comment. Seems ok to me. You might want to update the dependencies of this diff so that the pre-merge checks don't fail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149179/new/ https://reviews.llvm.org/D149179

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

bulbazord requested review of this revision. bulbazord added a comment. In D149111#4296670 , @mib wrote: > I think we should still be able to use this new flag with the previous ones > (i.e. `watchpoint set -l c++ -w read -- _var`) You can do that

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

bulbazord added inline comments. Comment at: lldb/source/Interpreter/OptionGroupWatchpoint.cpp:67 + "Number of bytes to use to watch a region."}, +{LLDB_OPT_SET_2, + false, jasonmolenda wrote: > I don't care much, but the formatting of this entry is

[Lldb-commits] [PATCH] D149175: [lldb/test] Update lldbutil.fetch_next_event to match broadcaster class

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/D149175/new/ https://reviews.llvm.org/D149175 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D149175: [lldb/test] Update lldbutil.fetch_next_event to match broadcaster class

bulbazord added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:1206-1207 +def fetch_next_event(test, listener, broadcaster, match_class=False, timeout=10): """Fetch one event from the listener and return it if it matches the provided

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda, kastiglione, Michael137. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This is useful in

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

bulbazord added a comment. Perhaps the right thing to do would be to try to do the larger FileSpec changes instead? The changes in `Symbols::LocateExecutableSymbolFile` can probably go in regardless though. We call `Resolve` on all the file specs in `debug_file_search_paths` on line 307, so no

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The goal of this patch is to speed up

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. I don't have an issue here then. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148395/new/ https://reviews.llvm.org/D148395

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord added inline comments. Comment at: lldb/include/lldb/Target/Process.h:122 m_async(false) { ProcessInfo::operator=(launch_info); SetProcessPluginName(launch_info.GetProcessPluginName()); bulbazord wrote: > mib wrote: > > Because we

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord added inline comments. Comment at: lldb/include/lldb/Target/Process.h:122 m_async(false) { ProcessInfo::operator=(launch_info); SetProcessPluginName(launch_info.GetProcessPluginName()); mib wrote: > Because we moved `m_listener_sp`

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord added inline comments. Comment at: lldb/source/Target/PathMappingList.cpp:72 void PathMappingList::Append(const PathMappingList , bool notify) { + std::scoped_lock locks(m_mutex, rhs.m_mutex); ++m_mod_id; nickdesaulniers wrote: > ``` >

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

bulbazord added a comment. Looks good to me. Might want to let @jingham or @JDevlieghere give it a look-over though. Comment at: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py:9 + +import os,json,struct,signal +import time

[Lldb-commits] [PATCH] D148548: [lldb] Improve breakpoint management for interactive scripted process

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. one nit but otherwise LGTM. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:124 + if

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord added a comment. In D148395#4285017 , @mib wrote: > In D148395#4270508 , @bulbazord > wrote: > >> Creating a ProcessAttachInfo from a ProcessLaunchInfo with this change means >> they'll have different

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (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/D148399/new/ https://reviews.llvm.org/D148399 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

bulbazord added a comment. We'd go from: LLDB_LOGF(log, "Process::SetPrivateState (plugin = %s, state = %s) state didn't " "change. Ignoring...", GetPluginName().data(), StateAsCString(new_state)); to LLDB_LOG(log, "(plugin = %s, state = %s) state didn't

[Lldb-commits] [PATCH] D148679: [lldb] Change setting descriptions to use StringRef instead of ConstString

This revision was automatically updated to reflect the committed changes. Closed by commit rG96a800c07f02: [lldb] Change setting descriptions to use StringRef instead of ConstString (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D148676: [lldb][NFCI] Stop creating additional temporary string in Log::VAPrintf

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG77e3914be7c9: [lldb][NFCI] Stop creating additional temporary string in Log::VAPrintf (authored by bulbazord). Repository: rG LLVM Github

[Lldb-commits] [PATCH] D148679: [lldb] Change setting descriptions to use StringRef instead of ConstString

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. These probably do not need to be in the ConstString StringPool

[Lldb-commits] [PATCH] D148400: [lldb] Fix bug to update process public run lock with process state

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/D148400/new/ https://reviews.llvm.org/D148400 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D148676: [lldb][NFCI] Stop creating additional temporary string in Log::VAPrintf

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Instead of creating a std::string from the

[Lldb-commits] [PATCH] D148579: [lldb] Change parameter type of StructuredData::ParseJSON

This revision was automatically updated to reflect the committed changes. Closed by commit rG8bddb13c2470: [lldb] Change parameter type of StructuredData::ParseJSON (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148579/new/

[Lldb-commits] [PATCH] D148579: [lldb] Change parameter type of StructuredData::ParseJSON

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Instead of taking a `const std::string &` we can take an

[Lldb-commits] [PATCH] D148402: [lldb] Remove use of ConstString from Args::GetShellSafeArgument

This revision was automatically updated to reflect the committed changes. Closed by commit rGc8bb7c234c68: [lldb] Remove use of ConstString from Args::GetShellSafeArgument (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

This revision was automatically updated to reflect the committed changes. Closed by commit rG43ac269bdd00: [lldb] Lock accesses to PathMappingListss internals (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148380/new/

[Lldb-commits] [PATCH] D148548: [lldb] Improve breakpoint management for interactive scripted process

bulbazord added a comment. Seems okay to me, but it's a little messy that we're having to manage breakpoints like this. Comment at: lldb/bindings/interface/SBTargetExtensions.i:144-171 +class watchpoints_access(object): +'''A helper object that will lazily

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

bulbazord added a comment. A few high-level comments: - You're doing a lot of type-annotations in python which is good, but you're not being very consistent about it. It would be tremendously helpful if you could add type annotations everywhere. - I would recommend using f-strings over `%` and

[Lldb-commits] [PATCH] D148401: [lldb/API] Add convenience constructor for SBError (NFC)

bulbazord added inline comments. Comment at: lldb/include/lldb/API/SBError.h:26 + SBError(const char *message); + JDevlieghere wrote: > Why not a StringRef? I'm not sure we have `StringRef` type map in swig? We would have to create one for swig to pick up on

[Lldb-commits] [PATCH] D148401: [lldb/API] Add convenience constructor for SBError (NFC)

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. I like this! Reading through the call stack, it looks like we shouldn't hit any issues if `message` is `nullptr`. Could you add a small test to `TestSBError.py` constructing an error

[Lldb-commits] [PATCH] D145296: [lldb/Plugin] Add breakpoint setting support to ScriptedProcesses.

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Seems ok to me, one small thought. Shouldn't hold this patch up though. Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:270-271 +Status

[Lldb-commits] [PATCH] D148400: [lldb] Fix bug to update process public run lock with process state

bulbazord requested changes to this revision. bulbazord added a comment. This revision now requires changes to proceed. Idea is good, few concerns about the implementation. Comment at: lldb/source/Target/Process.cpp:404-419 +llvm::StringRef

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

bulbazord added a comment. +1 to what Jonas said. `LLDB_LOG` would greatly simplify this since it puts `__FILE__` and `__func__` in each message, which is what these are doing manually. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148399/new/

[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in passthrough mode to event listeners

bulbazord added inline comments. Comment at: lldb/include/lldb/Utility/Broadcaster.h:520-522 +/// A optional listener that all private events get also broadcasted to, +/// on top the hijacked / default listeners. +lldb::ListenerSP m_passthrough_listener = nullptr;

[Lldb-commits] [PATCH] D148541: [lldb] fix build issue on MSVC because of missing byte-swap builtins

bulbazord accepted this revision. bulbazord 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/D148541/new/ https://reviews.llvm.org/D148541

[Lldb-commits] [PATCH] D148541: [lldb] fix build issue on MSVC because of missing byte-swap builtins

bulbazord requested changes to this revision. bulbazord added a comment. This revision now requires changes to proceed. Instead of not using `__builtin_bswap{32,64}` entirely, we should check if they're available. You can use the clang feature-checking macro `__has_builtin` (and if it doesn't

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

bulbazord accepted this revision. bulbazord added a comment. In D146965#4274274 , @DavidSpickett wrote: > I'm sure there's something missing but LGTM. > > @bulbazord ? I'm satisfied with this since there are tests. I'm not sure what might be missing

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord added a comment. Creating a ProcessAttachInfo from a ProcessLaunchInfo with this change means they'll have different listeners. Is that ProcessLaunchInfo kept around for any other reason? Or is it just made to create the ProcessAttachInfo? This seems like a reasonable move to me, but

[Lldb-commits] [PATCH] D148402: [lldb] Remove use of ConstString from Args::GetShellSafeArgument

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Having the names of various shells in

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord added a comment. In D148380#4270090 , @JDevlieghere wrote: > In D148380#4270085 , @bulbazord > wrote: > >> In D148380#4269876 , @bulbazord >> wrote: >> >>>

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord added a comment. In D148380#4269876 , @bulbazord wrote: > In D148380#4269862 , @JDevlieghere > wrote: > >> Does this actually have to be a //recursive// mutex? > > Good point, I don't think it does.

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord added a comment. In D148380#4269862 , @JDevlieghere wrote: > Does this actually have to be a //recursive// mutex? Good point, I don't think it does. I'll update this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This class is not safe in multithreaded code. It's

[Lldb-commits] [PATCH] D148282: Fix the help for "type X delete" to make the -a & -w behaviors clear

bulbazord added inline comments. Comment at: lldb/source/Commands/CommandObjectType.cpp:112-113 +return "format"; + } +}; + maybe add an `llvm_unreachable` after the switch statement? Comment at:

[Lldb-commits] [PATCH] D148050: [lldb] Change formatter helper function parameter list to remove ConstString

This revision was automatically updated to reflect the committed changes. Closed by commit rG0e22b8cba786: [lldb] Change formatter helper function parameter list to remove ConstString (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D148050: [lldb] Change formatter helper function parameter list to remove ConstString

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, jingham, mib, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. All of these functions take a ConstString for the

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

This revision was automatically updated to reflect the committed changes. Closed by commit rG6ebf1bc66b89: [lldb] Change return type of EventData::GetFlavor (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147833/new/

[Lldb-commits] [PATCH] D147841: [lldb][NFC] Update syntax description for language cplusplus demangle

This revision was automatically updated to reflect the committed changes. Closed by commit rG469bdbd62ce2: [lldb][NFC] Update syntax description for language cplusplus demangle (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

bulbazord updated this revision to Diff 511954. bulbazord added a comment. Change return type to llvm::StringRef Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147833/new/ https://reviews.llvm.org/D147833 Files:

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

bulbazord added a comment. In D147833#4253416 , @jasonmolenda wrote: > idk maybe I'm over-thinking it, but this does make me a little uncomfortable. > We have at least one instance where someone did > `platform_sp->GetPluginName() == "ios-simulator"`

[Lldb-commits] [PATCH] D147841: [lldb][NFC] Update syntax description for language cplusplus demangle

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, Michael137, aprantl. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Also added some tests because this is completely

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

bulbazord added a comment. In D147833#4252651 , @jasonmolenda wrote: > Am I missing something, how does this work when we have uses like > `event_data->GetFlavor() == TargetEventData::GetFlavorString()` - is this > changing from a one-time

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, labath, jingham, mib, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. There's no reason these strings need to be

[Lldb-commits] [PATCH] D147801: [lldb] Add unittests for a few FileSpec methods

This revision was automatically updated to reflect the committed changes. Closed by commit rGde5f96e99aed: [lldb] Add unittests for a few FileSpec methods (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147801/new/

[Lldb-commits] [PATCH] D147801: [lldb] Add unittests for a few FileSpec methods

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, jingham, mib, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This adds tests for: -

[Lldb-commits] [PATCH] D147746: [lldb][NFC] Delete unused function Breakpoint::GetEventIdentifier

This revision was automatically updated to reflect the committed changes. Closed by commit rGb04bc87c9edc: [lldb][NFC] Delete unused function Breakpoint::GetEventIdentifier (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D147746: [lldb][NFC] Delete unused function Breakpoint::GetEventIdentifier

bulbazord created this revision. bulbazord added reviewers: mib, JDevlieghere, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This is unused Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D147736: [lldb] Add an LLDB_DEPRECATED macro similar to LLVM_DEPRECATED

bulbazord accepted this revision. bulbazord added inline comments. This revision is now accepted and ready to land. Comment at: lldb/include/lldb/API/SBDefines.h:33 +// supports the attribute. +#if defined(SWIG) or _cplusplus < 201300 +#undef LLDB_DEPRECATED

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

bulbazord added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873 LLDB_LOG(log, "ObjectFilePECOFF::AppendFromExportTable - failed to get export " "table entry name: {0}",

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

bulbazord added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873 LLDB_LOG(log, "ObjectFilePECOFF::AppendFromExportTable - failed to get export " "table entry name: {0}",

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

bulbazord added a comment. In D145580#4245911 , @DavidSpickett wrote: > @bulbazord Please take a look and see if I've done the plugin/core code split > correctly. This looks fine. Thanks for being flexible and working with us to find something that

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

bulbazord added inline comments. Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:127 llvm::Triple::hexagon, llvm::Triple::mips, llvm::Triple::mips64el, - llvm::Triple::mipsel, llvm::Triple::systemz}, + llvm::Triple::mipsel,

[Lldb-commits] [PATCH] D147587: Fix the check in StopInfoBreakpoint for "are we running an expression"

bulbazord accepted this revision. bulbazord added a comment. LGTM. It'd be nice if we could have a test that wasn't specific to Darwin or ObjC though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147587/new/ https://reviews.llvm.org/D147587

<    1   2   3   4   5   6   7   8   9   10   >