[PATCH] D67241: Regex: Make "match" and "sub" const member functions

[Nicolas Guillemot via Phabricator via llvm-commits]

nlguillemot marked 2 inline comments as done.
nlguillemot added inline comments.


================
Comment at: include/llvm/Support/Regex.h:76-77
     ///
+    /// \param Error - If non-null, any errors in the matching will be recorded
+    /// as a non-empty string.
+    ///
----------------
thopre wrote:
> Could you mention that any error will be cleared by the function?
The error is cleared in both `match` and `sub`, so I'll update the comment for both of those.


================
Comment at: lib/Support/Regex.cpp:88-90
+  // Check if the regex itself didn't successfully compile.
+  if (Error ? !isValid(*Error) : !isValid())
     return false;
----------------
thopre wrote:
> I think I'd prefer the API to enforce the user to check compile error when creating the regex and just have an assert here. match should only report errors in matching IMHO.
I completely agree in principle, though right now that would cause some API breakage.
For example, the following usage in `lib/Transforms/Utils/SymbolRewriter.cpp`:

```
     std::string Name = Regex(Pattern).sub(Transform, C.getName(), &Error);
     if (!Error.empty())
       report_fatal_error("unable to transforn " + C.getName() + " in " +
                          M.getModuleIdentifier() + ": " + Error);
```

I guess we could make the change anyways and update all the call sites. Maybe good for a follow-up patch?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67241