[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

2023-10-27 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. @Mordante @Michael137 This seems to fail on older versions of compiler/libcxx https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-matrix/7247/ You can probably fix this by just requiring a minimum clang version in the tests Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

2023-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:996 + TypeSummaryImplSP(new StringSummaryFormat( + eTypeOptionHideChildren | eTypeOptionHideValue, "${var.__rep_}"))); } Nice! ===

[Lldb-commits] [PATCH] D158958: [LLDB][REPL] Change the default tab size

2023-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. > @aprantl, how should I proceed with the swift branch? Should I create a new > branch in https://github.com/apple/swift and share it here so it's available > for whoever does the merge? Actually, I can just cherry-pick it myself, let's not worry about this. Repositor

[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()

2023-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Is this covered by any tests? Comment at: lldb/source/Core/IOHandler.cpp:512 +StringList IOHandlerEditline::GetCurrentLines() const { +#if LLDB_ENABLE_LIBEDIT + if (m_editline_up) I think this would benefit from a comment explaining th

[Lldb-commits] [PATCH] D158958: [LLDB][REPL] Change the default tab size

2023-08-28 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This should probably be a language-specific setting, but I see no problem in doing it this way right now, since there is practically only one language. Please update any REPL tests on the Swi

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

2023-08-24 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Test LGTM, I'll defer to others for the dynamic loader plugin. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158583/new/ https://reviews.llvm.org/D158583 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

2023-08-23 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/test/API/functionalities/dyld-multiple-rdebug/TestDyldWithMultupleRDebug.py:54 +self.assertIn("main", + thread.GetFrameAtIndex(0).GetDisplayFunctionName()) +process.Continue() Y

[Lldb-commits] [PATCH] D158470: [lldb] Add support for recognizing swift mangled names

2023-08-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Core/Mangled.cpp:61 + // Swift's older style of mangling used "_T" as a mangling prefix. This can + // lead to false positives with other symbols that just so happen to start fdeazeve wrote: > JDevliegher

[Lldb-commits] [PATCH] D158470: [lldb] Add support for recognizing swift mangled names

2023-08-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Core/Mangled.cpp:61 + // Swift's older style of mangling used "_T" as a mangling prefix. This can + // lead to false positives with other symbols that just so happen to start Feel free to completely ignor

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

2023-08-16 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/include/lldb/Core/Module.h:127 const FileSpec &file_spec, const ArchSpec &arch, - const ConstString *object_name = nullptr, - lldb::offset_t object_offset = 0, + const ConstString object_name, lldb::offset_t ob

[Lldb-commits] [PATCH] D157512: [lldb][PlatformDarwin] Only parse SDK from debug-info if target language is Objective-C

2023-08-14 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Apart from Jim's comment (does this still work?) this seems like a reasonable heuristic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157512/

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

2023-08-04 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. > If it's straightforward, I think would be nice to have a unit test with two > threads modifying the same OptionValue. That way a TSan run would catch this > issue. If that's more work than expected then this is fine as is. We might just want to set up a bot that runs

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

2023-08-04 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Interpreter/OptionValue.cpp:55 const OptionValueBoolean *OptionValue::GetAsBoolean() const { + std::lock_guard lock(m_mutex); if (GetType() == OptionValue::eTypeBoolean) If you are following @kastiglione

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

2023-07-26 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1363 + XcodeSDK sdk; + for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) +if (auto

[Lldb-commits] [PATCH] D156279: [lldb] Increase the default timeouts when running under ASan

2023-07-25 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nevermind, you answered my question in the description already! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156279/new/ https://reviews.llvm.org/D156279 _

[Lldb-commits] [PATCH] D156279: [lldb] Increase the default timeouts when running under ASan

2023-07-25 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. This LGTM, but I'm surprized you didn't have to delete any older code that tried to do the same thing. Did the thing I remember not survive the transition to tablegen or is this orthogonal? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156279/new/ https://revi

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

2023-07-24 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/include/lldb/Target/Platform.h:479 +/// to an internal SDK +bool found_internal_sdk = false; + These flags really only make sense in the context of an XcodeSDK, so why not just return an XcodeSDK or XcodeSD

[Lldb-commits] [PATCH] D155198: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

2023-07-21 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf09f0a6b1076: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr (authored by fdeazeve, committed by aprantl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D155198: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

2023-07-20 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. As far as I'm concerned, this doesn't look too controversial to me and it is unblocking one of the bots. I think it would be okay to tentatively land this, but be on the lookout and promptly react to any post-commit feedback from @clayborg. Repository: rG LLVM Githu

[Lldb-commits] [PATCH] D153054: [lldb][NFCI] TypeSystemClang::GetTypeForIdentifier should take a StringRef

2023-06-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:76 + + compiler_type = scratch_ts_sp->GetTypeForIdentifier(g_lldb_autogen_nspair); + w

[Lldb-commits] [PATCH] D152872: Add support for __debug_line_str in Mach-O

2023-06-14 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG27fac4a72ae5: Add support for __debug_line_str in Mach-O (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D152872: Add support for __debug_line_str in Mach-O

2023-06-13 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision. aprantl added reviewers: bulbazord, fdeazeve, rastogishubham, JDevlieghere. Herald added a project: All. aprantl requested review of this revision. This patch resolves an issue that currently accounts for the vast majority of failures on the matrix bot. https://re

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

2023-06-13 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Core/ValueObjectDynamicValue.cpp:169 + if (known_type == lldb::eLanguageTypeObjC && + UseSwiftRuntime(*m_parent, exe_ctx)) { +runtime = process->GetLanguageRuntime(lldb::eLanguageTypeSwift); This is

[Lldb-commits] [PATCH] D152590: Streamline expression parser error messages

2023-06-12 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG133c3eaac0a5: Streamline expression parser error messages. (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINC

[Lldb-commits] [PATCH] D152590: Streamline expression parser error messages

2023-06-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. pinging after the weekend. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152590/new/ https://reviews.llvm.org/D152590 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li

[Lldb-commits] [PATCH] D152708: [RFC][Draft] Enable primitive support for Two-Level Line Tables in LLVM

2023-06-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Thanks for prototyping this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152708/new/ https://reviews.llvm.org/D152708 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D152590: Streamline expression parser error messages

2023-06-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision. aprantl added reviewers: jingham, kastiglione, JDevlieghere. Herald added a project: All. aprantl requested review of this revision. Currently the expression parser prints a mostly useless generic error before printing the compiler error: (lldb) p 1+x) error: e

[Lldb-commits] [PATCH] D152560: Make TestStepNodebug more robust against codegen changes

2023-06-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl abandoned this revision. aprantl added a comment. According to an offline conversation with @jingham this is still not the expected behavior from the stepping engine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152560/new/ https://reviews.llvm.org/D152560 ___

[Lldb-commits] [PATCH] D152560: Make TestStepNodebug more robust against codegen changes

2023-06-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. lldb) image dump line-table with-debug.c Line table for /Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c in `a.out 0x00013d4c: /Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debu

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

2023-06-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. That looks useful! Comment at: lldb/scripts/generate-project.py:175 +# Building Objects and their respective swiftmodule +f.write(f"$(OBJDIR)/%.o: %.swift objdir\n") +f.write("\t$(SWIFT_FE) -c -primary-file $< -I objs/ \\\n") ---

[Lldb-commits] [PATCH] D152560: Make TestStepNodebug more robust against codegen changes

2023-06-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision. aprantl added reviewers: jingham, Michael137. Herald added a subscriber: kristof.beyls. Herald added a project: All. aprantl requested review of this revision. Depending on the compiler I've seen that for example the step-in command can do a column-step first before

[Lldb-commits] [PATCH] D152476: [lldb] Remove lldb's DWARFAbbreviationDeclarationSet in favor of llvm's

2023-06-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. Nice! Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:61 void DWARFDebugAbbrev::GetUnsupportedForms( std::set &invalid_forms) const { + for (const auto &pair : m_abbrevCollMap) { -

[Lldb-commits] [PATCH] D152409: [lldb] Never print children if the max depth has been reached

2023-06-09 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Would that be testable by implementing a python data formatter for a C struct that unconditionally returns an error? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152409/new/ https://reviews.llvm.org/D152409 _

[Lldb-commits] [PATCH] D151950: [lldb] Unconditionally increment depth when printing children

2023-06-08 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added inline comments. Comment at: lldb/source/DataFormatters/ValueObjectPrinter.cpp:610 if (child_sp.get()) { ValueObjectPrinter child_printer( I would probably factor out: ``` if (does_consume_ptr_depth) --cu

[Lldb-commits] [PATCH] D152324: [lldb][NFCI] Change return type of PersistentExpressionState::GetNextPersistentVariableName

2023-06-07 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Is the idea to remove the ConstString from these APIs too? If yes, this LGTM, otherwise it seems to needlessly complicate things. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152324/new/ https://reviews.llvm.org/D152324

[Lldb-commits] [PATCH] D152319: [lldb] Run crashlog inside lldb when invoked in interactive mode from the command line

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This is super convenient! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152319/new/ https://reviews.llvm.org/D152319 ___ lldb-co

[Lldb-commits] [PATCH] D152315: [lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:868 +lldb::BasicType TypeSystemClang::GetBasicTypeEnumeration(llvm::StringRef name) { + if (name.empty()) +return eBasicTypeInvalid; bulbazord wrote: > apran

[Lldb-commits] [PATCH] D152315: [lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:868 +lldb::BasicType TypeSystemClang::GetBasicTypeEnumeration(llvm::StringRef name) { + if (name.

[Lldb-commits] [PATCH] D152225: [lldb] fix dangling reference in `ClangHost.cpp`

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Thanks! FWIW, it seems like an unnecessary micro optimization to make both these variables static. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152225/new/ https://reviews.llvm.org/D152225 __

[Lldb-commits] [PATCH] D152225: [lldb] fix dangling reference in `ClangHost.cpp`

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6c02e365711c: [lldb] fix dangling reference in `ClangHost.cpp` (authored by paperchalice, committed by aprantl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D151950: [lldb] Unconditionally increment depth when printing children

2023-06-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Is ity possible to test this with a nested C++ type? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151950/new/ https://reviews.llvm.org/D151950 ___ lldb-commits mailing list lldb

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

2023-06-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:889 +"Package unit with a non-zero abbreviation offset"); + + auto *unit_contrib = index_entry->

[Lldb-commits] [PATCH] D151351: [lldb] Prevent dwim-print from showing kNoResult error

2023-05-30 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:1 //===-- CommandObjectDWIMPrint.cpp --*- C++ -*-===// // ni

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

2023-05-30 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. If we can come up with a counterexample where the heuristic in this patch is doing the wrong thin then I think emitting a DW_AT_LLVM_no_unique_address attribute sounds reasonable to me. But otherwise I don't see a problem with using a heuristic. Repository: rG LLVM

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

2023-05-26 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. In D151588#4377128 , @bulbazord wrote: > What is the motivation behind this change? > > Also, why did you factor out `--show-sdk-path` into an argument? I assume you > want to use `xcrun` for other purposes? This is in preparati

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

2023-05-26 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision. aprantl added reviewers: bulbazord, JDevlieghere. Herald added a project: All. aprantl requested review of this revision. This is an API needed by swift-lldb. https://reviews.llvm.org/D151591 Files: lldb/include/lldb/Host/HostInfoBase.h lldb/include/lldb/Host/

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

2023-05-26 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl updated this revision to Diff 526194. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151588/new/ https://reviews.llvm.org/D151588 Files: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

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

2023-05-26 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision. aprantl added reviewers: JDevlieghere, bulbazord. Herald added a project: All. aprantl requested review of this revision. https://reviews.llvm.org/D151588 Files: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX

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

2023-05-25 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. > @aprantl Do you know if we can detect the end of the scope ? I'm not sure > it's possible currently ... But even if we could do it, that wouldn't cover > the following case: When setting a variable watchpoint, you would have to store the scope the variable is in, and

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

2023-05-24 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Just so I understand the limitations: This works for int main() { int val = 0; // Break here val++; val++; return 0; } but not for int main() { { int val = 0; // Break here val++; val++; } { int other =

[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

2023-05-18 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp:155 + llvm::StringRef selector_name = GetSelector(); + std::string name_sans_category; + bulbazord wrote: > aprantl wrote: > > Using an llvm string stream to construct

[Lldb-commits] [PATCH] D150716: [lldb][NFCI] Switch to using llvm::DWARFAbbreviationDeclaration

2023-05-18 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:56 if (m_idx_offset == UINT32_MAX) { -DWARFAbbreviationDeclarationCollConstIter pos; -DWARFAbbreviationDeclarationCollConstIter end = m_decls.end(); -for (pos = m_d

[Lldb-commits] [PATCH] D150716: [lldb][NFCI] Switch to using llvm::DWARFAbbreviationDeclaration

2023-05-18 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Overall seems good to me. Minor comments inline. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:27 lldb::of

[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

2023-05-18 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. One more comment inline but otherwise this is fine with me. Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp:155 + llvm::StringRef selector_name = GetSelecto

[Lldb-commits] [PATCH] D150590: [lldb][DWARFASTParserClang][NFC] Extract condition for unnamed bitfield creation into helper function

2023-05-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:239 + bool ShouldCreateUnnamedBitfield( + FieldInfo const &last_field_info, uint64_t last

[Lldb-commits] [PATCH] D150383: [lldb] Correct elision of line zero in mixed disassembly

2023-05-11 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/test/Shell/Commands/command-disassemble-mixed.test:12 + +// CHECK-NOT: do_not_show Can you add some positive check too? Otherwise this test succeeds even on an empty output.. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

2023-05-11 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. I'm not opposed to using this implementation, but have you considered using something like the stdlib regex library to do the heavy lifting? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149914/new/ https://reviews.llvm.or

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

2023-05-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp:203 +.Case(".debug_pubnames", eSectionTypeDWARFDebugPubNames) +.Case(".debug_pubtypes", eSectionTypeDWARFDebugPubTypes) +.Case(".debug_str", eSect

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

2023-05-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Is there a way to compile something with clang and then load the object file into lldb-test so we could add a shell test for it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149987/new/ https://reviews.llvm.org/D149987 _

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

2023-05-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Could you build this into a yaml2obj obj2yaml plugin, so we could test this? Alternatively, you could check in a minimal binary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149987/new/ https://reviews.llvm.org/D149987 __

[Lldb-commits] [PATCH] D149949: [lldb][TypeSystem] ForEach: Don't hold the TypeSystemMap lock across callback

2023-05-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Do we have other ForEach methods that contain a similar footgun? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149949/new/ https://reviews.llvm.org/D149949 ___ lldb-commits maili

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

2023-05-04 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Please do more of this! I haven't checked all the replacements for correctness but the direction is good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[Lldb-commits] [PATCH] D149702: [DebugInfo] Add language code for the new Mojo language

2023-05-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. You can also use a constant in the Vendor extension space, which might be more appropriate until the languages sees adoption by a wider audience. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149702/new/ https://reviews.ll

[Lldb-commits] [PATCH] D149702: [DebugInfo] Add language code for the new Mojo language

2023-05-02 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/include/lldb/lldb-enumerations.h:493 eLanguageTypeAda2012 = 0x002f, + eLanguageTypeMojo = 0x0030, bulbazord wrote: > These values correspond to DWARF5's official language codes and `0x0030` is > technically a

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

2023-05-02 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. I'll try adding a `# REQUIRES: lld` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147370/new/ https://reviews.llvm.org/D147370 ___ lldb-commits mailing list lldb-commits@lists.ll

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

2023-05-02 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. This test is failing on Darwin: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/lastFailedBuild/testReport/lldb-shell/SymbolFile_DWARF_x86/DW_OP_div_with_signed_s/ https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/lastFailedBuild/ ld: unknown option: -

[Lldb-commits] [PATCH] D148662: [lldb] Make the libcxx unique_ptr prettyprinter support custom deleters.

2023-05-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Thanks for the quick response! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148662/new/ https://reviews.llvm.org/D148662 ___ lldb-commits mailing list lldb-commits@lists.llvm.or

[Lldb-commits] [PATCH] D148662: [lldb] Make the libcxx unique_ptr prettyprinter support custom deleters.

2023-05-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Looks like this broke on the Darwin incremental bot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/54508/ Can you please take a look and revert in the mean time? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

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

2023-04-26 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149214/new/ https:/

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

2023-04-21 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py:7 + + +class TestFunctionNameWithoutArgs(TestBase): I guess this is a no debug info test? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148846/new/ ht

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

2023-04-21 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py:1 +import os +import lldb why? Comment at: lldb/test/API/macosx/format/main.c:2 +#include +#include + why? CHANGES SINCE

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

2023-04-21 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. It seems like we can quickly and unambiguously decide whether a name is mangled or not by looking at the first characters so that seems fine to me. Does Mangled try all Language plugins to d

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

2023-04-21 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG737820e6d6e2: Make diagnostics API safer to use (authored by aprantl). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

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

2023-04-20 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision. aprantl added reviewers: kastiglione, JDevlieghere, Michael137. Herald added a project: All. aprantl requested review of this revision. I received a crash report in DiagnosticManager that was causes by a nullptr diagnostic having been added. The API allows passing i

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

2023-04-13 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. Okay, then let the refactor fest begin! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148175/new/ https://reviews.llvm.org/D148175 ___ lldb-commit

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

2023-04-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. I understand that the opposite direction is explicit because actual work is being done. In this direction, it shouldn't affect memory management (since ConstStrings live forever) or performance, so I think this is good (and very convenient!). Does anyone else see a prob

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

2023-04-10 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:439 /// The section offset based address for this function. + /// \param[in] generic_trampoline + /// If this function is a generic trampoline. A generic trampoline Is th

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

2023-04-04 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. So the new bool flag effectively implements the leading `::` separator in the lookup. Seems reasonable! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D147362: [lldb] Add Clang module import logging

2023-04-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. This looks great, and I think this should also be easy to test? I think we have many other tests that write out a logfile and FileCheck the result. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147362/new/ https://reviews.

[Lldb-commits] [PATCH] D147385: [lldb] Add fixits for "frame variable"

2023-04-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. That looks like a nice quality of life improvement! Personally I would lean towards replicating the expr behavior, though I don't know how much work that would be. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147385/new/

[Lldb-commits] [PATCH] D143061: [lldb][Language] Add more language types

2023-04-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Well, I guess we'll have to revisit this for DWARF 6 split of language names and versions anyway Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

2023-04-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Just out of curiosity: namespace A { namespace B { namespace C { struct Bar {}; } } } namespace B { namespace C { struct Foo {}; } } Does this work? (lldb) expr C::Foo f Comment at: lldb/include/lldb/Symbol/SymbolFile.h:3

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

2023-03-28 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks for checking! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147012/new/ https://reviews.llvm.org/D147012 ___ lldb-commits mailing

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

2023-03-28 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Target/Language.cpp:272 + case eLanguageTypeC_plus_plus_17: + case eLanguageTypeC_plus_plus_20: case eLanguageTypeObjC_plus_plus: Can you check if we already have a similar function in Dwarf.h in LLVM?

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

2023-03-28 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp:205 +const lldb::offset_t slice_file_offset = +fat_arch.GetOffset() + file_offset; +if (fat_arch.GetOffset() < file_size

[Lldb-commits] [PATCH] D145609: [lldb] Change dwim-print to default to disabled persistent results

2023-03-23 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. > breaks basically anything that examines debugger output I'd be curious to learn about some example you have in mind here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145609/new/ https://reviews.llvm.org/D145609 __

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

2023-03-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137 + // See if we have made this type before and can reuse it. + CompilerType fields_type = ast->GetTypeForIdentifier( + ConstString(register_type_name.c_str())); + Davi

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

2023-03-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Commands/CommandObjectRegister.cpp:229 +if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info, + /*print_flags=*/true, type_system)) strm.Printf("%-12s = error: unavailab

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

2023-03-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137 + // See if we have made this type before and can reuse it. + CompilerType fields_type = ast->GetTypeForIdentifier( + ConstString(register_type_name.c_str())); + Davi

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

2023-03-21 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137 + // See if we have made this type before and can reuse it. + CompilerType fields_type = ast->GetTypeForIdentifier( + ConstString(register_type_name.c_str())); + bulb

[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms

2023-03-21 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks for adding the check! Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:185 + llvm::StringRef filename = module_file_s

[Lldb-commits] [PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

2023-03-21 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2640 if (!D || !D->isCompleteDefinition()) -return FwdDecl; +return {FwdDecl, nullptr}; I'm curious if this works if we encounter a forward declaration, early exit here, th

[Lldb-commits] [PATCH] D146265: [lldb] Introduce SymbolFile::ParseAllLanguages

2023-03-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. You could add a test similar to lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp Comment at: lldb/include/lldb/Symbol/SymbolFile.h:154 + /// case this function sho

[Lldb-commits] [PATCH] D146320: [lldb] Test direct ivar access in objc++ (NFC)

2023-03-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Otherwise LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146320/new/ https://reviews.llvm.org/D146320 ___

[Lldb-commits] [PATCH] D146320: [lldb] Test direct ivar access in objc++ (NFC)

2023-03-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/test/API/commands/frame/var/direct-ivar/objcpp/main.mm:6 + void fun() { +// check this + } I would recommend using `printf("check this")` to make 100% sure that there is code on this line. Repository: rG

[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms

2023-03-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Here is an example of how the Swift Runtime plugin detects both itself and whether ObjC interop is enabled. Basically all I'm asking is to not accidentally break this mechanism. https://github.com/apple/llvm-project/blob/7750ffa35111df6d38a5c73ab7e78ccebc9b43c3/lldb/sour

[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms

2023-03-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. In D146154#4198730 , @sgraenitz wrote: > In D146154#4197454 , @aprantl wrote: > >> One thing I just realized — we need to make sure that we don't accidentally >> create a GNUstep ObjC run

[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms

2023-03-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. I.e., can you detect based on the presence of a symbol or shared object that an GNUstep runtime is present? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146154/new/ https://reviews.llvm.org/D146154 __

[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms

2023-03-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl requested changes to this revision. aprantl added a comment. This revision now requires changes to proceed. One thing I just realized — we need to make sure that we don't accidentally create a GNUstep ObjC runtime in a Swift process that was built without ObjC support on Linux. How can w

[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms

2023-03-15 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. In general I don't think I have a problem with adding this, especially since Clang also supports the runtime as a compilation target, and LLDB is tightly integrated with Clang. Repository:

  1   2   3   4   5   6   7   8   9   10   >