[lldb] Fix that the expression commands --top-level flag overwrites --allow-jit false

[teemperor]

Summary

The --allow-jit flag allows the user to force the IR interpreter to run the provided expression.
The --top-level flag parses and injects the code as if its in the top level scope of a source file.

Both flags just change the ExecutionPolicy of the expression:

• --allow-jit true -> doesn't change anything (its the default)
• --allow-jit false -> ExecutionPolicyNever
• --top-level -> ExecutionPolicyTopLevel

Passing --allow-jit false and --top-level currently causes the --top-level to silently overwrite
the ExecutionPolicy value that was set by --allow-jit false. There isn't any ExecutionPolicy
value that says "top-level but only interpret", so I would say we reject this combination of
flags until someone finds time to refactor top-level feature out of the ExecutionPolicy enum.

The SBExpressionOptions suffer from a similar symptom as SetTopLevel and SetAllowJIT
just silently disable each other. But those functions don't have any error handling, so not
a lot we can do about this in the meantime.

[labath]

seems reasonable

[kastiglione]

But those functions don't have any error handling, so not a lot we can do about this in the meantime.

Could they throw an exception? Or log a warning?

lldb/source/Commands/CommandObjectExpression.cpp

bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
   // that use an input reader...
   ExecutionContext exe_ctx(m_interpreter.GetExecutionContext());
   Target *exe_target = exe_ctx.GetTargetPtr();
   Target &target = exe_target ? *exe_target : GetDummyTarget();
 
   lldb::ValueObjectSP result_valobj_sp;
   StackFrame *frame = exe_ctx.GetFramePtr();
 
+  if (m_command_options.top_level && !m_command_options.allow_jit) {
+    result.AppendErrorWithFormat(
+        "Can't disable JIT compilation for top-level expressions.\n");
+    result.SetStatus(eReturnStatusFailed);
+    return false;
+  }
+
   const EvaluateExpressionOptions options = GetEvalOptions(target);
   ExpressionResults success = target.EvaluateExpression(
       expr, frame, result_valobj_sp, options, &m_fixed_expression);
 
   // We only tell you about the FixIt if we applied it.  The compiler errors
   // will suggest the FixIt if it parsed.
   if (!m_fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {
     if (success == eExpressionCompleted)

lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py

def expr_options_test(self):
         options.SetAllowJIT(True)
         self.assertEqual(options.GetAllowJIT(), True, "Set back to True correctly")
 
         # And again, make sure this works:
         result = frame.EvaluateExpression("call_me(10)", options)
         self.assertSuccess(result.GetError())
         self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
 
+    def test_allow_jit_with_top_level(self):
+        """Test combined --allow-jit and --top-level flags"""
+        # Can't force interpreting for top-level expressions which are always
+        # injected.
+        self.expect("expr --allow-jit false --top-level -- int i;", error=True,
+                    substrs=["Can't disable JIT compilation for top-level expressions."])
+
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "Set a breakpoint here", lldb.SBFileSpec("main.c"))
+        # Allowing JITing for top-level expressions is redundant but should work.
+        self.expect("expr --allow-jit true --top-level -- int top_level_f() { return 2; }")
+        # Make sure we actually declared a working top-level function.
+        self.expect_expr("top_level_f()", result_value="2")