[Lldb-commits] [PATCH] D132307: [lldb] Switch RegularExpression from llvm::Regex to std::regex

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.


================
Comment at: lldb/include/lldb/Utility/RegularExpression.h:92
+  std::regex m_regex;
+  std::optional<std::regex_error> m_regex_error;
 };
----------------
JDevlieghere wrote:
> There's an ongoing discussion on the forums [1] to replace `llvm::Optional` with `std::optional`. Until that's resolved we should stick with llvm's variant for consistency. 
> 
> [1] https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716/10
thanks for the details, I was actually planning to do this change, so that it would be easy to cherry pick into apple/llvm-project.


================
Comment at: lldb/source/Utility/CMakeLists.txt:29
+  PROPERTIES COMPILE_OPTIONS
+  "-fcxx-exceptions"
+)
----------------
JDevlieghere wrote:
> kastiglione wrote:
> > the `std::regex` constructor throws `std::regex_error` if the pattern is invalid. For this reason, exceptions are enabled for this one file.
> What happens when exceptions are disabled? What does it mean to have this enabled for a single file? I don't know if it's part of the LLVM developer guide, but LLVM is supposed to build without RTTI and without exceptions. 
> What happens when exceptions are disabled?

This cmake config enables exceptions for this one file, independent of `LLVM_ENABLE_EH`. No other source files will be allowed to catch or throw exceptions.

> What does it mean to have this enabled for a single file?

It means this file can compile with a try/catch, and that inside this file, exceptions can be caught.

> I don't know if it's part of the LLVM developer guide, but LLVM is supposed to build without RTTI and without exceptions.

llvm has `LLVM_ENABLE_EH` which allows llvm to be built with exceptions support enabled. Similarly, `LLVM_ENABLE_RTTI` allows RTTI to be enabled. It seems that both are disabled as a default, but not as a hard requirement.

I wondered if enabling RTTI would needed for exceptions, but at least for this code, the answer is no. The `catch (const std::regex_error &e)` block is exercised by `TestBreakpointRegexError.py`, so we know this code can and does catch an exception of that type, and accesses the error's member functions.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132307/new/

https://reviews.llvm.org/D132307