[Lldb-commits] [PATCH] D131110: [lldb] Make LLDB resilient against failing dyld introspection APIs

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! Evidently I had trouble following the control flow in the earlier revision ;-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131110/new/ https://reviews.llvm.org/D131110 _

[Lldb-commits] [PATCH] D131332: [lldb][unittests] Add more test cases to CPlusPlusNameParser unit-tests

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. More tests are always welcome! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131332/new/ https://reviews.llvm.org/D131332 ___

[Lldb-commits] [PATCH] D131335: WIP: [lldb] abi_tag support 3/3 - Use mangle tree API to determine approximate mangled matches

aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:607 + std::vector param_matches; + for (size_t i = 0; i < alternates.size(); i++) { +ConstString alternate_mangled_name = alternates[i]; Could this be `f

[Lldb-commits] [PATCH] D131333: [lldb] abi_tag support 2/3 - Make FindBestAlternateFunctionMangledName local to the C++ language plugin

aprantl added inline comments. Comment at: lldb/include/lldb/Target/Language.h:317 + /// function names. + /// + /// \param[in] mangled_names List of mangled names to generate JDevlieghere wrote: > I was wondering the same. I think it's wor

[Lldb-commits] [PATCH] D131335: WIP: [lldb] abi_tag support 3/3 - Use mangle tree API to determine approximate mangled matches

aprantl added a comment. In D131335#3722595 , @Michael137 wrote: > As @labath mentioned, we do force clang to preserve the linkage name via > `asm()`, but only for class member functions. This was added in > `675767a5910d2ec77ef8b51c78fe312cf9022896` (

[Lldb-commits] [PATCH] D131974: [lldb][ClangExpression] Add asm() label to all FunctionDecls we create from DWARF

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! This looks like a fine incremental improvement. But we should aim at eventually getting rid of these XFAILs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[Lldb-commits] [PATCH] D132004: [lldb][Tests] Skip static-only tests in TestConstStaticIntegralMember.py for dsym variant

aprantl added a comment. This might be a bit aggressive. If it's easy we could add a `if clang > 14` condition before those last two tests so we don't loose coverage? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132004/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

aprantl added a comment. In D131758#3735621 , @Michael137 wrote: > This seems to cause all API tests to time out. > > See LLDB Incremental buildbot: > https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/ > > Can we reve

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

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

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:338-339 +if args.custom_libcpp: +os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp + JDevlieghere wrote: > Please don't rely on environment variables to pass arg

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:338-339 +if args.custom_libcpp: +os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp + JDevlieghere wrote: > aprantl wrote: > > JDevlieghere wrote: > > > Please do

[Lldb-commits] [PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h:1 +//===-- Coroutines.h ---*- C++//-*-===// +// typo: `-*- C++ -*-` Comment at: lldb/source/Plugins/Language

[Lldb-commits] [PATCH] D132598: [lldb] Add more dylib paths for exception breakpoints

aprantl added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp:457 +filter_modules.EmplaceBack("libc++abi.1.0.dylib"); +filter_modules.EmplaceBack("libc++abi.1.dylib"); } Should these

[Lldb-commits] [PATCH] D132598: [lldb] Add more dylib paths for exception breakpoints

aprantl added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp:457 +filter_modules.EmplaceBack("libc++abi.1.0.dylib"); +filter_modules.EmplaceBack("libc++abi.1.dylib"); } fdeazeve wrot

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

[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, I

[Lldb-commits] [PATCH] D122347: Expose GetAddressingBits() in the Process API

aprantl created this revision. aprantl added reviewers: JDevlieghere, jasonmolenda. Herald added a project: All. aprantl requested review of this revision. See https://github.com/apple/llvm-project/pull/4110 for motivation and context. https://reviews.llvm.org/D122347 Files: lldb/include/lldb

[Lldb-commits] [PATCH] D122347: Expose GetAddressingBits() in the Process API

This revision was automatically updated to reflect the committed changes. Closed by commit rG7504dd5e00f5: Expose GetAddressingBits() in the Process API. (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[Lldb-commits] [PATCH] D122347: Expose GetAddressingBits() in the Process API

aprantl added a comment. This turned out to be unnecessary, because Process already exposes this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122347/new/ https://reviews.llvm.org/D122347 ___ lldb-commi

[Lldb-commits] [PATCH] D122041: [llvm][utils] Fix llvm::Optional summary provider

aprantl added a comment. We don't at the moment. We could add a test to the LLDB testsuite that compiles a test program against Support. There's some CMake trickery necessary for this, which is probably why we've shied away from it thus far. Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D122041: [llvm][utils] Fix llvm::Optional summary provider

aprantl added a comment. In D122041#3405821 , @kastiglione wrote: >> There's some CMake trickery > > Can you shed some more light on this? Am I understanding right: these > formatters are llvm, but the test would be in lldb? It seems weird that > somet

[Lldb-commits] [PATCH] D122373: [NFC] Be more lazy about looking up simulator SDKs on macs

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Fair word of warning — i've broken more than one LLDB build in the past while messing with this code :-) So you're basically sinking the calls to GetXcodeSDKPath into PlatformAppleSimulator

[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

aprantl added inline comments. Comment at: lldb/include/lldb/Target/Platform.h:102 + static lldb::PlatformSP + GetPlatformForArchitectures(std::vector archs, Would you mind adding a doxygen comment for this? I'm asking because the semantics are not clear to

[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

aprantl added inline comments. Comment at: lldb/source/Target/Platform.cpp:1228 + platforms.erase(std::unique(platforms.begin(), platforms.end()), + platforms.end()); +} aprantl wrote: > would std::unique be more idiomatic? Not sure why I didn't

[Lldb-commits] [PATCH] D122912: Simplify ArchSpec::IsFullySpecifiedTriple() (NFC)

aprantl created this revision. aprantl added reviewers: jasonmolenda, JDevlieghere. Herald added a project: All. aprantl requested review of this revision. I found this function somewhat hard to read and removed a few entirely redundant checks and converted it to early exits. https://reviews.ll

[Lldb-commits] [PATCH] D122912: Simplify ArchSpec::IsFullySpecifiedTriple() (NFC)

aprantl added inline comments. Comment at: lldb/source/Utility/ArchSpec.cpp:1399 bool ArchSpec::IsFullySpecifiedTriple() const { - const auto &user_specified_triple = GetTriple(); - - bool user_triple_fully_specified = false; - - if ((user_specified_triple.getOS() != llvm::Tr

[Lldb-commits] [PATCH] D122912: Simplify ArchSpec::IsFullySpecifiedTriple() (NFC)

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG942c21ed23dc: Simplify ArchSpec::IsFullySpecifiedTriple() (NFC) (authored by aprantl). Herald added a project: LLDB. Repo

[Lldb-commits] [PATCH] D122946: Prevent HostInfoBase::GetAugmentedArchSpec() from attaching "unknown" environments

aprantl created this revision. aprantl added reviewers: JDevlieghere, jasonmolenda. Herald added a project: All. aprantl requested review of this revision. Messy stuff. Environments are optional and a missing environment is distinct from the default "unknown" environment enumerator. Also the test

[Lldb-commits] [PATCH] D122946: Prevent HostInfoBase::GetAugmentedArchSpec() from attaching "unknown" environments

This revision was automatically updated to reflect the committed changes. Closed by commit rGcf3e4011b57b: Prevent GetAugmentedArchSpec() from attaching "unknown" environments (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[Lldb-commits] [PATCH] D124568: [lldb] Fix escaping when launching in terminal with AppleScript

aprantl added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:222 if (working_dir) -command.Printf(" --working-dir '%s'", working_dir.GetCString()); +command.Printf(" --working-dir \\\"%s\\\"", working_dir.GetCString()); else { S

[Lldb-commits] [PATCH] D124568: [lldb] Fix escaping when launching in terminal with AppleScript

aprantl added a comment. I have some trivial suggestions inside, but otherwise, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124568/new/ https://reviews.llvm.org/D124568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https

[Lldb-commits] [PATCH] D123954: [lldb] Add setting for max depth of value object printing (NFC)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Few nitpicks inside otherwise I'm happy. Comment at: lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h:50 uint32_t max_depth; + bool max_depth_is_default;

[Lldb-commits] [PATCH] D124872: [lldb] Add a function to check if lldb is running in an interactive session

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thank you! This is equally useful and untestable :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124872/new/ https://reviews.llvm.org/D124872

[Lldb-commits] [PATCH] D124872: [lldb] Add a function to check if lldb is running in an interactive session

aprantl added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:411 +bool Host::IsInteractiveSession() { +#if !TARGET_OS_OSX Wait. I think this is a misnomer. An lldb session over SSH is interactive, but not graphical. Should this be called

[Lldb-commits] [PATCH] D124872: [lldb] Add a function to check if lldb is running in an interactive session

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. One last request: Could you add a Doxygen comment? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124872/new/ https://reviews.llvm.org/D124872 __

[Lldb-commits] [PATCH] D124872: [lldb] Add a function to check if lldb is running in an interactive session

aprantl added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:411 +bool Host::IsInteractiveSession() { +#if !TARGET_OS_OSX mib wrote: > mib wrote: > > aprantl wrote: > > > Wait. I think this is a misnomer. An lldb session over SSH is > > >

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

aprantl created this revision. aprantl added reviewers: ldionne, shafik, jingham. aprantl added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. aprantl requested review of this revision. https://reviews.llvm.org/D125496 changed the layout of std::string witho

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

aprantl added a comment. I'm going to land this quickly to get the bots going again, but a thorough review would still be very much appreciated! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126080/new/ https://reviews.llvm.org/D126080 ___ l

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGf4570ce442b4: Adapt C++ std::string dataformatter for D125496 (authored by aprantl). Repository: rG LLVM Github Monorep

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

aprantl added a comment. That seemed to have done the trick. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126080/new/ https://reviews.llvm.org/D126080 ___ lldb-commits mailing list lldb-commits@lists.ll

[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

aprantl added a comment. > How do we usually add tests for STL data structures? I guess that matrix bot sort of covers this. https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-matrix/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126080/new

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

aprantl accepted this revision. aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) +type = form_value; Could you add a com

[Lldb-commits] [PATCH] D127437: [LLDB][Docs] Fix formatting of example code-block

This revision was automatically updated to reflect the committed changes. Closed by commit rGd36757b511ea: [LLDB][Docs] Fix formatting of example code-block (authored by aprantl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127437/new/ https://rev

[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

aprantl added a comment. Thanks, this looks really good! I have a couple of small comments inline. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.h:77 +// libc++ std::span<> +bool LibcxxSpanSummaryProvider(ValueObject &valobj, Stream &stream, `///

[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

aprantl added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py:23 + '[4] = 12345', + '}']) + Something you could consid

[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

This revision was automatically updated to reflect the committed changes. Closed by commit rGea9ff9fac3a6: [LLDB][formatters] Add formatter for libc++'s std::span (authored by aprantl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127481/new/ https

[Lldb-commits] [PATCH] D127605: [lldb] Support non-pointer implicit this/self in GetValueForVariableExpressionPath

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This is good, but it also illustrates how the strings "->" and ".'" should actually come from the typesystem and not be hardcoded. We're just lucky that all languages have a "." operator.

[Lldb-commits] [PATCH] D127605: [lldb] Support non-pointer implicit this/self in GetValueForVariableExpressionPath

aprantl added a comment. More elegant would be to just add an API to TypeSystem to get the operator to access ivars. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127605/new/ https://reviews.llvm.org/D127605 __

[Lldb-commits] [PATCH] D128063: [LLDB][ExpressionParser] Fix indices inside format-strings passed to LLDB_LOG

This revision was automatically updated to reflect the committed changes. Closed by commit rGf000de8760c1: [LLDB][ExpressionParser] Fix indices inside format-strings passed to LLDB_LOG (authored by aprantl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[Lldb-commits] [PATCH] D128306: [lldb] Instantiate lazily named classes on macOS Ventura.

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

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

aprantl added inline comments. Comment at: lldb/source/Target/TargetProperties.td:57 " C++ standard library types.">; + def DynamicClassInfoHelper: Property<"dynamic-class-info-helper", "Enum">, +DefaultEnumValue<"eDynamicClassInfoHelperAuto">, Sho

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Patch looks fine. Wording suggestion for the user-visible parts inside. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128312/new/ https://reviews.llvm.org/D128312 _

[Lldb-commits] [PATCH] D128377: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I'm curious now why there is both `count` and `max_class_infos` and if the second is workaround for this bug? Anyway, this *looks* plausible! CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D128378: [lldb] Return the correct utility function in AppleObjCRuntimeV2

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Oh boy, these are scary. Should LLDB fail harder when the utility expression fails, so we can detect these earlier? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128378/new/ https:

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

aprantl added inline comments. Comment at: lldb/include/lldb/Utility/Log.h:98 +class SystemLogHandler : public LogHandler { +public: Doxygen comment? Comment at: lldb/source/Utility/Log.cpp:406 + if (__builtin_available(macos 10.12, iOS 10,

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

aprantl accepted this revision. aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/Type.h:101 + /// Users will need to call one of the SetMatchContext() functions prior to + /// doing name lookups. + TypeQuery() = delete; this comment is out o

[Lldb-commits] [PATCH] D140030: [lldb][TypeSystemClang][NFC] Make TemplateParameterInfos members private

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.cpp:2067 +if (name && !name[0]) + name = nullptr; I wish we had

[Lldb-commits] [PATCH] D140067: Fix an ASAN bug I introduced in debugserver, accessing off the end of an array intentionally

aprantl added inline comments. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:2026-2037 +if (reg == gpr_pc) + value->value.uint64 = clear_pac_bits( + reinterpret_cast(m_state.context.gpr.__opaque_pc)); +else if (

[Lldb-commits] [PATCH] D140051: [lldb] Add LTO dependency to lldb test suite

aprantl added a comment. @DavidSpickett Is it possible to configure LLVM to not build with libLTO? Which CMake flag controls this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140051/new/ https://reviews.llvm.org/D140051

[Lldb-commits] [PATCH] D140056: [lldb] Report clang module build remarks

aprantl added a comment. Nice! I have a suggestion to speed up the test and make it a little more robust. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:82 Log *m_log; + std::unique_ptr m_current_progress_up; + std::vector m_module_build

[Lldb-commits] [PATCH] D140056: [lldb] Report clang module build remarks

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I like the new test! LGTM if @mib is happy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140056/new/ https://reviews.llvm.org/D140056 __

[Lldb-commits] [PATCH] D140051: [lldb] Add LTO dependency to lldb test suite

aprantl added a comment. > Well that was my confusion, no there isn't an option. So how does one end up > with a build that doesn't include it. Perhaps a standalone build of lldb, > built with a prebuilt llvm that didn't package libLTO? This is just about adding a dependency between lldb-test-d

[Lldb-commits] [PATCH] D137838: [Support] Move TargetParsers to new component

aprantl added a comment. Can you please update `llvm/include/llvm/module.modulemap` for this change or revert the patch? This is breaking all bots that build with `-DLLVM_ENABLE_MODULES=On`. For example: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/lastFailedBuild/consoleFull#1110

[Lldb-commits] [PATCH] D137838: [Support] Move TargetParsers to new component

aprantl added a comment. I think I fixed it in a685bb8e333e , but please take another look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D1

[Lldb-commits] [PATCH] D137838: [Support] Move TargetParsers to new component

aprantl added a comment. Sorry, I pasted in the wrong hash: 6bdf378dcd349d97152846bb687c1d1de511d138 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.ll

[Lldb-commits] [PATCH] D141087: Revert an unintentional API ABI break

This revision was automatically updated to reflect the committed changes. Closed by commit rG69f2b5fcf1be: Revert an unintentional API ABI break (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] D141100: Return a shared_ptr from ScratchTypeSystemClang::GetForTarget()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf8d7ab8cf8e8: Return a shared_ptr from ScratchTypeSystemClang::GetForTarget() (authored by aprantl). Herald added a subscriber: lldb-commits. Change

[Lldb-commits] [PATCH] D141539: [lldb][test] Replace use of p with expression (NFC)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I think it makes sense to have one test that tests the `p` command alias and its parameter handling and have all other tests be explicit about whether they test the expression evaluator or t

[Lldb-commits] [PATCH] D142034: [lldb][Language] List supported languages in expr error text

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nice. IIUC, this will query every already registered plugin, so it should also work without modification for e.g., LLDB with Swift support. Repository: rG LLVM Github Monorepo CHANGES SI

[Lldb-commits] [PATCH] D141539: [lldb][test] Replace use of p with expression (NFC)

aprantl added inline comments. Comment at: lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py:74 +self.expect("expression/d ((int*)&first)[0]", substrs=['= 5']) +self.expect("expression/d ((int*)&second)[0]", substrs=['= 6']) self.assert

[Lldb-commits] [PATCH] D142150: [lldb] Remove timer from SBModule copy ctor

aprantl added a comment. What about adding a `trivial` parameter to the macro that has the effect of skipping it in instrumentation if the instrumentation is the costly signpost mechanism? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142150/new/

[Lldb-commits] [PATCH] D141828: [lldb] Add support for DW_AT_default_value in template params

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2057 +case DW_AT_default_value: + if (attributes.ExtractFormValueAtIndex(i, form_value)) { +is_default_template_arg = form_value.Boolean(); ---

[Lldb-commits] [PATCH] D142683: Manual DWARF index: don't skip over -gmodules debug info

aprantl created this revision. aprantl added reviewers: clayborg, JDevlieghere. Herald added a subscriber: arphaman. Herald added a project: All. aprantl requested review of this revision. This fixes a regression introduced by https://reviews.llvm.org/D131437. The intention of the patch was to av

[Lldb-commits] [PATCH] D141828: [lldb] Add support for DW_AT_default_value in template params

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. LGTM! (I think you some superfluous #includes now) Comment at: lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp:21 +#include +#include + I th

[Lldb-commits] [PATCH] D142683: Manual DWARF index: don't skip over -gmodules debug info

aprantl updated this revision to Diff 492782. aprantl added a comment. I added an unambiguous check for DWARF5+. The remaining incorrectly handled case is a DWARF4 + GNU fission extension where the .dwo file has been deleted. Is there any harm in indexing the skeleton in that case? It shouldn't

[Lldb-commits] [PATCH] D142683: Manual DWARF index: don't skip over -gmodules debug info

aprantl added a comment. In D142683#4086026 , @aprantl wrote: > It shouldn't be able to interfere with the declarations in the (missing) .dwo > file in that case, right? Maybe I should have bothered reading your comment in the code. That's exactly the

[Lldb-commits] [PATCH] D142683: Manual DWARF index: don't skip over -gmodules debug info

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:180-182 +// The unit has a dwo_id, but this isn't a .dwo skeleton unit, so +// the assumption is that this is a file produced by -gmodules and +// that we want to inde

[Lldb-commits] [PATCH] D142683: Manual DWARF index: don't skip over -gmodules debug info

aprantl updated this revision to Diff 492852. aprantl added a comment. Added another heuristic to distinguish gmodules and fission. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142683/new/ https://reviews.llvm.org/D142683 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h lld

[Lldb-commits] [PATCH] D142683: Manual DWARF index: don't skip over -gmodules debug info

This revision was automatically updated to reflect the committed changes. Closed by commit rG9000a36f5cbd: Manual DWARF index: don't skip over -gmodules debug info (authored by aprantl). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D142683?vs=492852&id=492939#

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

aprantl added inline comments. Comment at: lldb/include/lldb/lldb-enumerations.h:492 + eLanguageTypeAda2005 = 0x002e, + eLanguageTypeAda2012 = 0x002f, + Would it make sense to generate this list from the macros in `llvm/include/llvm/BinaryFormat/Dwarf.def` wit

[Lldb-commits] [PATCH] D143068: [lldb][SymbolFileDWARF] Support by-name lookup of global variables in inline namespaces

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2227 +(actual_parent_decl_ctx != parent_decl_ctx && + !parent_decl_ctx.IsContainedInLookup(actual_parent_decl_ctx))) return true; ---

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

aprantl added inline comments. Comment at: lldb/include/lldb/lldb-enumerations.h:492 + eLanguageTypeAda2005 = 0x002e, + eLanguageTypeAda2012 = 0x002f, + Michael137 wrote: > aprantl wrote: > > Would it make sense to generate this list from the macros in > > `ll

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

aprantl added inline comments. Comment at: lldb/include/lldb/lldb-enumerations.h:492 + eLanguageTypeAda2005 = 0x002e, + eLanguageTypeAda2012 = 0x002f, + aprantl wrote: > Michael137 wrote: > > aprantl wrote: > > > Would it make sense to generate this list from t

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

aprantl added inline comments. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:548 +[[fallthrough]]; case lldb::eLanguageTypeC_plus_plus: case lldb::eLanguageTypeC_plus_plus_11: Why no case C++17? Com

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

aprantl added a comment. This change makes me a little nervous, but mostly because I don't quite understand the design here. So ASTImporter does not have a check whether src context and dst context are identical? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D143127: [LLDB] Fix assertion failure by removing `CopyType` in `std::coroutine_handle` pretty printer

aprantl accepted this revision. aprantl added a comment. > Well.. I don't believe we have any ASTImporter calls in any of the other > pretty printers, so I think this change is good. And if it causes some other > issues, then maybe we need to look at solving them in some other way... Okay I gue

[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

aprantl added a comment. SGTM. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3480 + clang::NamespaceDecl *namespace_decl = + static_cast(m_die_to_decl_ctx[die.GetDIE()]); + if (namespace_decl) The common pattern in LLVM (and th

[Lldb-commits] [PATCH] D143501: [WIP][clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

aprantl added a comment. Nice! Does `expr -- std::basic_string s` still work after this change? Not that anyone would want to type this over `std::string` ... Comment at: clang/test/CodeGen/debug-info-preferred-names.cpp:1 +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limite

[Lldb-commits] [PATCH] D143652: [lldb][DWARFASTParserClang] Attach linkage name to ctors/dtors if missing

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Nice! Comment at: lldb/test/API/lang/cpp/external_ctor_dtor_lookup/TestExternalCtorDtorLookup.py:2 +""" +Test that we can constructors/destructors +without a linkage name

[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

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.cpp:3480 + if (auto it = m_die_to_decl_ctx.find(die.GetDIE()); + it != m_die_to_decl_ctx.end(

[Lldb-commits] [PATCH] D143772: Adapt TestCustomShell and TestMultipleDebuggers to run under ASAN

aprantl added a comment. Generally LGTM, with one possible improvement inline! Comment at: lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py:20 + # We need this in order to run under ASAN, in case only LLDB is ASANified. + 'ASAN_OPTIONS':'

[Lldb-commits] [PATCH] D143772: Adapt TestCustomShell and TestMultipleDebuggers to run under ASAN

aprantl added a comment. > Why are only these two tests affected? Should this be something we set > globally for all the tests? The API tests already have support for forwarding > ASAN_OPTIONS and lit has a similar concept. We do, but only for dotest tests, and our dotest test launches another

[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

aprantl accepted this revision. aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:358 case DW_TAG_namespace: +case DW_TAG_imported_declaration: if (name) SGTM. Repository: rG LLVM Github Monor

[Lldb-commits] [PATCH] D143842: [lldb][test] Add check for Xcode binutils version to test-runner

aprantl added a comment. Should we put something like #if DARWIN STRIP=$(shell xcrun -find strip) #else STRIP=strip #fi in Makefile.rules instead? Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:889 +toolchain. Please make sure the Xcode tools are before a

[Lldb-commits] [PATCH] D143501: [clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

aprantl added a comment. > Alternatively, I suppose, the DWARF emission could just look at the preferred > name and use that as the DW_AT_type in all cases anyway? Avoids needing a new > attribute, etc, though would be a bit quirky in its own way. Am I understanding you correctly that you sugge

[Lldb-commits] [PATCH] D144114: [lldb] Add expression command options in dwim-print

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Conceptually, I think this looks good to me. Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:68 // First, try `expr` as the name of a frame variable. - if (

[Lldb-commits] [PATCH] D144114: [lldb] Add expression command options in dwim-print

aprantl accepted this revision. aprantl added a comment. Yeah, this is better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144114/new/ https://reviews.llvm.org/D144114 ___ lldb-commits mailing list lld

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

aprantl added inline comments. Comment at: lldb/source/Commands/CommandObjectExpression.cpp:149 + case 'C': { +// 'C' for "caching", since both 'P' and 'p' for persist are taken. Both 'R' Do we expect people to actually use this flag or to just use an ali

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

aprantl added a comment. Did you forget to git-add the test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144230/new/ https://reviews.llvm.org/D144230 ___ lldb-commits mailing list lldb-commits@lists.ll

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

aprantl added inline comments. Comment at: lldb/source/Commands/Options.td:389 "not supported by the interpreter (defaults to true).">; + def persistent_result : Option<"persistent-result", "\\x01">, Groups<[1,2]>, +Arg<"Boolean">, Is that the approved

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