[PATCH] D27463: Change llvm::Regex to expose a fallible constructor.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: include/llvm/Support/YAMLTraits.h:437-438
+  }
+  // Regex constant failed to compile -- this crashes with asserts enabled.
   return false;
 }
----------------
In addition to what Richard said, I would also try to use a pattern that ends up working more like:

  if (auto MyMatcher = Regex::compile(...))
    return MyMatcher->match(...);
  else
    llvm_unreachable("boom");

This will give us nice assert like behavior in assert builds and will actually kill the other edge in optimized builds.


================
Comment at: include/llvm/Support/YAMLTraits.h:434-437
+          "^(\\.[0-9]+|[0-9]+(\\.[0-9]*)?)([eE][-+]?[0-9]+)?$")) {
+    if (FloatMatcher->match(S))
+      return true;
+  }
----------------
No need for braces IMO.


================
Comment at: lib/IR/DiagnosticInfo.cpp:45-54
     if (!Val.empty()) {
-      Pattern = std::make_shared<Regex>(Val);
-      std::string RegexError;
-      if (!Pattern->isValid(RegexError))
+      if (auto RegexOrError = Regex::compile(Val)) {
+        Pattern = std::make_shared<Regex>(std::move(*RegexOrError));
+      } else {
         report_fatal_error("Invalid regular expression '" + Val +
-                               "' in -pass-remarks: " + RegexError,
+                               "' in -pass-remarks: " +
+                               toString(RegexOrError.takeError()),
----------------
Do you mind reflowing all of this to use early return so that there are less braces and indentation?


https://reviews.llvm.org/D27463