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

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.

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

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.

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

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

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

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

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

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

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

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()

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

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: >

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

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

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

aprantl added inline comments. Comment at: lldb/include/lldb/Core/Module.h:127 const FileSpec _spec, const ArchSpec , - const ConstString *object_name = nullptr, - lldb::offset_t object_offset = 0, + const ConstString object_name, lldb::offset_t

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

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

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

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

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

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

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

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

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/

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

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

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

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

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

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

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

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); +

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

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

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.

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

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

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

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

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

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

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

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

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:

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

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

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:

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

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

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

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

aprantl accepted this revision. aprantl added a comment. Nice! Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:61 void DWARFDebugAbbrev::GetUnsupportedForms( std::set _forms) const { + for (const auto : m_abbrevCollMap) { at some

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

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

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)

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

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

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-commits] [PATCH] D152315: [lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration

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: >

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

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

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

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`

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

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

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-commits] [PATCH] D151919: [lldb][NFCI] Apply IndexEntry to DWARFUnitHeader outside of extraction

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 =

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

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++ -*-===// //

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

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)

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

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

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-commits] [PATCH] D151588: Factor out xcrun (NFC)

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)

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-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

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,

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

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

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

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

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 =

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

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-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

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 =

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

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 _field_info, uint64_t

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

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

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

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/

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

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",

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

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

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

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

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

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

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

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/

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

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

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

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] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

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.

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] [PATCH] D148662: [lldb] Make the libcxx unique_ptr prettyprinter support custom deleters.

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

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

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/

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

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/

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

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

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

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

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

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

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

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

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-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

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

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

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

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

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

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

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/

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

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

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

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

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-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

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

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

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

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

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

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())); +

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

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:

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

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())); +

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

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())); +

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

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 =

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

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,

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

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

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

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)

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

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.

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

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

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

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

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

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

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.

  1   2   3   4   5   6   7   8   9   10   >