[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

mib updated this revision to Diff 528069. mib added a comment. Address @bulbazord comments: - Add test & usage descriptions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152011/new/ https://reviews.llvm.org/D152011 Files: lldb/docs/python_api_enums.rst

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

mib updated this revision to Diff 528058. mib added a comment. Add test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152013/new/ https://reviews.llvm.org/D152013 Files: lldb/source/Commands/CommandCompletions.cpp lldb/test/API/functionalities/completion/TestCompletion.py Index:

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

smurfd abandoned this revision. smurfd added a comment. Hey, sorry for wasting your time with this. Should have read the error message more carefully. Its related to `pkgsrc` missing `getopt_long_only()` in `getopt.h` Checking/filing a bug towards them instead Repository: rG LLVM Github

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf5371eb3d3ae: [Damangle] convert dlangDemangle to use std::string_view (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo

[Lldb-commits] [lldb] f5371eb - [Damangle] convert dlangDemangle to use std::string_view

Author: Nick Desaulniers Date: 2023-06-02T15:19:41-07:00 New Revision: f5371eb3d3aed06ba84a69533586a60243ad2f24 URL: https://github.com/llvm/llvm-project/commit/f5371eb3d3aed06ba84a69533586a60243ad2f24 DIFF:

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

nickdesaulniers updated this revision to Diff 528011. nickdesaulniers added a comment. This revision is now accepted and ready to land. - rebase on D149675 , remove nullptr check No, I don't want no nullptr-check A nullptr-check is a guard that can't get no love

[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG12d967c95f16: [Damangle] convert rustDemangle to use std::string_view (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [lldb] 12d967c - [Damangle] convert rustDemangle to use std::string_view

Author: Nick Desaulniers Date: 2023-06-02T15:08:14-07:00 New Revision: 12d967c95f1633bebd1b225ddd53573951a7ca43 URL: https://github.com/llvm/llvm-project/commit/12d967c95f1633bebd1b225ddd53573951a7ca43 DIFF:

[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

nickdesaulniers updated this revision to Diff 528006. nickdesaulniers added a comment. - rebase on top of D149675 (I wasn't sure which would land first) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149784/new/

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

nickdesaulniers added inline comments. Comment at: llvm/lib/Demangle/Demangle.cpp:49 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string ) { + if (!MangledName) +return false; MaskRay wrote: > nickdesaulniers wrote: > > nickdesaulniers

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. If `nonMicrosoftDemangle` is going to be changed to take a `string_view` argument, adding `if (!MangledName) return false;` should be fine. If possible, it's best for the change to be

[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

fdeazeve accepted this revision. fdeazeve added a comment. LGTM, thanks for picking this up! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152010/new/ https://reviews.llvm.org/D152010 ___ lldb-commits

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

kastiglione added a comment. To expand the conversation, I have also opened D152031 which makes the same change to `GetChildAtIndex`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151966/new/

[Lldb-commits] [PATCH] D152031: [lldb] Default can_create to true in GetChildAtIndex (NFC)

kastiglione created this revision. kastiglione added reviewers: bulbazord, jingham. Herald added a subscriber: arphaman. Herald added a project: All. kastiglione requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Existing callers of

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

kastiglione added a comment. correction: there's still an overload of `GetChildAtNamePath`, which takes a can_create value and passes it through to `GetChildMemberWithName`. However that function isn't used, and could be deleted. To delete the parameter, that overload of

[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

MaskRay added inline comments. Comment at: llvm/lib/Demangle/RustDemangle.cpp:150 -char *llvm::rustDemangle(const char *MangledName) { - if (MangledName == nullptr) -return nullptr; I see that this patch drops `if (MangledName == nullptr)` (D101444). This

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

MaskRay added inline comments. Comment at: llvm/lib/Demangle/Demangle.cpp:49 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string ) { + if (!MangledName) +return false; nickdesaulniers wrote: > nickdesaulniers wrote: > > MaskRay wrote: > >

[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

mib added a comment. In D152012#4391336 , @bulbazord wrote: > The key here is that you're expanding the path and checking for the validity > before calling `CrashLogParser.create()` right? It looks like for the > `load_crashlog_in_scripted_process`

[Lldb-commits] [PATCH] D151919: [lldb][NFCI] Apply IndexEntry to DWARFUnitHeader outside of extraction

bulbazord added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:882 + // we have a valid one to set it to. + assert(index_entry); + assert(!m_index_entry); fdeazeve wrote: > If LLVM's function is like this as well, we shouldn't

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

bulbazord added a comment. So the idea behind this is fine to me, allowing custom commands to specify a completion style seems like a very nifty feature to have. We should definitely add a test though. An API test or a Shell test where we create a custom command and then try to do some kind of

[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. The key here is that you're expanding the path and checking for the validity before calling `CrashLogParser.create()` right? It looks like for the `load_crashlog_in_scripted_process`

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

bulbazord added a comment. The change itself looks ok to me. Could you add a test for this? We do have some tests to test for completion so the machinery is there, but it's mostly for the expression evaluator right now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

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

[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

bulbazord updated this revision to Diff 527908. bulbazord added a comment. Remove `const` where unneeded. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152010/new/ https://reviews.llvm.org/D152010 Files: lldb/include/lldb/Utility/ConstString.h

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

mib created this revision. mib added reviewers: JDevlieghere, bulbazord, kastiglione. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should fix path completion starting from the root directory. To

[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

mib created this revision. mib added reviewers: bulbazord, kastiglione. mib added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should fix a crash in the opening a

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

mib created this revision. mib added reviewers: bulbazord, jingham, JDevlieghere. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should allow the user to set specific auto-completion type for their

[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

kastiglione added inline comments. Comment at: lldb/source/Utility/ConstString.cpp:305 -void ConstString::SetString(const llvm::StringRef ) { - m_string = StringPool().GetConstCStringWithLength(s.data(), s.size()); +void ConstString::SetString(const llvm::StringRef s) { +

[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

bulbazord added inline comments. Comment at: lldb/source/Utility/ConstString.cpp:305 -void ConstString::SetString(const llvm::StringRef ) { - m_string = StringPool().GetConstCStringWithLength(s.data(), s.size()); +void ConstString::SetString(const llvm::StringRef s) { +

[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

kastiglione added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/source/Utility/ConstString.cpp:305 -void ConstString::SetString(const llvm::StringRef ) { - m_string = StringPool().GetConstCStringWithLength(s.data(), s.size()); +void

[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

bulbazord created this revision. bulbazord added reviewers: fdeazeve, kastiglione, mib, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. StringRef was made to be passed by value

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

nickdesaulniers updated this revision to Diff 527900. nickdesaulniers added a comment. - rebase for presubmits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151003/new/ https://reviews.llvm.org/D151003 Files: lldb/source/Core/Mangled.cpp

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: llvm/lib/Demangle/Demangle.cpp:49 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string ) { + if (!MangledName) +return false;

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: llvm/lib/Demangle/Demangle.cpp:49 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string ) { + if (!MangledName) +return false; MaskRay

[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

nickdesaulniers updated this revision to Diff 527895. nickdesaulniers added a comment. - rebase for presubmits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149784/new/ https://reviews.llvm.org/D149784 Files:

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

DavidSpickett added a comment. > None of this should change behavior on linux, but if folks from the linux > world have a comment or reaction to this change, I would be interested to > hear opinions. I haven't done much testing beyond the one test corefile, and > I still need to work out how

[Lldb-commits] [PATCH] D151919: [lldb][NFCI] Apply IndexEntry to DWARFUnitHeader outside of extraction

fdeazeve accepted this revision. fdeazeve added a comment. I'm not exactly familiar with DWOs, but the code motions LGTM! Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:882 + // we have a valid one to set it to. + assert(index_entry); +

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

mib updated this revision to Diff 527776. mib added a comment. Address @bulbazord comments: - Make `Target::SetLabel` return an `llvm::Error` to propagate error messages to both the `CommandObject` and the `SBAPI` callers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151859/new/

Re: [Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

That's an option too, which I considered. I went with this as a more conservative change. If there's agreement to remove it then I can change it. On Thursday, June 1, 2023, Alex Langford via Phabricator < revi...@reviews.llvm.org> wrote: > bulbazord added a comment. > > If every caller sets this

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

bulbazord added a comment. If every caller sets this to true, why not remove the argument altogether? It looks like `ValueObjectRegister::GetChildMemberWithName` doesn't use the argument, `ValueObject::GetChildMemberWithName` and `ValueObjectSynthetic::GetChildMemberWithName` just pass it