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

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 7 12:07:33 PST 2016


vsk added a comment.

>> That's the end goal I'm aiming for in the current change, but by a different route: for regexes compiled from static char[] constants, I'm just letting the Expected<Regex> propagate the error.
> 
> err, rather, *not* propagate the error -- and crash with asserts enabled.

I think it'd be better to add in 'no error' asserts to the methods in Regex. The advantages are that (1) we can catch broken regexes / broken tests up front, instead of waiting for downstream projects to opt-in to the new constructor, (2) it doesn't require any code changes outside of the Regex class (except to fix broken code), and (3) it doesn't introduce any unchecked uses of Expected, which IMO is an abuse of the error handling API.


https://reviews.llvm.org/D27463





More information about the llvm-commits mailing list