[LLDB] Allow expression evaluators to set arbitrary timeouts

[wallace]

Summary

https://github.com/llvm/llvm-project/commit/59237bb52c9483fce395428bfab5996eabe54ed0 changed the behavior of the SetTimeout and GetTimeout methods of EvaluateExpressionOptions, forbidding the use of infinite timeouts, which are specified with Timeout<std::micro>(std::nullopt).
This broke the Mojo REPL and related services (https://docs.modular.com/mojo/) because it relies on having infinite timeouts. That's a necessity because developers often use the REPL for executing extremely long-running numeric jobs.
Having said that, EvaluateExpressionOptions shouldn't be that opinionated on this matter anyway. Instead, it should be the responsibility of the evaluator to define which timeout to use for each specific case.

[rriddle]

This makes sense to me, but again will defer to area owners for what the expectation is here.

[wallace]

Btw, the SBAPI expects the old behavior.

// Set the timeout for the expression, 0 means wait forever.
void SetTimeoutInMicroSeconds(uint32_t timeout = 0);

[JDevlieghere]

The motivation behind this change was that the IRInterpreter wasn't interruptible, which means that if you interpret an infinite loop, you'd be stuck forever. I fixed that in 61af957aeaa3 so I think that's less of a concern today. I'm fine with going back to allowing an infinite timeout as long as there's no code path in LLDB that sets it to zero by default.

[jasonmolenda]

@wallace this is breaking TestIRInterpreter.py on macOS now; https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ has been failing since it went in (there was another llvm issue causing failures right before this went in, so it was easy to miss). On my desktop this change results in TestIRInterpreter.py running without end; I think the bots kill tests that run for more than 5 minutes.

[wallace]

@jasonmolenda , yep. I have already reverted this patch. I'll figure out how to make that work nicely with these changes.

[jasonmolenda]

In D157764#4608265, @wallace wrote:

@jasonmolenda , yep. I have already reverted this patch. I'll figure out how to make that work nicely with these changes.

oh lol sorry took me a little while to confirm and I missed that. :)

[JDevlieghere]

In D157764#4608265, @wallace wrote:

@jasonmolenda , yep. I have already reverted this patch. I'll figure out how to make that work nicely with these changes.

My recommendation would be to change the timeout in the test from 0 to 500ms to maintain the previous behavior.

[wallace]

@JDevlieghere , that worked. Thanks!!

lldb/include/lldb/Target/Target.h

#endif
 
   lldb::DynamicValueType GetUseDynamic() const { return m_use_dynamic; }
 
   void
   SetUseDynamic(lldb::DynamicValueType dynamic = lldb::eDynamicCanRunTarget) {
     m_use_dynamic = dynamic;
   }
 
-  const Timeout<std::micro> &GetTimeout() const {
-    assert(m_timeout && m_timeout->count() > 0);
-    return m_timeout;
-  }
+  const Timeout<std::micro> &GetTimeout() const { return m_timeout; }
 
-  void SetTimeout(const Timeout<std::micro> &timeout) {
-    // Disallow setting a non-zero timeout.
-    if (timeout && timeout->count() > 0)
-      m_timeout = timeout;
-  }
+  void SetTimeout(const Timeout<std::micro> &timeout) { m_timeout = timeout; }
 
   const Timeout<std::micro> &GetOneThreadTimeout() const {
     return m_one_thread_timeout;
   }
 
   void SetOneThreadTimeout(const Timeout<std::micro> &timeout) {
     m_one_thread_timeout = timeout;
   }