Move the persistent variable counter into Target

[aprantl]

Summary

Move the persistent variable counter into Target so it can be shared across multiple language plugins.

In a multi-language project it is counterintuitive to have a result
variables reuse numbers just because they are using a different
language plugin in LLDB (but not for example, when they are
Objective-C versus C++, since they are both handled by Clang).

This is NFC on llvm.org except for the Go plugin.

rdar://problem/39299889

[aprantl]

ping.

[aprantl]

Thread an is_error flag through for plugins that need it.

[jingham]

Except for the inline comment, this is okay. If we thought hard we might find another likely reason why we'd want to use two result prefixes besides a language that has an error return side-channel. But I can't off the top of my head, so is_error is fine for now.

[aprantl]

I'm sorry: which inline comment? Did you perhaps forget to hit "Submit"?

[jingham]

Eh, must have. The page really shouldn't let you overall submit if there are uncommitted comments, but whatever. Let's see if I did it right this time..

source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

668

Don't hard-code "$" here, call GetPersistentVariablePrefix.

include/lldb/Expression/ExpressionVariable.h

public:
   CreatePersistentVariable(const lldb::ValueObjectSP &valobj_sp) = 0;
 
   virtual lldb::ExpressionVariableSP
   CreatePersistentVariable(ExecutionContextScope *exe_scope,
                            const ConstString &name, const CompilerType &type,
                            lldb::ByteOrder byte_order,
                            uint32_t addr_byte_size) = 0;
 
-  virtual ConstString GetNextPersistentVariableName() = 0;
+  virtual ConstString GetNextPersistentVariableName(Target &target) = 0;
 
   virtual void
   RemovePersistentVariable(lldb::ExpressionVariableSP variable) = 0;
 
   virtual lldb::addr_t LookupSymbol(const ConstString &name);
 
   void RegisterExecutionUnit(lldb::IRExecutionUnitSP &execution_unit_sp);
 

include/lldb/Target/Target.h

void ApplyNameToBreakpoints(BreakpointName &bp_name);
   lldb::ExpressionResults EvaluateExpression(
       llvm::StringRef expression, ExecutionContextScope *exe_scope,
       lldb::ValueObjectSP &result_valobj_sp,
       const EvaluateExpressionOptions &options = EvaluateExpressionOptions(),
       std::string *fixed_expression = nullptr);
 
   lldb::ExpressionVariableSP GetPersistentVariable(const ConstString &name);
 
+  /// Return the next available number for numbered persistent variables.
+  unsigned GetNextPersistentVariableIndex() {
+    return m_next_persistent_variable_index++;
+  }
+
   lldb::addr_t GetPersistentSymbol(const ConstString &name);
 
   //------------------------------------------------------------------
   // Target Stop Hooks
   //------------------------------------------------------------------
   class StopHook : public UserID {
   public:
     StopHook(const StopHook &rhs);
protected:
   lldb::SourceManagerUP m_source_manager_ap;
 
   typedef std::map<lldb::user_id_t, StopHookSP> StopHookCollection;
   StopHookCollection m_stop_hooks;
   lldb::user_id_t m_stop_hook_next_id;
   bool m_valid;
   bool m_suppress_stop_hooks;
   bool m_is_dummy_target;
+  unsigned m_next_persistent_variable_index = 0;
 
   static void ImageSearchPathsChanged(const PathMappingList &path_list,
                                       void *baton);
 
   //------------------------------------------------------------------
   // Utilities for `statistics` command.
   //------------------------------------------------------------------
 private:

source/Core/ValueObject.cpp

ValueObjectSP ValueObject::Persist() {
 
   PersistentExpressionState *persistent_state =
       target_sp->GetPersistentExpressionStateForLanguage(
           GetPreferredDisplayLanguage());
 
   if (!persistent_state)
     return nullptr;
 
-  ConstString name(persistent_state->GetNextPersistentVariableName());
+  ConstString name(persistent_state->GetNextPersistentVariableName(*target_sp));
 
   ValueObjectSP const_result_sp =
       ValueObjectConstResult::Create(target_sp.get(), GetValue(), name);
 
   ExpressionVariableSP clang_var_sp =
       persistent_state->CreatePersistentVariable(const_result_sp);
   clang_var_sp->m_live_sp = clang_var_sp->m_frozen_sp;
   clang_var_sp->m_flags |= ExpressionVariable::EVIsProgramReference;

source/Expression/Materializer.cpp

void Dematerialize(lldb::StackFrameSP &frame_sp, IRMemoryMap &map,
 
     if (!persistent_state) {
       err.SetErrorString("Couldn't dematerialize a result variable: "
                          "corresponding type system doesn't handle persistent "
                          "variables");
       return;
     }
 
-    ConstString name = m_delegate
-                           ? m_delegate->GetName()
-                           : persistent_state->GetNextPersistentVariableName();
+    ConstString name =
+        m_delegate
+            : persistent_state->GetNextPersistentVariableName(*target_sp);
 
     lldb::ExpressionVariableSP ret = persistent_state->CreatePersistentVariable(
         exe_scope, name, m_type, map.GetByteOrder(), map.GetAddressByteSize());
 
     if (!ret) {
       err.SetErrorStringWithFormat("couldn't dematerialize a result variable: "
                                    "failed to make persistent variable %s",
                                    name.AsCString());

source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h

public:
 
   //----------------------------------------------------------------------
   /// Return the next entry in the sequence of strings "$0", "$1", ... for
   /// use naming persistent expression convenience variables.
   ///
   /// @return
   ///     A string that contains the next persistent variable name.
   //----------------------------------------------------------------------
-  ConstString GetNextPersistentVariableName() override;
+  ConstString GetNextPersistentVariableName(Target &target) override;
 
   void RemovePersistentVariable(lldb::ExpressionVariableSP variable) override;
 
   void RegisterPersistentDecl(const ConstString &name, clang::NamedDecl *decl);
 
   clang::NamedDecl *GetPersistentDecl(const ConstString &name);
 
   void AddHandLoadedClangModule(ClangModulesDeclVendor::ModuleID module) {

source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp

 //===-- ClangPersistentVariables.cpp ----------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ClangPersistentVariables.h"
 
 #include "lldb/Core/Value.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "clang/AST/Decl.h"
 
 #include "llvm/ADT/StringMap.h"
 
void ClangPersistentVariables::RemovePersistentVariable(
   if (*name != '$')
     return;
   name++;
 
   if (strtoul(name, NULL, 0) == m_next_persistent_variable_id - 1)
     m_next_persistent_variable_id--;
 }
 
-ConstString ClangPersistentVariables::GetNextPersistentVariableName() {
+ConstString
+ClangPersistentVariables::GetNextPersistentVariableName(Target &target) {
   char name_cstr[256];
   ::snprintf(name_cstr, sizeof(name_cstr), "$%u",
-             m_next_persistent_variable_id++);
+             target.GetNextPersistentVariableIndex());
   ConstString name(name_cstr);
   return name;
 }
 
 void ClangPersistentVariables::RegisterPersistentDecl(const ConstString &name,
                                                       clang::NamedDecl *decl) {
   m_persistent_decls.insert(
       std::pair<const char *, clang::NamedDecl *>(name.GetCString(), decl));

source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

private:
   bool AddArguments(ExecutionContext &exe_ctx, std::vector<lldb::addr_t> &args,
                     lldb::addr_t struct_address,
                     DiagnosticManager &diagnostic_manager) override;
 
   ClangUserExpressionHelper m_type_system_helper;
 
   class ResultDelegate : public Materializer::PersistentVariableDelegate {
   public:
-    ResultDelegate();
+    ResultDelegate(lldb::TargetSP target) : m_target_sp(target) {}
     ConstString GetName() override;
     void DidDematerialize(lldb::ExpressionVariableSP &variable) override;
 
     void RegisterPersistentState(PersistentExpressionState *persistent_state);
     lldb::ExpressionVariableSP &GetVariable();
 
   private:
     PersistentExpressionState *m_persistent_state;
     lldb::ExpressionVariableSP m_variable;
+    lldb::TargetSP m_target_sp;
   };
 
   ResultDelegate m_result_delegate;
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_ClangUserExpression_h_

source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

 ClangUserExpression::ClangUserExpression(
     ExecutionContextScope &exe_scope, llvm::StringRef expr,
     llvm::StringRef prefix, lldb::LanguageType language,
     ResultType desired_type, const EvaluateExpressionOptions &options)
     : LLVMUserExpression(exe_scope, expr, prefix, language, desired_type,
                          options),
       m_type_system_helper(*m_target_wp.lock().get(),
                            options.GetExecutionPolicy() ==
-                               eExecutionPolicyTopLevel) {
+                               eExecutionPolicyTopLevel),
+      m_result_delegate(exe_scope.CalculateTarget()) {
   switch (m_language) {
   case lldb::eLanguageTypeC_plus_plus:
     m_allow_cxx = true;
     break;
   case lldb::eLanguageTypeObjC:
     m_allow_objc = true;
     break;
   case lldb::eLanguageTypeObjC_plus_plus:
m_result_synthesizer_up.reset(
       new ASTResultSynthesizer(passthrough, m_top_level, m_target));
 
   return m_result_synthesizer_up.get();
 }
 
 void ClangUserExpression::ClangUserExpressionHelper::CommitPersistentDecls() {
   if (m_result_synthesizer_up.get()) {
     m_result_synthesizer_up->CommitPersistentDecls();
   }
 }
 
-ClangUserExpression::ResultDelegate::ResultDelegate() {}
-
 ConstString ClangUserExpression::ResultDelegate::GetName() {
-  return m_persistent_state->GetNextPersistentVariableName();
+  return m_persistent_state->GetNextPersistentVariableName(*m_target_sp);
 }
 
 void ClangUserExpression::ResultDelegate::DidDematerialize(
     lldb::ExpressionVariableSP &variable) {
   m_variable = variable;
 }
 
 void ClangUserExpression::ResultDelegate::RegisterPersistentState(
     PersistentExpressionState *persistent_state) {
   m_persistent_state = persistent_state;
 }
 
 lldb::ExpressionVariableSP &ClangUserExpression::ResultDelegate::GetVariable() {
   return m_variable;
 }

source/Plugins/ExpressionParser/Go/GoUserExpression.h

 
 namespace lldb_private {
 class GoParser;
 
 class GoPersistentExpressionState : public PersistentExpressionState {
 public:
   GoPersistentExpressionState();
 
-  ConstString GetNextPersistentVariableName() override;
+  ConstString GetNextPersistentVariableName(Target &target) override;
 
   void RemovePersistentVariable(lldb::ExpressionVariableSP variable) override;
 
   lldb::addr_t LookupSymbol(const ConstString &name) override {
     return LLDB_INVALID_ADDRESS;
   }
 
   static bool classof(const PersistentExpressionState *pv) {

source/Plugins/ExpressionParser/Go/GoUserExpression.cpp

if (!result_val_sp) {
     return lldb::eExpressionDiscarded;
   }
   result.reset(new ExpressionVariable(ExpressionVariable::eKindGo));
   result->m_live_sp = result->m_frozen_sp = result_val_sp;
   result->m_flags |= ExpressionVariable::EVIsProgramReference;
   PersistentExpressionState *pv =
       target->GetPersistentExpressionStateForLanguage(eLanguageTypeGo);
   if (pv != nullptr) {
-    result->SetName(pv->GetNextPersistentVariableName());
+    result->SetName(pv->GetNextPersistentVariableName(*target));
     pv->AddVariable(result);
   }
   return lldb::eExpressionCompleted;
 }
 
 bool GoUserExpression::GoInterpreter::Parse() {
   for (std::unique_ptr<GoASTStmt> stmt(m_parser.Statement()); stmt;
        stmt.reset(m_parser.Statement())) {
if (!value)
     return nullptr;
   // TODO: Handle special conversions
   return value->Cast(type);
 }
 
 GoPersistentExpressionState::GoPersistentExpressionState()
     : PersistentExpressionState(eKindGo) {}
 
-ConstString GoPersistentExpressionState::GetNextPersistentVariableName() {
+ConstString
+GoPersistentExpressionState::GetNextPersistentVariableName(Target &target) {
   char name_cstr[256];
   // We can't use the same variable format as clang.
   ::snprintf(name_cstr, sizeof(name_cstr), "$go%u",
-             m_next_persistent_variable_id++);
+             target.GetNextPersistentVariableIndex());
   ConstString name(name_cstr);
   return name;
 }
 
 void GoPersistentExpressionState::RemovePersistentVariable(
     lldb::ExpressionVariableSP variable) {
   RemoveVariable(variable);
 

source/Target/ABI.cpp

ValueObjectSP ABI::GetReturnValueObject(Thread &thread, CompilerType &ast_type,
 
   // Now turn this into a persistent variable.
   // FIXME: This code is duplicated from Target::EvaluateExpression, and it is
   // used in similar form in a couple
   // of other places.  Figure out the correct Create function to do all this
   // work.
 
   if (persistent) {
+    Target &target = *thread.CalculateTarget();
     PersistentExpressionState *persistent_expression_state =
-        thread.CalculateTarget()->GetPersistentExpressionStateForLanguage(
+        target.GetPersistentExpressionStateForLanguage(
             ast_type.GetMinimumLanguage());
 
     if (!persistent_expression_state)
       return ValueObjectSP();
 
     ConstString persistent_variable_name(
-        persistent_expression_state->GetNextPersistentVariableName());
+        persistent_expression_state->GetNextPersistentVariableName(target));
 
     lldb::ValueObjectSP const_valobj_sp;
 
     // Check in case our value is already a constant value
     if (return_valobj_sp->GetIsConstant()) {
       const_valobj_sp = return_valobj_sp;
       const_valobj_sp->SetName(persistent_variable_name);
     } else