Breakpoint conditions were making result variables, which they should not do.

The result variables aren't useful, and if you have a breakpoint on a
common function you can generate a lot of these.  So I changed the
code that checks the condition to set ResultVariableIsInternal in the
EvaluateExpressionOptions that we pass to the execution.
Unfortunately, the check for this variable was done in the wrong place
(the static UserExpression::Evaluate) which is not how breakpoint
conditions execute expressions (UserExpression::Execute).  So I moved
the check to UserExpression::Execute (which Evaluate also calls) and made the
overridden method DoExecute.

llvm-svn: 266093

8 files changed, 48 insertions, 24 deletions

diff --git a/lldb/include/lldb/Expression/LLVMUserExpression.h b/lldb/include/lldb/Expression/LLVMUserExpression.h
index 43ac0eb5803..8775c8bd865 100644
--- a/lldb/include/lldb/Expression/LLVMUserExpression.h
+++ b/lldb/include/lldb/Expression/LLVMUserExpression.h
@@ -40,11 +40,6 @@ public:
                        const EvaluateExpressionOptions &options);
     ~LLVMUserExpression() override;
 
-    lldb::ExpressionResults
-    Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-            const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
-            lldb::ExpressionVariableSP &result) override;
-
     bool
     FinalizeJITExecution(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
                          lldb::ExpressionVariableSP &result,
@@ -70,6 +65,11 @@ public:
     lldb::ModuleSP GetJITModule() override;
 
 protected:
+    lldb::ExpressionResults
+    DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+              const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
+              lldb::ExpressionVariableSP &result) override;
+
     virtual void
     ScanContext(ExecutionContext &exe_ctx, lldb_private::Error &err) = 0;
 
diff --git a/lldb/include/lldb/Expression/UserExpression.h b/lldb/include/lldb/Expression/UserExpression.h
index b8b5e96e029..a038af1b953 100644
--- a/lldb/include/lldb/Expression/UserExpression.h
+++ b/lldb/include/lldb/Expression/UserExpression.h
@@ -107,7 +107,8 @@ public:
     MatchesContext (ExecutionContext &exe_ctx);
 
     //------------------------------------------------------------------
-    /// Execute the parsed expression
+    /// Execute the parsed expression by callinng the derived class's
+    /// DoExecute method.
     ///
     /// @param[in] diagnostic_manager
     ///     A diagnostic manager to report errors to.
@@ -133,9 +134,9 @@ public:
     /// @return
     ///     A Process::Execution results value.
     //------------------------------------------------------------------
-    virtual lldb::ExpressionResults
+    lldb::ExpressionResults
     Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
-            lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result) = 0;
+            lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result);
 
     //------------------------------------------------------------------
     /// Apply the side effects of the function to program state.
@@ -312,6 +313,10 @@ public:
     }
 
 protected:
+    virtual lldb::ExpressionResults
+    DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
+            lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result) = 0;
+
     static lldb::addr_t
     GetObjectPointer (lldb::StackFrameSP frame_sp,
                       ConstString &object_name,
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index ac2d021482b..c0e5343f3b3 100644
--- a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -179,4 +179,8 @@ class BreakpointConditionsTestCase(TestBase):
         # The hit count for the breakpoint should be 1.
         self.assertTrue(breakpoint.GetHitCount() == 1)
 
+        # Test that the condition expression didn't create a result variable:
+        options = lldb.SBExpressionOptions()
+        value = frame0.EvaluateExpression("$0", options)
+        self.assertTrue(value.GetError().Fail(), "Conditions should not make result variables.")
         process.Continue()
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 1d66d9dc6a4..3c3263543d5 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -324,6 +324,7 @@ BreakpointLocation::ConditionSaysStop (ExecutionContext &exe_ctx, Error &error)
     options.SetUnwindOnError(true);
     options.SetIgnoreBreakpoints(true);
     options.SetTryAllThreads(true);
+    options.SetResultIsInternal(true); // Don't generate a user variable for condition expressions.
 
     Error expr_error;
 
diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp
index 3c42a540535..60f68b129d3 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -77,9 +77,9 @@ LLVMUserExpression::~LLVMUserExpression()
 }
 
 lldb::ExpressionResults
-LLVMUserExpression::Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-                            const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
-                            lldb::ExpressionVariableSP &result)
+LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+                              const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
+                              lldb::ExpressionVariableSP &result)
 {
     // The expression log is quite verbose, and if you're just tracking the execution of the
     // expression, it's quite convenient to have these logs come out with the STEP log as well.
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index 2438708a79d..3e2e07e9cb2 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -356,11 +356,6 @@ UserExpression::Evaluate (ExecutionContext &exe_ctx,
             execution_results =
                 user_expression_sp->Execute(diagnostic_manager, exe_ctx, options, user_expression_sp, expr_result);
 
-            if (options.GetResultIsInternal() && expr_result && process)
-            {
-                process->GetTarget().GetPersistentExpressionStateForLanguage(language)->RemovePersistentVariable (expr_result);
-            }
-
             if (execution_results != lldb::eExpressionCompleted)
             {
                 if (log)
@@ -405,3 +400,21 @@ UserExpression::Evaluate (ExecutionContext &exe_ctx,
 
     return execution_results;
 }
+
+lldb::ExpressionResults
+UserExpression::Execute(DiagnosticManager &diagnostic_manager,
+                        ExecutionContext &exe_ctx,
+                        const EvaluateExpressionOptions &options,
+                        lldb::UserExpressionSP &shared_ptr_to_me,
+                        lldb::ExpressionVariableSP &result_var)
+{
+    lldb::ExpressionResults expr_result = DoExecute(diagnostic_manager, exe_ctx, options, shared_ptr_to_me, result_var);
+    Target *target = exe_ctx.GetTargetPtr();
+    if (options.GetResultIsInternal() && result_var && target)
+    {
+        target->GetPersistentExpressionStateForLanguage(m_language)->RemovePersistentVariable (result_var);
+    }
+    return expr_result;
+}
+
+
diff --git a/lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp
index 0457bfe0f32..f69c3e23457 100644
--- a/lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp
@@ -265,9 +265,9 @@ GoUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionContext
 }
 
 lldb::ExpressionResults
-GoUserExpression::Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-                          const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
-                          lldb::ExpressionVariableSP &result)
+GoUserExpression::DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+                            const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
+                            lldb::ExpressionVariableSP &result)
 {
     Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EXPRESSIONS | LIBLLDB_LOG_STEP));
 
diff --git a/lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.h b/lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.h
index 4d6cdd2fcd1..711a4c46215 100644
--- a/lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.h
+++ b/lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.h
@@ -70,11 +70,6 @@ class GoUserExpression : public UserExpression
             lldb_private::ExecutionPolicy execution_policy, bool keep_result_in_memory,
             bool generate_debug_info) override;
 
-      lldb::ExpressionResults
-      Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-              const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
-              lldb::ExpressionVariableSP &result) override;
-
       bool
       CanInterpret() override
       {
@@ -89,6 +84,12 @@ class GoUserExpression : public UserExpression
           return true;
     }
 
+  protected:
+      lldb::ExpressionResults
+      DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+                const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
+                lldb::ExpressionVariableSP &result) override;
+
   private:
     class GoInterpreter;
     std::unique_ptr<GoInterpreter> m_interpreter;