[Lldb-commits] [PATCH] D151425: [lldb][nfc] Place comment in the right place

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM modulo Doxygen comment. Comment at: lldb/include/lldb/Utility/RangeMap.h:47 + // Set the start value for the range, and keep the same size void SetRang

[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Utility/FileSpec.h:420 + /// A std::vector of std::strings for each path component. + std::vector GetComponents() const; + bulbazord wrote: > JDevlieghere wrote: > > I'm surprised this returns

[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Utility/FileSpec.h:420 + /// A std::vector of std::strings for each path component. + std::vector GetComponents() const; + I'm surprised this returns a vector of `std::string`s and not `llvm::

[Lldb-commits] [PATCH] D151392: Fix SBValue::FindValue for file static variables

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/D151392/new/ https://reviews.llvm.org/D151392 ___

[Lldb-commits] [PATCH] D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses

JDevlieghere added a comment. The array approach is cool but makes it hard to be backwards compatible: an old lldb is going to error out when presented with more than one value. If you made this two separate options, a client can use `settings set -e` to set the setting if it exists and still h

[Lldb-commits] [PATCH] D151381: [lldb][NFCI] Include in SBDefines for FILE * definition

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/D151381/new/ https://reviews.llvm.org/D151381 ___

[Lldb-commits] [PATCH] D151269: [lldb] Pass CMAKE_SYSROOT through to lldb tests

JDevlieghere added a comment. The change itself looks fine, but isn't this an issue for the API tests too? If so, how is the sys root passed to `dotest.py` and can the shell tests do the same? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151269/n

[Lldb-commits] [PATCH] D151236: [lldb][NFCI] Remove unused member from ObjectFileMachO

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/D151236/new/ https://reviews.llvm.org/D151236 ___

[Lldb-commits] [PATCH] D151043: [lldb] Add "Trace" stop reason in Scripted Thread

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. Are there other stop reasons that are currently not supported by scripted threads that we should consider supporting? Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[Lldb-commits] [PATCH] D151045: [lldb/crashlog] Remove tempfile prefix from inlined symbol object file

JDevlieghere added inline comments. Comment at: lldb/examples/python/crashlog.py:1165-1168 +if os.path.isdir(obj_dir.name): +for file in os.listdir(obj_dir.name): +os.unlink(os.path.join(obj_dir.name, file)) +os.rmdir(obj_dir.name)

[Lldb-commits] [PATCH] D150805: Proof of concept for reducing progress-reporting frequency.

JDevlieghere added a comment. In D150805#4351277 , @saugustine wrote: > This update switches to a time-based approach as suggested by Jordan. > However, the timing is about the same as the original. I believe because > calling getCurrentTime every iter

[Lldb-commits] [PATCH] D150954: [lldb][cmake] Allow specifying custom libcxx for tests in standalone builds

JDevlieghere added inline comments. Comment at: lldb/test/CMakeLists.txt:142-143 +set(LLDB_TEST_LIBCXX_ROOT_DIR "${LLVM_BINARY_DIR}" CACHE PATH +"The build root for libcxx. Used in standalone builds to point the API tests to a custom build of libcxx.") + I

[Lldb-commits] [PATCH] D150805: Proof of concept for reducing progress-reporting frequency.

JDevlieghere added a comment. Given that here we know the `total_progress` in advance and assuming the operation is relatively evenly distributed, would it make sense to report for every percentage? We could do this in `ManualDWARFIndex::Index` or we could add something like a "Policy" or "Gran

[Lldb-commits] [PATCH] D150630: [lldb][docs] Update SB API design document

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/D150630/new/ https://reviews.llvm.org/D150630 ___

[Lldb-commits] [PATCH] D150639: [lldb] Define lldbassert based on NDEBUG instead of LLDB_CONFIGURATION_DEBUG

This revision was automatically updated to reflect the committed changes. Closed by commit rG10a50762caa6: [lldb] Define lldbassert based on NDEBUG instead of LLDB_CONFIGURATION_DEBUG (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D15

[Lldb-commits] [PATCH] D150639: [lldb] Define lldbassert based on NDEBUG instead of LLDB_CONFIGURATION_DEBUG

JDevlieghere added a comment. In D150639#4346009 , @rupprecht wrote: > +1 to being surprised this is not already the case > > Some other places should be updated after this, e.g. > lldb/source/Symbol/SymbolFile.cpp also has a use that can be trivially >

[Lldb-commits] [PATCH] D150639: [lldb] Define lldbassert based on NDEBUG instead of LLDB_CONFIGURATION_DEBUG

JDevlieghere added a comment. This patch also uses `__FILE_NAME__` (Clang-specific extension that functions similar to __FILE__ but only renders the last path component (the filename) instead of an invocation dependent full path to that file.) when building with clang. CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D150639: [lldb] Define lldbassert based on NDEBUG instead of LLDB_CONFIGURATION_DEBUG

JDevlieghere created this revision. JDevlieghere added reviewers: bulbazord, labath, mib. Herald added a project: All. JDevlieghere requested review of this revision. Whether assertions are enabled or not is orthogonal to the build type which could lead to surprising behavior for lldbassert. Previ

[Lldb-commits] [PATCH] D150624: [lldb] Fix lua build after 27b6a4e63afe

JDevlieghere added a comment. I have enabled Lua testing on the incremental bot on GreenDragon (https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/). This has always been the intention (note how the job mentions "Python 3 and Lua 5.3") but wasn't enabled until earlier today. Repository

[Lldb-commits] [PATCH] D150621: lldb PlatformDarwinKernel, delay local filesystem scan until needed

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. You can do this simpler with a single `std::once_flag`. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:710-715 + std::lock_gu

[Lldb-commits] [PATCH] D150485: [lldb] Add support for negative integer to {SB, }StructuredData

JDevlieghere added inline comments. Comment at: lldb/include/lldb/API/SBStructuredData.h:70 /// Return the integer value if this data structure is an integer type. - uint64_t GetIntegerValue(uint64_t fail_value = 0) const; + template T GetIntegerValue(T fail_value = {}) con

[Lldb-commits] [PATCH] D150366: [lldb][NFCI] Use llvm's libDebugInfo for DebugRanges

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/D150366/new/ https://reviews.llvm.org/D150366 ___

[Lldb-commits] [PATCH] D150313: Fix libstdc++ data formatter for reference/pointer to std::string

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. I'm surprised this didn't work but was able to reproduce the failure on Linux with libstdcpp. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[Lldb-commits] [PATCH] D150418: [lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Given the numbers I'm surprised you decided to stick with 8 rather than 4? Unless I'm reading them wrong, it seems that despite the number of allocations, it's about as fast and us

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Target/StackFrameList.h:106 + /// Returns true if the function was interrupted, false otherwise. + bool GetFramesUpTo(uint32_t end_idx, bool allow_interrupt = true); I personally would prefer t

[Lldb-commits] [PATCH] D150168: [lldb] Simplify predicates of find_if in BroadcastManager

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

[Lldb-commits] [PATCH] D149774: [lldb] Use templates to simplify {Get, Set}PropertyAtIndex (NFC)

This revision was automatically updated to reflect the committed changes. JDevlieghere marked 2 inline comments as done. Closed by commit rG6f8b33f6dfd0: [lldb] Use templates to simplify {Get,Set}PropertyAtIndex (NFC) (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to comm

[Lldb-commits] [PATCH] D149774: [lldb] Use templates to simplify {Get, Set}PropertyAtIndex (NFC)

JDevlieghere marked 2 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/include/lldb/Interpreter/OptionValue.h:325-344 + template ::value, bool> = true> + std::optional GetValueAs() const { +if constexpr (std::is_same_v) + return GetUInt64Va

[Lldb-commits] [PATCH] D149900: [lldb] Expose a const iterator for SymbolContextList

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149900/new/ https://reviews.llvm.org/D149900 ___ lldb-commits mailing list lldb-commi

[Lldb-commits] [PATCH] D149804: [lldb][NFCI] Add unittests for ObjCLanguage::MethodName

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/D149804/new/ https://reviews.llvm.org/D149804 ___

[Lldb-commits] [PATCH] D149792: Add AArch64 MASK watchpoint support to debugserver

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Some small nits but LGTM Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:1079 + // masked off) -- a MASK value of 31. + uint64_t mas

[Lldb-commits] [PATCH] D149774: [lldb] Use templates to simplify {Get, Set}PropertyAtIndex (NFC)

JDevlieghere updated this revision to Diff 519580. JDevlieghere added a comment. Remove duplicate if-clause CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149774/new/ https://reviews.llvm.org/D149774 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Core/UserSettingsControl

[Lldb-commits] [PATCH] D149774: [lldb] Use templates to simplify {Get, Set}PropertyAtIndex (NFC)

JDevlieghere updated this revision to Diff 519570. JDevlieghere added a comment. Use C++17 to simplify things CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149774/new/ https://reviews.llvm.org/D149774 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Core/UserSettingsContr

[Lldb-commits] [PATCH] D149717: [lldb] Make some functions useful to REPLs public

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:505 + /// that directly use the \a lldb_private namespace and want to use the + /// default LLDB event handler thread instead of defining their own. + bool StartEventHandlerThread();

[Lldb-commits] [PATCH] D149774: [lldb] Use templates to simplify {Get, Set}PropertyAtIndex (NFC)

JDevlieghere created this revision. JDevlieghere added reviewers: labath, bulbazord, jingham. Herald added a subscriber: arphaman. Herald added a project: All. JDevlieghere requested review of this revision. Use templates to simplify `{Get,Set}PropertyAtIndex`. It has always bothered me how cumbe

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

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/D149663/new/ https://reviews.llvm.org/D149663 ___

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb12b35ad4bec: [lldb] Add debugger.external-editor setting (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https

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

JDevlieghere updated this revision to Diff 518558. JDevlieghere marked 2 inline comments as done. JDevlieghere added a comment. Make `external-editor` setting take precedence over `LLDB_EXTRENAL_EDITOR` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149565/new/ https://reviews.llvm.org/D

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

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Core/Disassembler.cpp:805 - ConstString const_key(key.c_str()); + llvm::StringRef key_ref(key); // Check value to

[Lldb-commits] [PATCH] D149567: [lldb] Group debugger properties (NFC)

JDevlieghere updated this revision to Diff 518385. JDevlieghere edited the summary of this revision. JDevlieghere added a comment. Use one big Doxygen group for all the properties to make this less noisy. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149567/new/ https://reviews.llvm.org

[Lldb-commits] [PATCH] D149567: [lldb] Group debugger properties (NFC)

JDevlieghere added a comment. In D149567#4309195 , @mib wrote: > nit: Sometimes we have newlines between methods and sometimes we don't ... It > would be nice to be consistent, otherwise LGTM Methods relating to the same property are grouped together (i

[Lldb-commits] [PATCH] D149567: [lldb] Group debugger properties (NFC)

JDevlieghere created this revision. JDevlieghere added reviewers: mib, bulbazord. Herald added a project: All. JDevlieghere requested review of this revision. Group debugger properties in Doxygen groups. https://reviews.llvm.org/D149567 Files: lldb/include/lldb/Core/Debugger.h Index: lldb/in

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

JDevlieghere marked 2 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:298 + llvm::StringRef GetExternalEditor() const; + bool SetExternalEditor(llvm::StringRef editor); mib wrote: > newline This was i

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

JDevlieghere created this revision. JDevlieghere added reviewers: bulbazord, mib, jingham. Herald added a project: All. JDevlieghere requested review of this revision. Add a setting (`debugger.external-editor`) to specify an external editor instead of having to rely on the `LLDB_EXTERNAL_EDITOR`

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

JDevlieghere closed this revision. JDevlieghere added a comment. 13dbc16b4d82 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149472/new/ https://reviews.llvm.org/D149472 __

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

JDevlieghere reopened this revision. JDevlieghere added a comment. That's my bad: I put the wrong differential URL in my commit (13dbc16b4d82b9adc98c0919873b2b59ccc46afd ) Repository: rG LLVM Github Monorepo CHANGES SINCE

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

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 rG13dbc16b4d82: [lldb] Refactor host::OpenFileInExternalEditor (authored by JDevlieghere). Changed prior to commit: https

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Thanks. This LGTM. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:856 + // user_size == 16 -> aligned_size == 16 + aligned_size = 1

[Lldb-commits] [PATCH] D149503: Remove i386 and armv7 native support from debugserver

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/D149503/new/ https://reviews.llvm.org/D149503 _

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

JDevlieghere added a comment. +1 on making the output deterministic with a sort Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149482/new/ https://reviews.llvm.org/D149482 ___ lldb-commits mailing list ll

[Lldb-commits] [PATCH] D149477: [lldb/crashlog] Fix json module loading issue

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/D149477/new/ https://reviews.llvm.org/D149477 ___

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

JDevlieghere updated this revision to Diff 518007. JDevlieghere marked 10 inline comments as done. JDevlieghere added a comment. - Return an `llvm::Erorr` and percolate errors up - Don't fall back to the default editor if the one specified can't be found - Limit roles to editors only CHANGES SIN

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

JDevlieghere marked 5 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:335 + + const std::string file_path = file_spec.GetPath(); + mib wrote: > I guess this doesn't need to be a `const std::string`,

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

JDevlieghere marked 3 inline comments as done. JDevlieghere 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); + bulbazord wrote: > nit: Move log line below c

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

JDevlieghere created this revision. JDevlieghere added reviewers: aprantl, bulbazord, mib. Herald added a project: All. JDevlieghere requested review of this revision. This patch refactors the macOS implementation of `OpenFileInExternalEditor`: - Fix `AppleEvent` memory leak - Fix caching of `LLD

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

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

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

JDevlieghere added a comment. IMHO this is optimizing the wrong thing. If FileSpec is slow, then we should try to make that faster, instead of updating the different places it's used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149096/new/ https

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

JDevlieghere added inline comments. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:833-835 +std::vector +DNBArchMachARM64::AlignRequestedWatchpoint(nub_addr_t user_addr, + nub_size_t user_size) {

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

JDevlieghere added a comment. In D149214#4300547 , @bulbazord wrote: > In D149214#4300491 , @aprantl wrote: > >> Did you also measure the extra memory consumption? I would be surprised if >> this mattered, but we

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

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/D149300/new/ https://reviews.llvm.org/D149300 ___

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

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

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/D148863/new/ https://reviews.llvm.org/D148863 ___

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

JDevlieghere added inline comments. Comment at: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py:9 + +import os,json,struct,signal +import time bulbazord wrote: > mib wrote: > > bulbazord wrote: > > > nit: import these a

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

This revision was automatically updated to reflect the committed changes. JDevlieghere marked an inline comment as done. Closed by commit rGc1d55d26d373: [lldb] Let Mangled decide whether a name is mangled or not (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

JDevlieghere marked 3 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py:1 +import os +import lldb aprantl wrote: > why? I assume the question is rhetoric, but on the off-chance it's

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

JDevlieghere added a subscriber: dexonsmith. JDevlieghere added a comment. I looked at all the call sites and they all seemed reasonable in terms of doing work on behalf of the user or not and selecting the most relevant frame respectively. My only concern is that we now have a bunch of places w

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

JDevlieghere added a comment. LGTM but I'll hold off on accepting to give Alex a change to take another look as he had more significant comments. Comment at: lldb/source/Interpreter/ScriptInterpreter.cpp:78-91 lldb::DataExtractorSP ScriptInterpreter::GetDataExtractorFromSBDa

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

JDevlieghere added a comment. In D148397#4285143 , @mib wrote: > In D148397#4275792 , @JDevlieghere > wrote: > >> I find "passthrough" somewhat confusing in this context. When using a >> listener in this new mod

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

JDevlieghere updated this revision to Diff 515522. JDevlieghere added a comment. Add simple test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148846/new/ https://reviews.llvm.org/D148846 Files: lldb/include/lldb/Core/Mangled.h lldb/source/Core/Mangled.cpp lldb/source/Plugins/Obj

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

JDevlieghere created this revision. JDevlieghere added reviewers: jingham, labath, jasonmolenda, aprantl. Herald added a subscriber: kristof.beyls. Herald added a reviewer: shafik. Herald added a project: All. JDevlieghere requested review of this revision. We have a handful of places in LLDB wher

[Lldb-commits] [PATCH] D148823: Make diagnostics API safer to use

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

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

JDevlieghere accepted this revision. JDevlieghere added a comment. Ship it CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148400/new/ https://reviews.llvm.org/D148400 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm

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

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/D148679/new/ https://reviews.llvm.org/D148679 ___

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

JDevlieghere added inline comments. Comment at: lldb/source/Target/Process.cpp:404-414 +llvm::StringRef Process::GetAttachSynchronousHijackListenerName() { + return "lldb.internal.Process.AttachSynchronous.hijack"; +} + +llvm::StringRef Process::GetLaunchSynchronousHijackListene

[Lldb-commits] [PATCH] D148603: Remove hardcoding of addressing bits in ABIMacOSX_arm64

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/D148603/new/ https://reviews.llvm.org/D148603 ___

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

JDevlieghere added a comment. I find "passthrough" somewhat confusing in this context. When using a listener in this new mode, where does the event go after it has pass trough this event listener? Maybe my understanding of how the event listeners is incomplete, but I was under the impression th

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

JDevlieghere accepted this revision. JDevlieghere added inline comments. Comment at: lldb/include/lldb/API/SBError.h:26 + SBError(const char *message); + mib wrote: > bulbazord wrote: > > JDevlieghere wrote: > > > Why not a StringRef? > > I'm not sure we have

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

JDevlieghere added inline comments. Comment at: lldb/include/lldb/API/SBError.h:26 + SBError(const char *message); + Why not a StringRef? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148401/new/ https://review

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

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:270-271 +Status ScriptedProcess::EnableBreakpointSite(BreakpointSite *bp_site) { + assert(bp_site != nullptr); + bulbazord wrote: > I like the assert, but

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

JDevlieghere added a comment. I didn't see this until now. I'll update the test. We shouldn't need strip for this test but we're sharing the Makefile for `SymbolFileJSON`. I'll see if it makes more sense to split up the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

JDevlieghere added a comment. I think a lot of this can be simplified by using `LLDB_LOG` instead of `LLDB_LOGF`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148399/new/ https://reviews.llvm.org/D148399 _

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

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/D148380/new/ https://reviews.llvm.org/D148380 ___

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

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

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

JDevlieghere added a comment. Does this actually have to be a //recursive// mutex? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148380/new/ https://reviews.llvm.org/D148380 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D147292: [lldb] Add support for the DW_AT_trampoline attribute with a boolean

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM with Adrian's and my comments addressed. Comment at: lldb/include/lldb/Symbol/Function.h:439 /// The section offset based address for this function. +

[Lldb-commits] [PATCH] D148228: [lldb] Change some pointers to refs in register printing code

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/D148228/new/ https://reviews.llvm.org/D148228 ___

[Lldb-commits] [PATCH] D143062: [lldb] Allow evaluating expressions in C++20 mode

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:518 +// because C++14 is the default standard for Clang but enabling CPlusPlus14 +// expression evaluatino doesn't pass the test-suite cleanly. +lang_

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

This revision was automatically updated to reflect the committed changes. Closed by commit rG27f27d15f6c9: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

JDevlieghere accepted this revision. JDevlieghere added a comment. +1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148175/new/ https://reviews.llvm.org/D148175 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

This revision was automatically updated to reflect the committed changes. Closed by commit rG0e5cdbf07e6d: [lldb] Make ObjectFileJSON loadable as a module (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

JDevlieghere updated this revision to Diff 513327. JDevlieghere marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148172/new/ https://reviews.llvm.org/D148172 Files: lldb/examples/python/crashlog.py lldb/examples/python/scripted_process/crashlog_scripted

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

JDevlieghere added inline comments. Comment at: lldb/examples/python/crashlog.py:515 def parse_images(self, json_images): -idx = 0 -for json_image in json_images: +for idx, json_image in enumerate(json_images): img_uuid = uuid.UUID(json_i

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

JDevlieghere updated this revision to Diff 513047. JDevlieghere marked 4 inline comments as done. JDevlieghere added a comment. Address @mib's code review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148172/new/ https://reviews.llvm.org/D148172 Files: lldb/examples/python/c

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

JDevlieghere marked 7 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/examples/python/crashlog.py:460-464 +def __init__(self, debugger, path, verbose): +super().__init__(debugger, path, verbose) +# List of DarwinImages sorted by t

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

JDevlieghere updated this revision to Diff 513045. JDevlieghere marked 4 inline comments as done. JDevlieghere added a comment. Address @mib's code review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148062/new/ https://reviews.llvm.org/D148062 Files: lldb/include/lldb/Core

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/Core/Section.cpp:683-684 + llvm::json::ObjectMapper o(value, path); + return o && o.map("name", section.name) && o.map("type", section.type) && + o.map("size", sec

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

JDevlieghere updated this revision to Diff 513027. JDevlieghere added a comment. Add missing `CHECK` line in `skipped_status_interactive_crashlog.test`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148172/new/ https://reviews.llvm.org/D148172 Files: lldb/examples/python/crashlog.py

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

JDevlieghere updated this revision to Diff 513025. JDevlieghere retitled this revision from "[lldb] Use ObjectFileJSON to create modules for interactive crashlogs (WIP)" to "[lldb] Use ObjectFileJSON to create modules for interactive crashlogs". JDevlieghere edited the summary of this revision.

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs (WIP)

JDevlieghere created this revision. JDevlieghere added a reviewer: mib. Herald added a project: All. JDevlieghere requested review of this revision. se ObjectFileJSON to create modules for interactive crashlogs. Currently still WIP as I need to update the textual and non-interactive variants.

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

JDevlieghere updated this revision to Diff 512963. JDevlieghere added a comment. Fix bug in ObjectFileJSON where we wouldn't read the rest of the file and fail parsing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148062/new/ https://reviews.llvm.org/D148062 Files: lldb/include/lld

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