[PATCH] D68054: Regex: Add static convenience functions for "match" and "sub"

[Nicolas Guillemot via Phabricator via llvm-commits]

nlguillemot added a comment.

In D68054#1755059 <https://reviews.llvm.org/D68054#1755059>, @thopre wrote:

> Not committing to remove the ability to call match/sub on an invalid regex implies that the addition of the new API needs to be justified independently of the extra check that needs to be performed in match/sub to allow the current behaviour. And in fact, the only motivation for this patch I could think of was consistency with other languages and I was finding it a hard sell and was about to apologise for the extra work I made you do.
>
> However I overlooked one benefit of removing the ability to match an invalid regex: robustness. All the cases where a call to match/sub is guards a code other than throwing an error is likely to result in wrong behaviour in case the regex is invalid. So I think it would be safer to use a separate API for combined regex creation + match/sub as per the previous revision of this patch and prevent the existing approach, so that users are mindful when not doing separate checks.
>
> What do you think? If you agree that would mean reverting to the previous version of this patch and the simultaneous apologies from me for requesting the current version.


If I understand correctly, you're suggesting we re-add the assert inside `match()` that requires the regex to be compiled successfully as a precondition? If so, I agree that the API should be strict. The problem with the `assert()` was that it affected other projects like clang, but now that there's the monorepo I feel more confident that we can safely make a breaking change by fixing any uses in the other projects in the monorepo.

If we want to re-add the `assert()`, I would suggest that we first commit this patch to lay the groundwork, then create a separate patch that adds the `assert()` back in. Isolating the patch with the API breakage would make it easier to revert if necessary.

By the way, I'm going to be on vacation from tomorrow up to December 3rd, so I might not be able to answer to comments in a timely way. Apologies in advance.


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

https://reviews.llvm.org/D68054