[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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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

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] 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] 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] 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. 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D151460: [NFC][Py Reformat] Reformat python files in lldb

JDevlieghere created this revision. JDevlieghere added reviewers: LLDB, thieta. Herald added subscribers: pmatos, asb, ormris, mstorsjo, wenlei, arphaman, steven_wu, kbarton, hiraditya, fedor.sergeev, sbc100, nemanjai, emaste. Herald added a reviewer: alexander-shaposhnikov. Herald added a project

[Lldb-commits] [PATCH] D151460: [NFC][Py Reformat] Reformat python files in lldb

JDevlieghere added a comment. In D151460#4372711 , @thieta wrote: > Thanks for doing this! Going to assume you ran the tests and it worked :) Yup, no regressions. I'll keep an eye on the bots to make sure that holds true for other configurations too.

[Lldb-commits] [PATCH] D151460: [NFC][Py Reformat] Reformat python files in lldb

JDevlieghere added a comment. In D151460#4372779 , @thieta wrote: > In D151460#4372763 , @mib wrote: > >> Can you add this commit hash to the `.git-blame-ignore-revs` file so it >> doesn't show up in the git hist

[Lldb-commits] [PATCH] D151451: [lldb][nfc] Refactor methods with out parameter

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

[Lldb-commits] [PATCH] D151460: [NFC][Py Reformat] Reformat python files in lldb

This revision was automatically updated to reflect the committed changes. Closed by commit rG2238dcc39358: [NFC][Py Reformat] Reformat python files in lldb (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D151460?vs=525647&id=525759#toc

[Lldb-commits] [PATCH] D151497: [lldb] Improve function caller error message

JDevlieghere created this revision. JDevlieghere added reviewers: jingham, aprantl, bulbazord, mib. Herald added a project: All. JDevlieghere requested review of this revision. When trying to run an expression after a process has existed, you currently are shown the following error message: (l

[Lldb-commits] [PATCH] D151497: [lldb] Improve function caller error message

JDevlieghere added a comment. In D151497#4374080 , @bulbazord wrote: > I think it's good to improve the error messaging but I think we can probably > do better. "Function caller" is specific to the internals of LLDB and isn't > really meaningful for man

[Lldb-commits] [PATCH] D151497: [lldb] Improve function caller error message

JDevlieghere updated this revision to Diff 525861. JDevlieghere added a comment. Rephrase error message for `UserExpression::Evaluate` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151497/new/ https://reviews.llvm.org/D151497 Files: lldb/source/Expression/UserExpression.cpp lldb/so

[Lldb-commits] [PATCH] D151497: [lldb] Improve function caller error message

JDevlieghere added inline comments. Comment at: lldb/source/Expression/UserExpression.cpp:211 +error.SetErrorStringWithFormatv( +"unable to evaluate expression while the process is {0}: the process " +"must be stopped because the expression might requires allo

[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

JDevlieghere added inline comments. Comment at: lldb/source/Breakpoint/Watchpoint.cpp:127 + +bool Watchpoint::VariableWatchpointDisabler(void *baton, +StoppointCallbackContext *context, I think this should return an `ll

[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

JDevlieghere added inline comments. Comment at: lldb/source/Breakpoint/Watchpoint.cpp:127 + +bool Watchpoint::VariableWatchpointDisabler(void *baton, +StoppointCallbackContext *context, mib wrote: > JDevlieghere wrote:

[Lldb-commits] [PATCH] D151524: [lldb][NFCI] Change type of SBDebugger::m_instance_name

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

[Lldb-commits] [PATCH] D151497: [lldb] Improve function caller error message

This revision was automatically updated to reflect the committed changes. Closed by commit rGe31f99464216: [lldb] Improve error message when evaluating expression when not stopped (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D151497

[Lldb-commits] [PATCH] D151516: [lldb][NFCI] Remove use of ConstString from UnixSignals::SignalCode

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

[Lldb-commits] [PATCH] D150805: Rate limit progress reporting

JDevlieghere added a comment. My suggestion was to add rate limiting support to the listener so that all events could benefit from this. The current patch seems unnecessary ad-hoc: it's limited to (1) progress events and (2) the default event handler. I expects that lldb-vscode plugin users suf

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

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. I suspect you'll run into the same problem when you get the API tests working, but we can deal with that when the time comes. Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

JDevlieghere added a comment. I think this test could benefit from switching to string literals (`'''`) and doing a single `self.assertIn` rather than scanning the whole output for every line. Also please use `black` (version 23) to format the test. CHANGES SINCE LAST ACTION https://reviews.

[Lldb-commits] [PATCH] D151570: Fix Build error on Mac M1 : error: use of undeclared identifier 'getopt_long_only'; did you mean 'getopt_long'?

JDevlieghere added a comment. What host compiler are you using when you see this error? `getopt_long_only` is available on macOS. Does this compile for you? #include int main(int argc, char** argv) { struct option longopts[] = { { "foo", required_argument, 0, 'f' }, };

[Lldb-commits] [PATCH] D151591: HostInfoMacOS: Add a utility function for finding an SDK-specific tool

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/unittests/Host/HostInfoTest.cpp:90 + EXPECT_FALSE(find_tool("MacOSX.sdk", "clang").empty()); + EXPECT_TRUE(find_tool("MacOSX.sdk", "CeciNe

[Lldb-commits] [PATCH] D151588: Factor out xcrun (NFC)

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

<    14   15   16   17   18   19   20   21   22   23   >