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

Thomas Preud'homme via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 21 06:08:37 PST 2019


thopre added a comment.

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.


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

https://reviews.llvm.org/D68054





More information about the llvm-commits mailing list