[lldb-dap] Adjust VariableReferenceStorage lifetime management. - #183176
Merged
Merged
Conversation
Adjusting `VariableReferenceStorage` to only need to track permanent vs temporary storage by making `VariableStore` the common base class. Moved the subclasses of `VariableStore` into the Variables.cpp file, since they're no long referenced externally. Expanding on the tests by adding an updated core dump with variables in the argument scope we can use to validate variable storage.
Member
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesAdjusting Moved the subclasses of Expanding on the tests by adding an updated core dump with variables in the argument scope we can use to validate variable storage. Patch is 301.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/183176.diff 11 Files Affected:
diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
index 77d9f03554a2a..f2117d81f315c 100644
--- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
@@ -133,8 +133,8 @@ EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const {
body.type = desc.display_type_name;
if (value.MightHaveChildren() || ValuePointsToCode(value))
- body.variablesReference = dap.reference_storage.InsertVariable(
- value, /*is_permanent=*/is_repl_context);
+ body.variablesReference =
+ dap.reference_storage.Insert(value, /*is_permanent=*/is_repl_context);
if (lldb::addr_t addr = value.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
body.memoryReference = EncodeMemoryReference(addr);
diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
index 3786d583e0e87..b2b006baf8a5e 100644
--- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
@@ -36,8 +36,7 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const {
frame.GetThread().SetSelectedFrame(frame.GetFrameID());
}
- std::vector<protocol::Scope> scopes =
- dap.reference_storage.CreateScopes(frame);
+ std::vector<protocol::Scope> scopes = dap.reference_storage.Insert(frame);
return ScopesResponseBody{std::move(scopes)};
}
diff --git a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
index 725d5de094c95..8fe424b55c9b1 100644
--- a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
@@ -67,7 +67,7 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const {
// is_permanent is false because debug console does not support
// setVariable request.
const var_ref_t new_var_ref =
- dap.reference_storage.InsertVariable(variable, /*is_permanent=*/false);
+ dap.reference_storage.Insert(variable, /*is_permanent=*/false);
if (variable.MightHaveChildren()) {
body.variablesReference = new_var_ref;
if (desc.type_obj.IsArrayType())
diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
index 26719b3a14d4d..46dda9e4c5d08 100644
--- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
@@ -13,7 +13,6 @@
#include "Protocol/DAPTypes.h"
#include "Protocol/ProtocolRequests.h"
#include "Variables.h"
-#include "llvm/Support/ErrorExtras.h"
using namespace llvm;
using namespace lldb_dap::protocol;
diff --git a/lldb/tools/lldb-dap/Protocol/DAPTypes.h b/lldb/tools/lldb-dap/Protocol/DAPTypes.h
index 5c5d279656d83..5564489cf2e69 100644
--- a/lldb/tools/lldb-dap/Protocol/DAPTypes.h
+++ b/lldb/tools/lldb-dap/Protocol/DAPTypes.h
@@ -28,8 +28,7 @@ namespace lldb_dap::protocol {
enum ReferenceKind : uint8_t {
eReferenceKindTemporary = 0,
eReferenceKindPermanent = 1,
- eReferenceKindScope = 1 << 1,
- eReferenceKindInvalid = 0xFF
+ eReferenceKindInvalid = 0xFF,
};
/// The var_ref_t hold two values, the `ReferenceKind` and the
@@ -63,7 +62,6 @@ struct var_ref_t {
switch (current_kind) {
case eReferenceKindTemporary:
case eReferenceKindPermanent:
- case eReferenceKindScope:
return current_kind;
default:
return eReferenceKindInvalid;
diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp
index 1f429b087ac0e..7891ee64ac9fc 100644
--- a/lldb/tools/lldb-dap/Variables.cpp
+++ b/lldb/tools/lldb-dap/Variables.cpp
@@ -1,4 +1,4 @@
-//===-- Variables.cpp ---------------------------------------------------*-===//
+//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -15,7 +15,6 @@
#include "ProtocolUtils.h"
#include "SBAPIExtras.h"
#include "lldb/API/SBFrame.h"
-#include "lldb/API/SBProcess.h"
#include "lldb/API/SBValue.h"
#include "lldb/API/SBValueList.h"
#include "llvm/ADT/Sequence.h"
@@ -27,285 +26,394 @@
using namespace lldb_dap;
using namespace lldb_dap::protocol;
-namespace lldb_dap {
-
-protocol::Scope CreateScope(ScopeKind kind, var_ref_t variablesReference,
- bool expensive) {
- protocol::Scope scope;
- scope.variablesReference = variablesReference;
- scope.expensive = expensive;
+namespace {
+
+/// A Variable store for fetching variables within a specific scope (locals,
+/// globals, or registers) for a given stack frame.
+class ScopeStore final : public VariableStore {
+public:
+ explicit ScopeStore(ScopeKind kind, const lldb::SBFrame &frame)
+ : m_frame(frame), m_kind(kind) {}
+
+ std::vector<protocol::Variable>
+ GetVariables(VariableReferenceStorage &storage,
+ const protocol::Configuration &config,
+ const protocol::VariablesArguments &args) override {
+ LoadVariables();
+ if (m_kind == lldb_dap::eScopeKindRegisters)
+ SetRegistersFormat();
+
+ const bool format_hex = args.format ? args.format->hex : false;
+ std::vector<Variable> variables;
+ if (m_kind == eScopeKindLocals)
+ AddReturnValue(storage, config, variables, format_hex);
+
+ const uint64_t count = args.count;
+ const uint32_t start_idx = 0;
+ const uint32_t num_children = m_children.GetSize();
+ const uint32_t end_idx = start_idx + ((count == 0) ? num_children : count);
+
+ // We first find out which variable names are duplicated.
+ std::map<llvm::StringRef, uint32_t> variable_name_counts;
+ for (auto i = start_idx; i < end_idx; ++i) {
+ lldb::SBValue variable = m_children.GetValueAtIndex(i);
+ if (!variable.IsValid())
+ break;
+ variable_name_counts[GetNonNullVariableName(variable)]++;
+ }
- // TODO: Support "arguments" and "return value" scope.
- // At the moment lldb-dap includes the arguments and return_value into the
- // "locals" scope.
- // VS Code only expands the first non-expensive scope. This causes friction
- // if we add the arguments above the local scope, as the locals scope will not
- // be expanded if we enter a function with arguments. It becomes more
- // annoying when the scope has arguments, return_value and locals.
- switch (kind) {
- case eScopeKindLocals:
- scope.presentationHint = protocol::Scope::eScopePresentationHintLocals;
- scope.name = "Locals";
- break;
- case eScopeKindGlobals:
- scope.name = "Globals";
- break;
- case eScopeKindRegisters:
- scope.presentationHint = protocol::Scope::eScopePresentationHintRegisters;
- scope.name = "Registers";
- break;
+ // Now we construct the result with unique display variable names.
+ for (auto i = start_idx; i < end_idx; ++i) {
+ lldb::SBValue variable = m_children.GetValueAtIndex(i);
+
+ if (!variable.IsValid())
+ break;
+
+ const var_ref_t frame_var_ref =
+ storage.Insert(variable, /*is_permanent=*/false);
+ if (LLVM_UNLIKELY(frame_var_ref.AsUInt32() >=
+ var_ref_t::k_variables_reference_threshold)) {
+ DAP_LOG(storage.log,
+ "warning: scopes variablesReference threshold reached. "
+ "current: {} threshold: {}, maximum {}.",
+ frame_var_ref.AsUInt32(),
+ var_ref_t::k_variables_reference_threshold,
+ var_ref_t::k_max_variables_references);
+ }
+
+ if (LLVM_UNLIKELY(frame_var_ref.Kind() == eReferenceKindInvalid))
+ break;
+
+ variables.emplace_back(CreateVariable(
+ variable, frame_var_ref, format_hex,
+ config.enableAutoVariableSummaries,
+ config.enableSyntheticChildDebugging,
+ variable_name_counts[GetNonNullVariableName(variable)] > 1));
+ }
+ return variables;
}
- return scope;
-}
-
-std::vector<Variable>
-ScopeStore::GetVariables(VariableReferenceStorage &storage,
- const Configuration &config,
- const VariablesArguments &args) {
- LoadVariables();
- if (m_kind == lldb_dap::eScopeKindRegisters)
- SetRegistersFormat();
-
- const bool format_hex = args.format ? args.format->hex : false;
- std::vector<Variable> variables;
- if (m_kind == eScopeKindLocals)
- AddReturnValue(storage, config, variables, format_hex);
-
- const uint64_t count = args.count;
- const uint32_t start_idx = 0;
- const uint32_t num_children = m_children.GetSize();
- const uint32_t end_idx = start_idx + ((count == 0) ? num_children : count);
-
- // We first find out which variable names are duplicated.
- std::map<llvm::StringRef, uint32_t> variable_name_counts;
- for (auto i = start_idx; i < end_idx; ++i) {
- lldb::SBValue variable = m_children.GetValueAtIndex(i);
- if (!variable.IsValid())
- break;
- variable_name_counts[GetNonNullVariableName(variable)]++;
+ lldb::SBValue FindVariable(llvm::StringRef name) override {
+ LoadVariables();
+
+ lldb::SBValue variable;
+ const bool is_name_duplicated = name.contains(" @");
+ // variablesReference is one of our scopes, not an actual variable it is
+ // asking for a variable in locals or globals or registers.
+ const uint32_t end_idx = m_children.GetSize();
+ // Searching backward so that we choose the variable in closest scope
+ // among variables of the same name.
+ for (const uint32_t i : llvm::reverse(llvm::seq<uint32_t>(0, end_idx))) {
+ lldb::SBValue curr_variable = m_children.GetValueAtIndex(i);
+ std::string variable_name =
+ CreateUniqueVariableNameForDisplay(curr_variable, is_name_duplicated);
+ if (variable_name == name) {
+ variable = curr_variable;
+ break;
+ }
+ }
+ return variable;
}
- // Now we construct the result with unique display variable names.
- for (auto i = start_idx; i < end_idx; ++i) {
- lldb::SBValue variable = m_children.GetValueAtIndex(i);
+ lldb::SBValue GetVariable() const override { return lldb::SBValue(); }
- if (!variable.IsValid())
- break;
-
- const var_ref_t frame_var_ref =
- storage.InsertVariable(variable, /*is_permanent=*/false);
- if (LLVM_UNLIKELY(frame_var_ref.AsUInt32() >=
- var_ref_t::k_variables_reference_threshold)) {
- DAP_LOG(storage.log,
- "warning: scopes variablesReference threshold reached. "
- "current: {} threshold: {}, maximum {}.",
- frame_var_ref.AsUInt32(),
- var_ref_t::k_variables_reference_threshold,
- var_ref_t::k_max_variables_references);
- }
+private:
+ void LoadVariables() {
+ if (m_variables_loaded)
+ return;
- if (LLVM_UNLIKELY(frame_var_ref.Kind() == eReferenceKindInvalid))
+ m_variables_loaded = true;
+ switch (m_kind) {
+ case eScopeKindLocals:
+ m_children = m_frame.GetVariables(/*arguments=*/true,
+ /*locals=*/true,
+ /*statics=*/false,
+ /*in_scope_only=*/true);
break;
-
- variables.emplace_back(CreateVariable(
- variable, frame_var_ref, format_hex, config.enableAutoVariableSummaries,
- config.enableSyntheticChildDebugging,
- variable_name_counts[GetNonNullVariableName(variable)] > 1));
- }
- return variables;
-}
-
-lldb::SBValue ScopeStore::FindVariable(llvm::StringRef name) {
- LoadVariables();
-
- lldb::SBValue variable;
- const bool is_name_duplicated = name.contains(" @");
- // variablesReference is one of our scopes, not an actual variable it is
- // asking for a variable in locals or globals or registers.
- const uint32_t end_idx = m_children.GetSize();
- // Searching backward so that we choose the variable in closest scope
- // among variables of the same name.
- for (const uint32_t i : llvm::reverse(llvm::seq<uint32_t>(0, end_idx))) {
- lldb::SBValue curr_variable = m_children.GetValueAtIndex(i);
- std::string variable_name =
- CreateUniqueVariableNameForDisplay(curr_variable, is_name_duplicated);
- if (variable_name == name) {
- variable = curr_variable;
+ case eScopeKindGlobals:
+ m_children = m_frame.GetVariables(/*arguments=*/false,
+ /*locals=*/false,
+ /*statics=*/true,
+ /*in_scope_only=*/true);
break;
+ case eScopeKindRegisters:
+ m_children = m_frame.GetRegisters();
}
}
- return variable;
-}
-void ScopeStore::LoadVariables() {
- if (m_variables_loaded)
- return;
-
- m_variables_loaded = true;
- switch (m_kind) {
- case eScopeKindLocals:
- m_children = m_frame.GetVariables(/*arguments=*/true,
- /*locals=*/true,
- /*statics=*/false,
- /*in_scope_only=*/true);
- break;
- case eScopeKindGlobals:
- m_children = m_frame.GetVariables(/*arguments=*/false,
- /*locals=*/false,
- /*statics=*/true,
- /*in_scope_only=*/true);
- break;
- case eScopeKindRegisters:
- m_children = m_frame.GetRegisters();
+ void SetRegistersFormat() {
+ // Change the default format of any pointer sized registers in the first
+ // register set to be the lldb::eFormatAddressInfo so we show the pointer
+ // and resolve what the pointer resolves to. Only change the format if the
+ // format was set to the default format or if it was hex as some registers
+ // have formats set for them.
+ const uint32_t addr_size =
+ m_frame.GetThread().GetProcess().GetAddressByteSize();
+ for (lldb::SBValue reg : m_children.GetValueAtIndex(0)) {
+ const lldb::Format format = reg.GetFormat();
+ if (format == lldb::eFormatDefault || format == lldb::eFormatHex) {
+ if (reg.GetByteSize() == addr_size)
+ reg.SetFormat(lldb::eFormatAddressInfo);
+ }
+ }
}
-}
-void ScopeStore::SetRegistersFormat() {
- // Change the default format of any pointer sized registers in the first
- // register set to be the lldb::eFormatAddressInfo so we show the pointer
- // and resolve what the pointer resolves to. Only change the format if the
- // format was set to the default format or if it was hex as some registers
- // have formats set for them.
- const uint32_t addr_size =
- m_frame.GetThread().GetProcess().GetAddressByteSize();
- for (lldb::SBValue reg : m_children.GetValueAtIndex(0)) {
- const lldb::Format format = reg.GetFormat();
- if (format == lldb::eFormatDefault || format == lldb::eFormatHex) {
- if (reg.GetByteSize() == addr_size)
- reg.SetFormat(lldb::eFormatAddressInfo);
+ void AddReturnValue(VariableReferenceStorage &storage,
+ const protocol::Configuration &config,
+ std::vector<protocol::Variable> &variables,
+ bool format_hex) {
+ assert(m_kind == eScopeKindLocals &&
+ "Return Value Should only be in local scope");
+ if (m_children.GetSize() == 0) {
+ // Check for an error in the SBValueList that might explain why we don't
+ // have locals. If we have an error display it as the sole value in the
+ // the locals.
+
+ // "error" owns the error string so we must keep it alive as long as we
+ // want to use the returns "const char *".
+ lldb::SBError error = m_children.GetError();
+ if (const char *var_err = error.GetCString()) {
+ // Create a fake variable named "error" to explain why variables were
+ // not available. This new error will help let users know when there was
+ // a problem that kept variables from being available for display and
+ // allow users to fix this issue instead of seeing no variables. The
+ // errors are only set when there is a problem that the user could
+ // fix, so no error will show up when you have no debug info, only when
+ // we do have debug info and something that is fixable can be done.
+ Variable err_var;
+ err_var.name = "<error>";
+ err_var.type = "const char *";
+ err_var.value = var_err;
+ variables.push_back(std::move(err_var));
+ }
+ return;
}
- }
-}
-void ScopeStore::AddReturnValue(VariableReferenceStorage &storage,
- const Configuration &config,
- std::vector<Variable> &variables,
- bool format_hex) {
- assert(m_kind == eScopeKindLocals &&
- "Return Value Should only be in local scope");
- if (m_children.GetSize() == 0) {
- // Check for an error in the SBValueList that might explain why we don't
- // have locals. If we have an error display it as the sole value in the
- // the locals.
-
- // "error" owns the error string so we must keep it alive as long as we
- // want to use the returns "const char *".
- lldb::SBError error = m_children.GetError();
- if (const char *var_err = error.GetCString()) {
- // Create a fake variable named "error" to explain why variables were
- // not available. This new error will help let users know when there was
- // a problem that kept variables from being available for display and
- // allow users to fix this issue instead of seeing no variables. The
- // errors are only set when there is a problem that the user could
- // fix, so no error will show up when you have no debug info, only when
- // we do have debug info and something that is fixable can be done.
- Variable err_var;
- err_var.name = "<error>";
- err_var.type = "const char *";
- err_var.value = var_err;
- variables.push_back(std::move(err_var));
+ // Show return value if there is any ( in the local top frame )
+ lldb::SBThread selected_thread = m_frame.GetThread();
+ lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
+
+ if (stop_return_value.IsValid() &&
+ (selected_thread.GetSelectedFrame().GetFrameID() == 0)) {
+ auto renamed_return_value = stop_return_value.Clone("(Return Value)");
+ var_ref_t return_var_ref{var_ref_t::k_no_child};
+
+ if (stop_return_value.MightHaveChildren() ||
+ stop_return_value.IsSynthetic()) {
+ return_var_ref = storage.Insert(stop_return_value,
+ /*is_permanent=*/false);
+ }
+ variables.emplace_back(
+ CreateVariable(renamed_return_value, return_var_ref, format_hex,
+ config.enableAutoVariableSummaries,
+ config.enableSyntheticChildDebugging, false));
}
- return;
}
- // Show return value if there is any ( in the local top frame )
- lldb::SBThread selected_thread = m_frame.GetThread();
- lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
+ lldb::SBFrame m_frame;
+ lldb::SBValueList m_children;
+ ScopeKind m_kind;
+ bool m_variables_loaded = false;
+};
+
+/// Variable store for expandable values.
+///
+/// Manages children variables of complex types (structs, arrays, pointers,
+/// etc.) that can be expanded in the debugger UI.
+class ExpandableValueStore final : public VariableStore {
+
+public:
+ explicit ExpandableValueStore(const lldb::SBValue &value) : m_value(value) {}
+
+ std::vector<protocol::Variable>
+ GetVariables(VariableReferenceStorage &storage,
+ const protocol::Configuration &config,
+ const protocol::VariablesArguments &args) override {
+ const var_ref_t var_ref = args.variablesReference;
+ const uint64_t count = args.count;
+ const uint64_t start = args.start;
+ const bool hex = args.format ? args.format->hex : false;
+
+ lldb::SBValue variable = storage.GetVariable(var_ref);
+ if (!variable)
+ return {};
+
+ // We are expanding a variable that has children, so we will return its
+ // children.
+ ...
[truncated]
|
Contributor
Author
|
Split off of pr #180858 |
JDevlieghere
approved these changes
Feb 25, 2026
JDevlieghere
left a comment
JDevlieghere
left a comment
Member
There was a problem hiding this comment.
This refactoring seems sensible. LGTM with a few nits.
DrSergei
approved these changes
Feb 25, 2026
da-viper
reviewed
Feb 25, 2026
da-viper
left a comment
da-viper
left a comment
Contributor
There was a problem hiding this comment.
Is there a motivation behind this ?. As we are giving up the ability to know what type of variableReference was requested if there is a crash. We are also adding an extra indirection and allocation.
| lldb::SBValue m_value; | ||
| }; | ||
|
|
||
| class ExpandableValueListStore final : public VariableStore { |
Contributor
There was a problem hiding this comment.
I could not find any thing using this class.
Only
VariableReferenceStorage::Insert(const lldb::SBValueList &values) and that is not used by any one.
… to reduce repetition.
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
Contributor
Author
This is used in #180858 but I am splitting things out into smaller PRs |
… Variables.cpp and ProtocolUtils.cpp, maybe in the future we can move the code out of ProtocolUtils.cpp.
da-viper
reviewed
Feb 25, 2026
da-viper
reviewed
Feb 25, 2026
Contributor
Author
|
While working on the latest changes I ran into #183578, for now I have a work around, but |
da-viper
reviewed
Feb 26, 2026
Comment on lines
+88
to
+92
| std::optional<std::string> custom_name; | ||
| auto name_it = name_overrides.find(variable.GetID()); | ||
| if (name_it != name_overrides.end()) | ||
| custom_name = name_it->second; | ||
|
|
Contributor
There was a problem hiding this comment.
Can we handle the local scope differently. as It seems to me we are trying to make the make_variable abstraction work. Instead we end up paying for things that are not used by most callers.
Contributor
Author
There was a problem hiding this comment.
I was using SBValue.Clone() at first to perform the rename but I ran into #183578 that was causing issues. We use the name override for the '[raw]' synthetic value as well.
Contributor
Author
There was a problem hiding this comment.
I may follow up this with moving CreateVariable into this file and trying to simplify some of this further, but I wanted to minimize the changes for now.
da-viper
approved these changes
Feb 27, 2026
da-viper
left a comment
da-viper
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
Modulo the return value is the last item in the local scope.
… check that its first.
Contributor
|
@ashgti this is causing failures on the mac green dragon bots: Could you have a look please? https://green.lab.llvm.org/job/as-lldb-cmake-os-verficiation/979/ |
Contributor
Author
|
What arch / OS is the CI running? That test works for me locally, I am on arm64 (M4 Max) 26.3. |
sujianIBM
pushed a commit
to sujianIBM/llvm-project
that referenced
this pull request
Mar 5, 2026
…#183176) Adjusting `VariableReferenceStorage` to only need to track permanent vs temporary storage by making `VariableStore` the common base class. Moved the subclasses of `VariableStore` into the Variables.cpp file, since they're no long referenced externally. Expanding on the tests by adding an updated core dump with variables in the argument scope we can use to validate variable storage.
This was referenced Apr 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adjusting
VariableReferenceStorageto only need to track permanent vs temporary storage by makingVariableStorethe common base class.Moved the subclasses of
VariableStoreinto the Variables.cpp file, since they're no long referenced externally.Expanding on the tests by adding an updated core dump with variables in the argument scope we can use to validate variable storage.