[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

tschuett added inline comments. Comment at: lldb/tools/debugserver/source/RNBRemote.cpp:6264 major_version, minor_version, patch_version); -if (platform) { +if (platform.has_value()) { os_handled = true;

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

tonkosi added a comment. Thanks for the fix Will! Just one question: cpp test file contains the expression `a[10].c` and the test complains about the expression path `a->c`. If I understand correctly, with the latest fix, that would become `a[10]->c`, which is not the original expression

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo added a comment. In D136572#3888177 , @mstorsjo wrote: > This broke builds where clang/lld/lldb are symlinked into `llvm/tools` > instead of specified via `LLVM_ENABLE_PROJECTS` - since > `${CMAKE_CURRENT_SOURCE_DIR}/../cmake` doesn't find

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mgorny added a comment. I've reverted it right now, and I'll update the patch later today. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136572/new/ https://reviews.llvm.org/D136572 ___ lldb-commits

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

labath added a comment. Yeah, something like that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136761/new/ https://reviews.llvm.org/D136761 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

labath added a comment. I can't say I fully understand all of this code, but I also don't know who would, so I guess I'll just say it "looks good" :) I am wondering about the testing situation though. If I understand correctly, you've run the test suite with hardcoded simplified names, and it

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo added a comment. Herald added subscribers: JDevlieghere, jdoerfert. This broke builds where clang/lld/lldb are symlinked into `llvm/tools` instead of specified via `LLVM_ENABLE_PROJECTS` - since `${CMAKE_CURRENT_SOURCE_DIR}/../cmake` doesn't find anything in that context. (The reason

[Lldb-commits] [PATCH] D136801: [intelpt] Update Python tests to account for new errrors

This revision was automatically updated to reflect the committed changes. Closed by commit rG17c65e51b916: [intelpt] Update Python tests to account for new errrors (authored by jj10306). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136801/new/

[Lldb-commits] [lldb] 17c65e5 - [intelpt] Update Python tests to account for new errrors

Author: Jakob Johnson Date: 2022-10-27T05:09:08-07:00 New Revision: 17c65e51b91620181662302e9abd7e9694154c5d URL: https://github.com/llvm/llvm-project/commit/17c65e51b91620181662302e9abd7e9694154c5d DIFF: https://github.com/llvm/llvm-project/commit/17c65e51b91620181662302e9abd7e9694154c5d.diff

[Lldb-commits] [PATCH] D136362: [LLDB][RISCV] Add RV64C instruction support for EmulateInstructionRISCV

Emmmer added a comment. In D136362#3887927 , @seehearfeel wrote: > After commit 05ae747a5353811f93f5814f24d2335e6229d78a > > ("[LLDB][RISCV] Add RV64C instruction support for

[Lldb-commits] [lldb] 88d7508 - Harmonize cmake_policy() across standalone builds of all projects

Author: Michał Górny Date: 2022-10-27T13:46:56+02:00 New Revision: 88d7508dc479210f07abccb17f0194b66264b125 URL: https://github.com/llvm/llvm-project/commit/88d7508dc479210f07abccb17f0194b66264b125 DIFF: https://github.com/llvm/llvm-project/commit/88d7508dc479210f07abccb17f0194b66264b125.diff

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG88d7508dc479: Harmonize cmake_policy() across standalone builds of all projects (authored by mgorny). Herald added projects: clang, LLDB, Flang.

[Lldb-commits] [PATCH] D136209: [LLDB][NativePDB] Fix parameter size for member functions LF_MFUNCTION

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D136209#3880121 , @zequanwu wrote: > In D136209#3878821 , @labath wrote: > >> So if I understand

[Lldb-commits] [lldb] 995d556 - [lldb][test] Remove empty setUp/tearDown methods (NFC)

Author: Dave Lee Date: 2022-10-27T08:41:00-07:00 New Revision: 995d556f427d2ac7d3d9f80041adc7c51e80308b URL: https://github.com/llvm/llvm-project/commit/995d556f427d2ac7d3d9f80041adc7c51e80308b DIFF: https://github.com/llvm/llvm-project/commit/995d556f427d2ac7d3d9f80041adc7c51e80308b.diff

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mgorny updated this revision to Diff 471173. mgorny added a comment. Move `LLVM_COMMON_CMAKE_UTILS` earlier and use it for CMakePolicy path. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136572/new/ https://reviews.llvm.org/D136572 Files: clang/CMakeLists.txt

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mgorny added a reviewer: mstorsjo. mgorny added a comment. @mstorsjo, could you check this version? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136572/new/ https://reviews.llvm.org/D136572 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D136890: [lldb-vscode] Don't call SBValue.GetError after generating a summary.

jgorbe created this revision. jgorbe added reviewers: yinghuitan, clayborg. jgorbe added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jgorbe requested review of this revision. In some occasions, SBValue::GetError can invalidate its cached `m_summary_str`

[Lldb-commits] [PATCH] D136890: [lldb-vscode] Don't call SBValue.GetError after generating a summary.

clayborg added inline comments. Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:143 +llvm::StringRef value = v.GetValue(); +llvm::StringRef summary = v.GetSummary(); +llvm::StringRef type_name = v.GetType().GetDisplayTypeName(); yinghuitan wrote: >

[Lldb-commits] [PATCH] D136900: Don't test dyld_process_create_for_current_task symbol before calling it in HostInfoMacOSX.mm

jasonmolenda created this revision. jasonmolenda added a reviewer: aprantl. jasonmolenda added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. In a patch Fred landed a

[Lldb-commits] [PATCH] D136900: Don't test dyld_process_create_for_current_task symbol before calling it in HostInfoMacOSX.mm

jasonmolenda added a comment. The patch only removes the tests for the deployment target (compile-time) and for the symbol to be defined (runtime); clang-format shifted everything around to make it look like the diff is larger. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D136900: Don't test dyld_process_create_for_current_task symbol before calling it in HostInfoMacOSX.mm

This revision was automatically updated to reflect the committed changes. Closed by commit rGbc196970b549: Remove compile-time and runtime checks for SPI in HostInfoMacOSX (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D136362: [LLDB][RISCV] Add RV64C instruction support for EmulateInstructionRISCV

seehearfeel added a comment. In D136362#3888328 , @Emmmer wrote: > In D136362#3887927 , @seehearfeel > wrote: > >> After commit 05ae747a5353811f93f5814f24d2335e6229d78a >>

[Lldb-commits] [PATCH] D136890: [lldb-vscode] Don't call SBValue.GetError after generating a summary.

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

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just a few nits and this is good to go Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:56 +formatted += text.substr(0, backslash_pos).str(); +//

[Lldb-commits] [PATCH] D136900: Don't test dyld_process_create_for_current_task symbol before calling it in HostInfoMacOSX.mm

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Two less levels of indentation 拾 LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136900/new/ https://reviews.llvm.org/D136900

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

aeubanks updated this revision to Diff 471330. aeubanks added a comment. fix nested types by introducing Type::GetBaseName() add expr eval tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134378/new/ https://reviews.llvm.org/D134378 Files:

[Lldb-commits] [lldb] bc19697 - Remove compile-time and runtime checks for SPI in HostInfoMacOSX

Author: Jason Molenda Date: 2022-10-27T16:57:48-07:00 New Revision: bc196970b549782bffde5fdbfa450c565af11fc1 URL: https://github.com/llvm/llvm-project/commit/bc196970b549782bffde5fdbfa450c565af11fc1 DIFF: https://github.com/llvm/llvm-project/commit/bc196970b549782bffde5fdbfa450c565af11fc1.diff

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

aeubanks added a comment. regarding the failure in TestCPPBreakpointLocations.py (added recently in https://reviews.llvm.org/D135921, seems like it's catching real issues with this patch), it looks like the manual dwarf index needs a similar fix to this In D134378#3888462

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

Michael137 added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:3831 + + clang::PrintingPolicy printing_policy(GetTypePrintingPolicy()); + printing_policy.SuppressScope = true; Could we just re-use `GetTypeName` and add

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

aeubanks updated this revision to Diff 471375. aeubanks marked an inline comment as done. aeubanks added a comment. combine GetBaseName into GetTypeName Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134378/new/ https://reviews.llvm.org/D134378

[Lldb-commits] [PATCH] D126014: Show error message for optimized variables

jgorbe added inline comments. Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:136-156 llvm::StringRef value = v.GetValue(); llvm::StringRef summary = v.GetSummary(); llvm::StringRef type_name = v.GetType().GetDisplayTypeName(); + lldb::SBError error = v.GetError();

[Lldb-commits] [PATCH] D136890: [lldb-vscode] Don't call SBValue.GetError after generating a summary.

yinghuitan added inline comments. Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:143 +llvm::StringRef value = v.GetValue(); +llvm::StringRef summary = v.GetSummary(); +llvm::StringRef type_name = v.GetType().GetDisplayTypeName(); Sorry for the late

[Lldb-commits] [lldb] 3b0f38b - [LLDB][NativePDB] Fix parameter size for member functions LF_MFUNCTION

Author: Zequan Wu Date: 2022-10-27T16:36:05-07:00 New Revision: 3b0f38bb5186cdda52ab38b13965678036aba22c URL: https://github.com/llvm/llvm-project/commit/3b0f38bb5186cdda52ab38b13965678036aba22c DIFF: https://github.com/llvm/llvm-project/commit/3b0f38bb5186cdda52ab38b13965678036aba22c.diff

[Lldb-commits] [PATCH] D136209: [LLDB][NativePDB] Fix parameter size for member functions LF_MFUNCTION

This revision was automatically updated to reflect the committed changes. Closed by commit rG3b0f38bb5186: [LLDB][NativePDB] Fix parameter size for member functions LF_MFUNCTION (authored by zequanwu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

clayborg added a comment. What happens when we have a log message error? I see we log to the console, but we should test this. Also, if there is an error, do we fail the breakpoint setting completely or do we just not set the log message? We need a test for these cases. Repository: rG LLVM

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

aeubanks added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:3809 +ConstString TypeSystemClang::GetBaseName(lldb::opaque_compiler_type_t type) { + if (!type) this was mostly copied from `GetTypeName` above, not sure if

[Lldb-commits] [lldb] 7d4d25c - [lldb-vscode] Don't call SBValue.GetError after generating a summary.

Author: Jorge Gorbe Moya Date: 2022-10-27T15:05:27-07:00 New Revision: 7d4d25c42a682ec5abcc008225268a6ce82673f3 URL: https://github.com/llvm/llvm-project/commit/7d4d25c42a682ec5abcc008225268a6ce82673f3 DIFF:

[Lldb-commits] [PATCH] D136890: [lldb-vscode] Don't call SBValue.GetError after generating a summary.

This revision was automatically updated to reflect the committed changes. Closed by commit rG7d4d25c42a68: [lldb-vscode] Dont call SBValue.GetError after generating a summary. (authored by jgorbe). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

yinghuitan updated this revision to Diff 471314. yinghuitan added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136697/new/ https://reviews.llvm.org/D136697 Files:

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

yinghuitan updated this revision to Diff 471318. yinghuitan added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136697/new/ https://reviews.llvm.org/D136697 Files:

[Lldb-commits] [PATCH] D136795: [LLDB] Mark placeholder modules in `target module list` output.

zequanwu updated this revision to Diff 471337. zequanwu added a comment. - Modify file spec of placeholder object file. - Change it to "(not loaded)" which is more obvious than "(placeholder)". - Use prefix so the file extension is preseved. Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Nice use of std::optional. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136719/new/ https://reviews.llvm.org/D136719

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

yinghuitan added inline comments. Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:29 +lldb::SBError BreakpointBase::AppendLogMessagePart(llvm::StringRef part, + bool is_expr) { + if (is_expr) {

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

MaskRay added a comment. It'll be good if we have a wiki describing how downstream users invoke cmake, so that large cmake refactoring can be verified beforehand. (like usage verification https://github.com/opencollab/llvm-toolchain-integration-test-suite, but for cmake invocations) CHANGES

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo accepted this revision. mstorsjo added a comment. In D136572#3888630 , @mgorny wrote: > @mstorsjo, could you check this version? LGTM, this seems to work for my usecase (with lld,lldb,clang enabled this way). Thanks for the quick revert and

[Lldb-commits] [lldb] 4e10b2b - [lldb][test] Remove explicit mydir definitions (NFC)

Author: Dave Lee Date: 2022-10-27T09:08:54-07:00 New Revision: 4e10b2bee71a45ffbf07fcce00d24ddf51867a0c URL: https://github.com/llvm/llvm-project/commit/4e10b2bee71a45ffbf07fcce00d24ddf51867a0c DIFF: https://github.com/llvm/llvm-project/commit/4e10b2bee71a45ffbf07fcce00d24ddf51867a0c.diff

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

yinghuitan added inline comments. Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:306 } + output.push_back('\n'); // Ensure log message has line break. g_vsc.SendOutput(OutputType::Console, output.c_str()); clayborg wrote: > I would still only push

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo added a comment. Also for the record, I'd love to get rid of this symlink based setup, if I could make cmake produce project files where caching works in the same way. The main requirement for that would be to have the whole cmake build started from the toplevel llvm-project directory

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

jasonmolenda updated this revision to Diff 471272. jasonmolenda added a comment. Incorporate Thorsten's suggestions for more idiomatic use of the std::optional result. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136719/new/

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

This revision was automatically updated to reflect the committed changes. Closed by commit rGd40f4636c454: Handle an unknown binary platform type in debugserver (authored by jasonmolenda). Changed prior to commit: https://reviews.llvm.org/D136719?vs=471272=471278#toc Repository: rG LLVM

[Lldb-commits] [lldb] d40f463 - Handle an unknown binary platform type in debugserver

Author: Jason Molenda Date: 2022-10-27T13:11:20-07:00 New Revision: d40f4636c454cf278b390b6591bf1e8dde8252aa URL: https://github.com/llvm/llvm-project/commit/d40f4636c454cf278b390b6591bf1e8dde8252aa DIFF: https://github.com/llvm/llvm-project/commit/d40f4636c454cf278b390b6591bf1e8dde8252aa.diff

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

JDevlieghere added inline comments. Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:725 +MachProcess::GetPlatformString(unsigned char platform) { + std::optional platform_string; switch (platform) { This is unused Repository: rG LLVM

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

Michael137 added a comment. > Keep the argument printing part, and replace the basename parsing with a call > to the language parser ? The two main caveats here are: 1. the name parser drops the return type when parsing a basename 2. the code-path is also used for cases where a language plugin

[Lldb-commits] [PATCH] D136362: [LLDB][RISCV] Add RV64C instruction support for EmulateInstructionRISCV

seehearfeel added a comment. After commit 05ae747a5353811f93f5814f24d2335e6229d78a ("[LLDB][RISCV] Add RV64C instruction support for EmulateInstructionRISCV"), build failed when "ninja check-lldb" on my local machine:

[Lldb-commits] [PATCH] D136795: [LLDB] Mark placeholder modules in `target module list` output.

labath added a comment. It's a good idea, but this is a pretty bad layering violation. The "placeholder" object files aren't even first class plugins (though I think they should be -- as the concept is useful in other situations as well), but an internal detail of the minidump process plugin.