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

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: lib/Support/Regex.cpp:88-90
+  // Check if the regex itself didn't successfully compile.
+  if (Error ? !isValid(*Error) : !isValid())
     return false;
----------------
nlguillemot wrote:
> 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?
Yes, doing it in a separate would be nice. Ideally update call sites in a first patch and then apply this patch on top with those asserts I suggested.


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

https://reviews.llvm.org/D67241