[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/96290 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/96290 >From 67d8bab2d2d42ca3ec5d07efd3be94e614dde2e9 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 20 Jun 2024 18:29:15 +0100 Subject: [PATCH 1/3] [lldb][ExpressionParser] Add DoPrepareForExecution API --- .../lldb/Expression/ExpressionParser.h| 25 ++- lldb/source/Expression/CMakeLists.txt | 1 + lldb/source/Expression/ExpressionParser.cpp | 73 +++ .../Clang/ClangExpressionParser.cpp | 46 +--- .../Clang/ClangExpressionParser.h | 23 ++ .../Clang/ClangUserExpression.cpp | 15 6 files changed, 103 insertions(+), 80 deletions(-) create mode 100644 lldb/source/Expression/ExpressionParser.cpp diff --git a/lldb/include/lldb/Expression/ExpressionParser.h b/lldb/include/lldb/Expression/ExpressionParser.h index ab5223c915530..2ef7e036909c7 100644 --- a/lldb/include/lldb/Expression/ExpressionParser.h +++ b/lldb/include/lldb/Expression/ExpressionParser.h @@ -119,14 +119,35 @@ class ExpressionParser { /// \return /// An error code indicating the success or failure of the operation. /// Test with Success(). - virtual Status + Status PrepareForExecution(lldb::addr_t &func_addr, lldb::addr_t &func_end, std::shared_ptr &execution_unit_sp, ExecutionContext &exe_ctx, bool &can_interpret, - lldb_private::ExecutionPolicy execution_policy) = 0; + lldb_private::ExecutionPolicy execution_policy); bool GetGenerateDebugInfo() const { return m_generate_debug_info; } +protected: + virtual Status + DoPrepareForExecution(lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) = 0; + +private: + /// Run all static initializers for an execution unit. + /// + /// \param[in] execution_unit_sp + /// The execution unit. + /// + /// \param[in] exe_ctx + /// The execution context to use when running them. Thread can't be null. + /// + /// \return + /// The error code indicating the + Status RunStaticInitializers(lldb::IRExecutionUnitSP &execution_unit_sp, + ExecutionContext &exe_ctx); + protected: Expression &m_expr; ///< The expression to be parsed bool m_generate_debug_info; diff --git a/lldb/source/Expression/CMakeLists.txt b/lldb/source/Expression/CMakeLists.txt index 9ba5fefc09b6a..be1e132f7aaad 100644 --- a/lldb/source/Expression/CMakeLists.txt +++ b/lldb/source/Expression/CMakeLists.txt @@ -3,6 +3,7 @@ add_lldb_library(lldbExpression NO_PLUGIN_DEPENDENCIES DWARFExpression.cpp DWARFExpressionList.cpp Expression.cpp + ExpressionParser.cpp ExpressionTypeSystemHelper.cpp ExpressionVariable.cpp FunctionCaller.cpp diff --git a/lldb/source/Expression/ExpressionParser.cpp b/lldb/source/Expression/ExpressionParser.cpp new file mode 100644 index 0..ac727be78e8d3 --- /dev/null +++ b/lldb/source/Expression/ExpressionParser.cpp @@ -0,0 +1,73 @@ +//===-- ExpressionParser.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Expression/ExpressionParser.h" +#include "lldb/Expression/DiagnosticManager.h" +#include "lldb/Expression/IRExecutionUnit.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/ThreadPlanCallFunction.h" + +using namespace lldb_private; + +Status ExpressionParser::PrepareForExecution( +lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) { + Status status = + DoPrepareForExecution(func_addr, func_end, execution_unit_sp, exe_ctx, +can_interpret, execution_policy); + if (status.Success() && exe_ctx.GetProcessPtr() && exe_ctx.HasThreadScope()) { +status = RunStaticInitializers(execution_unit_sp, exe_ctx); + } + return status; +} + +Status ExpressionParser::RunStaticInitializers( +lldb::IRExecutionUnitSP &execution_unit_sp, ExecutionContext &exe_ctx) { + lldb_private::Status err; + + lldbassert(execution_unit_sp.get()); + lldbassert(exe_ctx.HasThreadScope()); + + if (!execution_unit_sp.get()) { +err.SetErrorString( +"can't run static initializers for a NULL execution unit"); +return err; + } + + if (!exe_ctx.HasThreadScope()) { +err.SetErrorString("can't run static initializers without a thread"); +
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/96290 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
@@ -0,0 +1,73 @@ +//===-- ExpressionParser.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Expression/ExpressionParser.h" +#include "lldb/Expression/DiagnosticManager.h" +#include "lldb/Expression/IRExecutionUnit.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/ThreadPlanCallFunction.h" + +using namespace lldb_private; + +Status ExpressionParser::PrepareForExecution( +lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) { + Status status = + DoPrepareForExecution(func_addr, func_end, execution_unit_sp, exe_ctx, +can_interpret, execution_policy); + if (status.Success() && exe_ctx.GetProcessPtr() && exe_ctx.HasThreadScope()) { +status = RunStaticInitializers(execution_unit_sp, exe_ctx); + } + return status; +} + +Status ExpressionParser::RunStaticInitializers( +lldb::IRExecutionUnitSP &execution_unit_sp, ExecutionContext &exe_ctx) { + lldb_private::Status err; JDevlieghere wrote: Nit: Drop `lldb_private::` https://github.com/llvm/llvm-project/pull/96290 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
@@ -0,0 +1,73 @@ +//===-- ExpressionParser.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Expression/ExpressionParser.h" +#include "lldb/Expression/DiagnosticManager.h" +#include "lldb/Expression/IRExecutionUnit.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/ThreadPlanCallFunction.h" + +using namespace lldb_private; + +Status ExpressionParser::PrepareForExecution( +lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) { + Status status = + DoPrepareForExecution(func_addr, func_end, execution_unit_sp, exe_ctx, +can_interpret, execution_policy); + if (status.Success() && exe_ctx.GetProcessPtr() && exe_ctx.HasThreadScope()) { +status = RunStaticInitializers(execution_unit_sp, exe_ctx); + } JDevlieghere wrote: Nit: no braces. https://github.com/llvm/llvm-project/pull/96290 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
@@ -0,0 +1,73 @@ +//===-- ExpressionParser.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Expression/ExpressionParser.h" +#include "lldb/Expression/DiagnosticManager.h" +#include "lldb/Expression/IRExecutionUnit.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/ThreadPlanCallFunction.h" + +using namespace lldb_private; JDevlieghere wrote: If you add `using namespace lldb` you could drop some `lldb::` prefixes. https://github.com/llvm/llvm-project/pull/96290 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
@@ -0,0 +1,73 @@ +//===-- ExpressionParser.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Expression/ExpressionParser.h" +#include "lldb/Expression/DiagnosticManager.h" +#include "lldb/Expression/IRExecutionUnit.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/ThreadPlanCallFunction.h" + +using namespace lldb_private; + +Status ExpressionParser::PrepareForExecution( +lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) { + Status status = + DoPrepareForExecution(func_addr, func_end, execution_unit_sp, exe_ctx, +can_interpret, execution_policy); + if (status.Success() && exe_ctx.GetProcessPtr() && exe_ctx.HasThreadScope()) { +status = RunStaticInitializers(execution_unit_sp, exe_ctx); + } + return status; +} + +Status ExpressionParser::RunStaticInitializers( +lldb::IRExecutionUnitSP &execution_unit_sp, ExecutionContext &exe_ctx) { + lldb_private::Status err; + + lldbassert(execution_unit_sp.get()); + lldbassert(exe_ctx.HasThreadScope()); + + if (!execution_unit_sp.get()) { +err.SetErrorString( +"can't run static initializers for a NULL execution unit"); +return err; + } + + if (!exe_ctx.HasThreadScope()) { +err.SetErrorString("can't run static initializers without a thread"); +return err; + } JDevlieghere wrote: If we're going to handle the error graciously, then I don't see the point of the `lldbassert` above. If we want to avoid this situation in the test suite we can make them regular asserts, or just omit them altogether. https://github.com/llvm/llvm-project/pull/96290 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/96290 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes This patch adds a new `DoPrepareForExecution` API, which can be implemented by the Clang and Swift language plugins. This also moves `RunStaticInitializers` into `ExpressionParser::PrepareForExecution`, so we call it consistently between language plugins. This *should* be mostly NFC (the static initializers will still only run after we finished parsing). We've been living on this patch downstream for sometime now. --- Full diff: https://github.com/llvm/llvm-project/pull/96290.diff 6 Files Affected: - (modified) lldb/include/lldb/Expression/ExpressionParser.h (+23-2) - (modified) lldb/source/Expression/CMakeLists.txt (+1) - (added) lldb/source/Expression/ExpressionParser.cpp (+73) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+1-45) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h (+5-18) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp (-15) ``diff diff --git a/lldb/include/lldb/Expression/ExpressionParser.h b/lldb/include/lldb/Expression/ExpressionParser.h index ab5223c915530..2ef7e036909c7 100644 --- a/lldb/include/lldb/Expression/ExpressionParser.h +++ b/lldb/include/lldb/Expression/ExpressionParser.h @@ -119,14 +119,35 @@ class ExpressionParser { /// \return /// An error code indicating the success or failure of the operation. /// Test with Success(). - virtual Status + Status PrepareForExecution(lldb::addr_t &func_addr, lldb::addr_t &func_end, std::shared_ptr &execution_unit_sp, ExecutionContext &exe_ctx, bool &can_interpret, - lldb_private::ExecutionPolicy execution_policy) = 0; + lldb_private::ExecutionPolicy execution_policy); bool GetGenerateDebugInfo() const { return m_generate_debug_info; } +protected: + virtual Status + DoPrepareForExecution(lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) = 0; + +private: + /// Run all static initializers for an execution unit. + /// + /// \param[in] execution_unit_sp + /// The execution unit. + /// + /// \param[in] exe_ctx + /// The execution context to use when running them. Thread can't be null. + /// + /// \return + /// The error code indicating the + Status RunStaticInitializers(lldb::IRExecutionUnitSP &execution_unit_sp, + ExecutionContext &exe_ctx); + protected: Expression &m_expr; ///< The expression to be parsed bool m_generate_debug_info; diff --git a/lldb/source/Expression/CMakeLists.txt b/lldb/source/Expression/CMakeLists.txt index 9ba5fefc09b6a..be1e132f7aaad 100644 --- a/lldb/source/Expression/CMakeLists.txt +++ b/lldb/source/Expression/CMakeLists.txt @@ -3,6 +3,7 @@ add_lldb_library(lldbExpression NO_PLUGIN_DEPENDENCIES DWARFExpression.cpp DWARFExpressionList.cpp Expression.cpp + ExpressionParser.cpp ExpressionTypeSystemHelper.cpp ExpressionVariable.cpp FunctionCaller.cpp diff --git a/lldb/source/Expression/ExpressionParser.cpp b/lldb/source/Expression/ExpressionParser.cpp new file mode 100644 index 0..ac727be78e8d3 --- /dev/null +++ b/lldb/source/Expression/ExpressionParser.cpp @@ -0,0 +1,73 @@ +//===-- ExpressionParser.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Expression/ExpressionParser.h" +#include "lldb/Expression/DiagnosticManager.h" +#include "lldb/Expression/IRExecutionUnit.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/ThreadPlanCallFunction.h" + +using namespace lldb_private; + +Status ExpressionParser::PrepareForExecution( +lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) { + Status status = + DoPrepareForExecution(func_addr, func_end, execution_unit_sp, exe_ctx, +can_interpret, execution_policy); + if (status.Success() && exe_ctx.GetProcessPtr() && exe_ctx.HasThreadScope()) { +status = RunStaticInitializers(execution_unit_sp, exe_ctx); + } + return status; +} + +Status ExpressionParser::RunStaticInitializers( +lldb::IRExecutionUnitSP &execution_unit_sp, ExecutionContext &exe_ctx) { + lldb_private::Status err; + + lldbassert(execution_unit_sp.get()); + lldbassert(ex
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Add new DoPrepareForExecution interface to be implemented by language plugins (PR #96290)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/96290 This patch adds a new `DoPrepareForExecution` API, which can be implemented by the Clang and Swift language plugins. This also moves `RunStaticInitializers` into `ExpressionParser::PrepareForExecution`, so we call it consistently between language plugins. This *should* be mostly NFC (the static initializers will still only run after we finished parsing). We've been living on this patch downstream for sometime now. >From 67d8bab2d2d42ca3ec5d07efd3be94e614dde2e9 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 20 Jun 2024 18:29:15 +0100 Subject: [PATCH] [lldb][ExpressionParser] Add DoPrepareForExecution API --- .../lldb/Expression/ExpressionParser.h| 25 ++- lldb/source/Expression/CMakeLists.txt | 1 + lldb/source/Expression/ExpressionParser.cpp | 73 +++ .../Clang/ClangExpressionParser.cpp | 46 +--- .../Clang/ClangExpressionParser.h | 23 ++ .../Clang/ClangUserExpression.cpp | 15 6 files changed, 103 insertions(+), 80 deletions(-) create mode 100644 lldb/source/Expression/ExpressionParser.cpp diff --git a/lldb/include/lldb/Expression/ExpressionParser.h b/lldb/include/lldb/Expression/ExpressionParser.h index ab5223c915530..2ef7e036909c7 100644 --- a/lldb/include/lldb/Expression/ExpressionParser.h +++ b/lldb/include/lldb/Expression/ExpressionParser.h @@ -119,14 +119,35 @@ class ExpressionParser { /// \return /// An error code indicating the success or failure of the operation. /// Test with Success(). - virtual Status + Status PrepareForExecution(lldb::addr_t &func_addr, lldb::addr_t &func_end, std::shared_ptr &execution_unit_sp, ExecutionContext &exe_ctx, bool &can_interpret, - lldb_private::ExecutionPolicy execution_policy) = 0; + lldb_private::ExecutionPolicy execution_policy); bool GetGenerateDebugInfo() const { return m_generate_debug_info; } +protected: + virtual Status + DoPrepareForExecution(lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) = 0; + +private: + /// Run all static initializers for an execution unit. + /// + /// \param[in] execution_unit_sp + /// The execution unit. + /// + /// \param[in] exe_ctx + /// The execution context to use when running them. Thread can't be null. + /// + /// \return + /// The error code indicating the + Status RunStaticInitializers(lldb::IRExecutionUnitSP &execution_unit_sp, + ExecutionContext &exe_ctx); + protected: Expression &m_expr; ///< The expression to be parsed bool m_generate_debug_info; diff --git a/lldb/source/Expression/CMakeLists.txt b/lldb/source/Expression/CMakeLists.txt index 9ba5fefc09b6a..be1e132f7aaad 100644 --- a/lldb/source/Expression/CMakeLists.txt +++ b/lldb/source/Expression/CMakeLists.txt @@ -3,6 +3,7 @@ add_lldb_library(lldbExpression NO_PLUGIN_DEPENDENCIES DWARFExpression.cpp DWARFExpressionList.cpp Expression.cpp + ExpressionParser.cpp ExpressionTypeSystemHelper.cpp ExpressionVariable.cpp FunctionCaller.cpp diff --git a/lldb/source/Expression/ExpressionParser.cpp b/lldb/source/Expression/ExpressionParser.cpp new file mode 100644 index 0..ac727be78e8d3 --- /dev/null +++ b/lldb/source/Expression/ExpressionParser.cpp @@ -0,0 +1,73 @@ +//===-- ExpressionParser.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Expression/ExpressionParser.h" +#include "lldb/Expression/DiagnosticManager.h" +#include "lldb/Expression/IRExecutionUnit.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/ThreadPlanCallFunction.h" + +using namespace lldb_private; + +Status ExpressionParser::PrepareForExecution( +lldb::addr_t &func_addr, lldb::addr_t &func_end, +std::shared_ptr &execution_unit_sp, +ExecutionContext &exe_ctx, bool &can_interpret, +lldb_private::ExecutionPolicy execution_policy) { + Status status = + DoPrepareForExecution(func_addr, func_end, execution_unit_sp, exe_ctx, +can_interpret, execution_policy); + if (status.Success() && exe_ctx.GetProcessPtr() && exe_ctx.HasThreadScope()) { +status = RunStaticInitializers(execution_unit_sp, exe_ctx); + } + return status; +} + +Status ExpressionParser::RunStaticInitializers( +lldb::IRExecutionUnitSP &execution_