[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

aprantl added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:971 + "need module to resolve file address for %s", dw_op_type); +return {}; + } clayborg wrote: > Do we prefer "return {}" over "return llvm::None"? Personally,

[Lldb-commits] [PATCH] D121831: Modifying expression code in MakeLoadImageUtilityFunction to be more consistent

aprantl added a comment. Did you check if we have other helper expressions with the same problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121831/new/ https://reviews.llvm.org/D121831 ___

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! Comment at: lldb/source/Expression/DWARFExpression.cpp:962 +/// check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a +/// success if

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

aprantl added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s:1 +# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s +# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s Out of curiosity:

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

aprantl added a comment. This is nice, I have a few mostly stylistic comments inline. Comment at: lldb/source/Expression/DWARFExpression.cpp:947 +llvm::Optional +ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp, Maybe add a

[Lldb-commits] [PATCH] D121062: [lldb] Add a setting to change the progress ANSI escape codes

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Core/CoreProperties.td:141 +DefaultStringValue<"${ansi.faint}">, +Desc<"When displaying progress in a color-enabled (i.e. ANSI)

[Lldb-commits] [PATCH] D121131: [lldb] Support "bright" ANSI colors

aprantl added inline comments. Comment at: lldb/include/lldb/Utility/AnsiTerminal.h:12 + #define ANSI_FG_COLOR_BLACK 30 #define ANSI_FG_COLOR_RED 31 Nice! Should we convert this to enums? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

aprantl added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1757 + File = GetOutputFile(); + if (!output.GetIsTerminalWithColors() || !GetShowProgress()) +return; And withColors also implies that it's an interactive TTY? CHANGES SINCE LAST

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

aprantl added a comment. This is pretty (and) awesome. Comment at: lldb/source/Core/Debugger.cpp:1670 +if (event_type & Debugger::eBroadcastBitProgress) + HandleProgressEvent(event_sp); } side note: this function could

[Lldb-commits] [PATCH] D120578: [lldb] Implement ObjectFile::GetCStrFromSection

aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/ObjectFile.h:676 + const char *GetCStrFromSection(Section *section, + lldb::offset_t section_offset) const; Can you add a doxygen comment for the method?

[Lldb-commits] [PATCH] D119915: Replace use of double underscore in identifiers

aprantl added a comment. Herald added a subscriber: JDevlieghere. Nice. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:261 if (m_pair_ptr) { - auto __i_(valobj_sp->GetChildMemberWithName(g___i_, true)); + auto

[Lldb-commits] [PATCH] D120105: Remove recursive include of GDBRemoteCommunicationServerCommon.h

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

[Lldb-commits] [PATCH] D119167: [lldb/test] Remove sleeps from some lldb-server tests

aprantl added a comment. This looks amazing! Thanks, Pavel. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119167/new/ https://reviews.llvm.org/D119167 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4647 +const bool UsePointerValue) { +

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4647 +const bool UsePointerValue) { + assert(CGM.getCodeGenOpts().hasReducedDebugInfo()); + assert(!LexicalBlockStack.empty() && "Region stack mismatch,

[Lldb-commits] [PATCH] D119178: Add support for generating debug-info for structured bindings of structs and arrays

aprantl added a comment. I think this looks pretty good! I have a few questions inside. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4667 + + SmallVector Expr; + AppendAddressSpaceXDeref(AddressSpace, Expr); 13 seems to be unnecessarily high. Shouldn't 1 be

[Lldb-commits] [PATCH] D118395: Disable TestLldbGdbServer on Dwarf2 and clang versions below 14

aprantl added a comment. Done in zorg and reverted this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118395/new/ https://reviews.llvm.org/D118395 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D118494: [lldb] Observe SG_READ_ONLY flag in MachO binaries

aprantl added inline comments. Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1436 result |= ePermissionsReadable; - if (seg_cmd.initprot & VM_PROT_WRITE) + if ((seg_cmd.initprot & VM_PROT_WRITE) && !(seg_cmd.flags & SG_READ_ONLY)) result |=

[Lldb-commits] [PATCH] D118395: Disable TestLldbGdbServer on Dwarf2 and clang versions below 14

aprantl added a comment. In D118395#3278431 , @labath wrote: > Also, if you're running these tests on some kind of a matrix bot, maybe you > can just skip all lldb-server tests completely (dotest.py --skip-categories > llgs). Given that they don't use

[Lldb-commits] [PATCH] D118265: [lldb] Make ReadCStringFromMemory default to read from the file-cache.

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. force_live_memory=false means use the file cache iff: - the address has a Section - the section is read-only Otherwise it reads from memory. Based on that this looks safe to me.

[Lldb-commits] [PATCH] D117632: [lldb] Instrument SB API with signposts

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Assuming UpdateBoundary is only called in the destructor this looks nice! Comment at: lldb/source/Utility/Instrumentation.cpp:21 +// Instrument SB API calls with

[Lldb-commits] [PATCH] D117632: Instrument SBAPI with scoped timers

aprantl added a comment. > I'm a little confused about what you want to achieve as well. With signposts you get a flamegraph of all nested signposted functions, which is really intuitive to look at. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117632/new/

[Lldb-commits] [PATCH] D117632: Instrument SBAPI with scoped timers

aprantl abandoned this revision. aprantl added a comment. In D117632#3255158 , @JDevlieghere wrote: > Do you actually care about the timers or is this really about getting these > methods instrumented with signposts on our platform? If it's the latter,

[Lldb-commits] [PATCH] D117623: [lldb] Print an error message when we're reading libobjc.A.dylib from memory

aprantl added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2379 +stream->Printf("warning: libobjc.A.dylib is being read from process " + "memory. This indicates that we could not %s. This

[Lldb-commits] [PATCH] D117623: [lldb] Print an error message when we're reading libobjc.A.dylib from memory

aprantl added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2377 +stream->Printf( +"warning: libobjc.A.dylib is being read from memory instead of the %s" +"shared cache. This will likely reduce

[Lldb-commits] [PATCH] D116847: [lldb] Remove most of the reproducer instrumentation

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. With regret, I accept this patch. Too bad couldn't make it work! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116847/new/ https://reviews.llvm.org/D116847

[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

aprantl added a comment. > it can vary on some platforms it is signed char but on others it is bool. For those interested, Objective-C `BOOL` is a signed char on macOS and 32-bit iOS and `bool` on 64-bit iOS and derived platforms (watchOS & tvOS). Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

aprantl added inline comments. Comment at: lldb/source/Commands/CommandObjectMemory.cpp:1695 + section_sp = section_sp->GetParent(); +section_name = section_sp->GetName(); + } Why did `section_sp->GetName();` return the Segment

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

aprantl added inline comments. Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:76 + +self.runCmd("run", RUN_SUCCEEDED) + Doesn't `lldbutil.run_break_set_by_symbol()` do the "file" and "run" already and this run command runs

[Lldb-commits] [PATCH] D116461: [lldb] Remove ProcessStructReader from NSStringSummaryProvider (NFC)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This doesn't look any less readable than the code it replaces and is definitely faster. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D116697: [lldb] Create a property to store the REPL language

aprantl added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1777 if (auto single_lang = repl_languages.GetSingularLanguage()) { language = *single_lang; As you said in the description, this is effectively dead code. Is this to be

[Lldb-commits] [PATCH] D115461: [lldb/Target] Refine source display warning for artificial locations (NFC)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks, I think this is much easier to understand! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115461/new/ https://reviews.llvm.org/D115461

[Lldb-commits] [PATCH] D115313: [lldb/Target] Slide source display for artificial locations at function start

aprantl added a comment. Thanks! I have some late wording suggestions. Comment at: lldb/source/Target/StackFrame.cpp:1904 + strm.Printf("Warning: the current PC is an artificial location " + "in function %s.", +

[Lldb-commits] [PATCH] D114742: [lldb] Search PrivateFrameworks when using an internal SDK

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:86 +private_frameworks = os.path.join(sdk_root,

[Lldb-commits] [PATCH] D111899: LLDB tests modification for hardware breakpoints

aprantl added a comment. FYI, this broke all the TestObjCNewSyntax* tests. I *think* I've fixed them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111899/new/ https://reviews.llvm.org/D111899 ___

[Lldb-commits] [PATCH] D113650: [lldb] fix -print-script-interpreter-info on windows

aprantl added a comment. This seems to have broken the green dragon incremental bot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/38387/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113650/new/ https://reviews.llvm.org/D113650

[Lldb-commits] [PATCH] D113445: Support looking up absolute symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rGc9881c7d99c6: Support looking up absolute symbols (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D113445: Support looking up absolute symbols

aprantl created this revision. aprantl added reviewers: jasonmolenda, jingham, JDevlieghere. aprantl requested review of this revision. The Swift stdlib uses absolute symbols in the dylib to communicate feature flags to the process. LLDB's expression evaluator needs to be able to find them.

[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Excellent, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112945/new/ https://reviews.llvm.org/D112945 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D112439: normalize file path when searching the source map

aprantl accepted this revision. aprantl added a comment. In D112439#3098307 , @xujuntwt95329 wrote: > Hi, @aprantl > > I've submitted a NFC patch here: > https://reviews.llvm.org/D112863 > > Seems that patch can't build by CI because it is based on this

[Lldb-commits] [PATCH] D112863: [lldb][NFC] avoid unnecessary roundtrips between different string types

aprantl accepted this revision. aprantl added a comment. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112863/new/ https://reviews.llvm.org/D112863 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

aprantl added inline comments. Comment at: lldb/test/Shell/ObjC/tagged_pointer_info.test:3 + +RUN: %clang_host -g -framework Foundation -o %t.out %S/Inputs/test.m + personal opinion without through reasoning behind it: This feels more like an API test to me.

[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`

aprantl added a comment. This is great, thanks! (one question inline) Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:1008 } - result.SetStatus(lldb::eReturnStatusSuccessFinishResult); - return true; + success

[Lldb-commits] [PATCH] D112439: normalize file path when searching the source map

aprantl added inline comments. Comment at: lldb/source/Target/PathMappingList.cpp:33 // nomalized path pairs to ensure things match up. ConstString NormalizePath(ConstString path) { // If we use "path" to construct a FileSpec, it will normalize the path for

[Lldb-commits] [PATCH] D112165: Cleanup a few more PR36048 skips

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112165/new/ https://reviews.llvm.org/D112165

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

aprantl added a comment. thanks! Comment at: lldb/packages/Python/lldbsuite/test/builders/darwin.py:62 +component = [arch, vendor, os + version] +if env: +components.append(env) So this tests for the empty string? CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D112676: [lldb] The os and version are not separate components in the triple

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Yup that's wrong. Personally I would only append env to the list if non-empty. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112676/new/ https://reviews.llvm.org/D112676

[Lldb-commits] [PATCH] D112662: [lldb] Fixup code addresses in the Objective-C language runtime

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This is all good of course, but I wonder if we should lift this into the type system and have functions returned a UnstrippedAddress type that you can only turn into an addr_t by calling

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

This revision was automatically updated to reflect the committed changes. Closed by commit rG4651576edd09: Recognize the Swift compiler in DW_AT_producer (authored by aprantl). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D111278?vs=377977=378001#toc

[Lldb-commits] [PATCH] D111339: [lldb] Parse and display reporting errors from JSON crashlogs

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

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl updated this revision to Diff 377977. aprantl added a comment. Const. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111278/new/ https://reviews.llvm.org/D111278 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:684 + llvm::SmallVector matches; + if (g_swiftlang_version_regex.Execute(producer, )) { + m_producer_version.tryParse(matches[1]); aprantl wrote: > JDevlieghere

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:684 + llvm::SmallVector matches; + if (g_swiftlang_version_regex.Execute(producer, )) { + m_producer_version.tryParse(matches[1]); JDevlieghere wrote: > For

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl updated this revision to Diff 377702. aprantl added a comment. Sort variables in a more logical order. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111278/new/ https://reviews.llvm.org/D111278 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

[Lldb-commits] [PATCH] D111278: Recognize the Swift compiler in DW_AT_producer

aprantl created this revision. aprantl added reviewers: JDevlieghere, kastiglione, jingham. aprantl requested review of this revision. This patch adds support for Swift compiler producer strings to DWARFUnit. https://reviews.llvm.org/D111278 Files:

[Lldb-commits] [PATCH] D111200: Use llvm::VersionTuple to store DWARF producer info (NFC)

This revision was automatically updated to reflect the committed changes. Closed by commit rG14aa3f3703b5: Use llvm::VersionTuple to store DWARF producer info (NFC) (authored by aprantl). Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D111200: Use llvm::VersionTuple to store DWARF producer info (NFC)

aprantl updated this revision to Diff 377403. aprantl added a comment. Fix typo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111200/new/ https://reviews.llvm.org/D111200 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h

[Lldb-commits] [PATCH] D111200: Use llvm::VersionTuple to store DWARF producer info (NFC)

aprantl updated this revision to Diff 377402. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111200/new/ https://reviews.llvm.org/D111200 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h Index:

[Lldb-commits] [PATCH] D111200: Use llvm::VersionTuple to store DWARF producer info (NFC)

aprantl created this revision. aprantl added reviewers: JDevlieghere, kastiglione, jingham. aprantl requested review of this revision. This has the nice side-effect that it can actually store the quadruple version numbers that Apple's tools are using nowadays. rdar://82982162

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

aprantl added a comment. In D110893#3035753 , @labath wrote: > Some general thoughts: > > - instead of throwing patches around in might be good to have a discussion > thread to determine all the requirements and figure out the general direction Got to

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Mechanically, this sgtm. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110893/new/ https://reviews.llvm.org/D110893 ___ lldb-commits

[Lldb-commits] [PATCH] D110013: [lldb][crashlog] Avoid specifying arch for image when a UUID is present

aprantl added a comment. Without having dug into this deeper, do you think not specifying the arch could cause any problems with universal binaries? I guess not, because we still have the UUID, and the UUID is per arch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D109004: [lldb] Catch all exceptions when checking if simulator exists

aprantl added a comment. What do you think about putting the ` json.loads(sim_devices_str)['devices']` into the decorator, so we know that xcodebuild has an SDK *and* the simulator has devices available? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D109263: [lldb] Update crashlog.py to accept multiple results from mdfind

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks, looks good! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109263/new/ https://reviews.llvm.org/D109263 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D108983: [lldb] Don't save empty expressions in the multiline editor history

aprantl added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108983/new/ https://reviews.llvm.org/D108983 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I think this is a great overall addition, might be good to have at least one other reviewer sign off though. (Summoning @jingham!) Should we add a python script to pretty-print a JSON log

[Lldb-commits] [PATCH] D101406: Rename human-readable name for DW_LANG_Mips_Assembler

aprantl added a comment. > It doesn't make sense to warn about that. :) Why not? LLDB doesn't have support for assembler as a source language, no assembler expression evaluator, etc.. I don't have much experience debugging assembler sources with LLDB, so I"m not sure about the expectations.

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

aprantl added a comment. FYI (you may have have already noticed) this causes some merge conflicts in swift-lldb. It would be great if we could work together to figure out how to best resolve them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

aprantl added inline comments. Comment at: lldb/test/API/lang/cpp/null_references/TestNullReferences.py:14 + +self.runCmd("continue") Why is the `continue` necessary and what is it actually that's triggering the crash? CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D108717: Fix Reference case for TypeSystemClang::GetChildCompilerTypeAtIndex(...) to avoid possible invalid cast

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. That looks obviously correct, thanks! Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6506 + CompilerType pointee_clang_type; + + if

[Lldb-commits] [PATCH] D108257: Add type to the output for FieldDecl when logging in ClangASTSource::layoutRecordType

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. If getType() doesn't trigger type completion then this LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108257/new/ https://reviews.llvm.org/D108257

[Lldb-commits] [PATCH] D101406: Rename human-readable name for DW_LANG_Mips_Assembler

aprantl added a comment. > This can't have been the intention of this commit? As far as I can see, there > *is* no plugin for plain assembler? This commit changes the warning from > This version of LLDB has no plugin for the mipsassem language. to > This version of LLDB has no plugin for the

[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

aprantl added a comment. More a comment than anything else: One thing I always wanted to explore was to implement LLDB's logging on Darwin on top of os_log (https://developer.apple.com/documentation/os/logging) which is also structured and also faster since it moves the the formatting out of

[Lldb-commits] [PATCH] D106171: [lldb] Avoid moving ThreadPlanSP from plans vector

aprantl added inline comments. Comment at: lldb/source/Target/ThreadPlanStack.cpp:145 - lldb::ThreadPlanSP plan_sp = std::move(m_plans.back()); + lldb::ThreadPlanSP plan_sp = m_plans.back(); + m_plans.pop_back(); Should we comment that we are intentionally

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +other_die = dcu->LookupAddress( +symtab->SymbolAtIndex(symbol_indexes[index])->GetFileAddress()); + } should there be an early exit

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added a comment. Could LLDB find the linkage name on the declaration, look that name up in the symbol table, and find the DW_TAG_subprogram DIE for the symbol's start address? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +if (!try_resolving_type) + return true; + aprantl wrote: > This block looks like it's more complicated than it needs to be. Could you > just say >

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added a comment. I think it would be best to split out the Clang change into a separately tested patch. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +if (!try_resolving_type) + return true; + This block looks like

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added a comment. I think it would be best to split out the Clang change into a separately tested patch. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1694 + // If the declared return type is "auto" we want the linkage name to go + // with the defintion. In

[Lldb-commits] [PATCH] D104407: Improve path remapping in cross-debugging scenarios

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG21e013303bb7: Improve path remapping in cross-debugging scenarios (authored by aprantl). Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D104724: Modernize Module::RemapFile to return an Optional

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa0e1b11fac7a: Modernize Module::RemapFile to return an Optional (NFC) (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM

[Lldb-commits] [PATCH] D104406: Express PathMappingList::FindFile() in terms of PathMappingList.h::RemapPath() (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG302b1b971809: Express PathMappingList::FindFile() in terms of PathMappingList::RemapPath() (authored by aprantl). Herald added a project: LLDB.

[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa346372200e7: Change PathMappingList::FindFile to return an optional result (NFC) (authored by aprantl). Herald added a project: LLDB. Changed

[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)

aprantl added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104405/new/ https://reviews.llvm.org/D104405 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104404: Change PathMappingList::RemapPath to return an optional result (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4cf7c6c6a44d: Change PathMappingList::RemapPath to return an optional result (NFC) (authored by aprantl). Herald added a project: LLDB. Changed

[Lldb-commits] [PATCH] D104407: Improve path remapping in cross-debugging scenarios

aprantl added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104407/new/ https://reviews.llvm.org/D104407 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104724: Modernize Module::RemapFile to return an Optional

aprantl added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104724/new/ https://reviews.llvm.org/D104724 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104406: Express PathMappingList::FindFile() in terms of PathMappingList.h::RemapPath() (NFC)

aprantl added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104406/new/ https://reviews.llvm.org/D104406 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

aprantl added inline comments. Comment at: lldb/source/Symbol/Symtab.cpp:642 + Symbol *symbol = FindSymbolByID(uid); + if (symbol == nullptr) +return 0; `if (!symbol)` ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D104404: Change PathMappingList::RemapPath to return an optional result (NFC)

aprantl added a comment. > if the return type of the new function is not too awful to type, maybe don't > use auto? It says it's returning a path, which is usually a string, not a > FileSpec... But that's more a matter of taste. I think that's generally advice. In this particular case, the

[Lldb-commits] [PATCH] D104404: Change PathMappingList::RemapPath to return an optional result (NFC)

aprantl added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104404/new/ https://reviews.llvm.org/D104404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104404: Change PathMappingList::RemapPath to return an optional result (NFC)

aprantl added a comment. Patch for `Module` is in https://reviews.llvm.org/D104724. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104404/new/ https://reviews.llvm.org/D104404 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)

aprantl updated this revision to Diff 353700. aprantl added a comment. Update to the correct patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104405/new/ https://reviews.llvm.org/D104405 Files: lldb/include/lldb/Target/PathMappingList.h lldb/source/Core/Module.cpp

[Lldb-commits] [PATCH] D104724: Modernize Module::RemapFile to return an Optional

aprantl created this revision. aprantl added reviewers: shafik, JDevlieghere. aprantl requested review of this revision. This addresses feedback raised in https://reviews.llvm.org/D104404. https://reviews.llvm.org/D104724 Files: lldb/include/lldb/Core/Module.h lldb/source/Core/Module.cpp

[Lldb-commits] [PATCH] D104404: Change PathMappingList::RemapPath to return an optional result (NFC)

aprantl added inline comments. Comment at: lldb/source/Core/Module.cpp:1604 bool Module::RemapSourceFile(llvm::StringRef path, std::string _path) const { shafik wrote: > Why not also have this return an `Optional`? That would be

[Lldb-commits] [PATCH] D104407: Improve path remapping in cross-debugging scenarios

aprantl created this revision. aprantl added reviewers: jingham, labath, shafik, JDevlieghere. aprantl requested review of this revision. This patch implements a slight improvement when debugging across platforms and remapping source paths that are in a non-native format. See the unit test for

[Lldb-commits] [PATCH] D104406: Express PathMappingList::FindFile() in terms of PathMappingList.h::RemapPath() (NFC)

aprantl created this revision. aprantl added reviewers: jingham, shafik, JDevlieghere, labath. aprantl requested review of this revision. This patch replaces the function body FindFile() with a call to RemapPath(), since the two functions implement the same functionality.

[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)

aprantl created this revision. aprantl added reviewers: JDevlieghere, shafik, jingham. aprantl requested review of this revision. This is an NFC modernization refactoring that replaces the combination of a bool return + reference argument, with an Optional return value.

[Lldb-commits] [PATCH] D104404: Change PathMappingList::RemapPath to return an optional result (NFC)

aprantl created this revision. aprantl added reviewers: JDevlieghere, shafik, jingham. aprantl requested review of this revision. This is an NFC modernization refactoring that replaces the combination of a bool return + reference argument, with an Optional return value.

[Lldb-commits] [PATCH] D103575: Allow signposts to take advantage of deferred string substitution

aprantl added a comment. I tried to work around it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103575/new/ https://reviews.llvm.org/D103575 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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