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.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This makes sense to me, but again will defer to area owners for what the expectation is here.
Btw, the SBAPI expects the old behavior.
// Set the timeout for the expression, 0 means wait forever. void SetTimeoutInMicroSeconds(uint32_t timeout = 0);
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.
@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.
@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.